Skip to content

Add error message for incorrect password. Fix issue #55#201

Merged
kylecarbs merged 1 commit into
coder:masterfrom
MichaelDesantis:fix/password-error-notify
Mar 12, 2019
Merged

Add error message for incorrect password. Fix issue #55#201
kylecarbs merged 1 commit into
coder:masterfrom
MichaelDesantis:fix/password-error-notify

Conversation

@MichaelDesantis
Copy link
Copy Markdown
Contributor

Describe in detail the problem you had and how this PR fixes it

Issue:
On initial welcome/login screen of binary, no error message is displayed in the event of an incorrect password. (See #55)

Changes:
Adds simple logic and styling for error display. Currently is of identical styling/design to the coder.com login page.

Test Environment:
OS: Mac OS High Sierra (10.13.6)
Browser: Google Chrome 72.0.3626.109 (Official Build) (64-bit)
(Result: Pass! Compile was successful and everything functions as expected)

Caveats:
None that I can think of at the moment. (I mean, the error message is hard coded. But this page has only one input and the page only has one purpose and does exactly one thing. So I don't know that this is necessarily a bad thing)

@MichaelDesantis
Copy link
Copy Markdown
Contributor Author

@nhooyr Can you review this PR? I can't request you as a reviewer and Asher and Kyle probably have a backlog that will last for a while.

@nhooyr
Copy link
Copy Markdown
Contributor

nhooyr commented Mar 12, 2019

@kylecarbs why can't he request me as a reviewer?

Comment thread packages/app/browser/src/app.html
Comment thread packages/app/browser/src/app.ts
Copy link
Copy Markdown
Contributor

@avelino avelino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kylecarbs kylecarbs merged commit 0efae1f into coder:master Mar 12, 2019
@MichaelDesantis MichaelDesantis deleted the fix/password-error-notify branch March 13, 2019 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants