Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

md-input-container: ng-touched added on page load #1485

Closed
epelc opened this issue Feb 12, 2015 · 24 comments
Closed

md-input-container: ng-touched added on page load #1485

epelc opened this issue Feb 12, 2015 · 24 comments
Assignees
Milestone

Comments

@epelc
Copy link
Contributor

epelc commented Feb 12, 2015

I think this commit has broken the styles for md-input-container. It's adding the invalid styles when the page is loaded instead of after the user first touches the input. Also I believe ng-touched is not supposed to be added on page load.

@epelc
Copy link
Contributor Author

epelc commented Feb 12, 2015

I should mention this is for inputs with required and possible the min/max length attributes.

@ThomasBurleson
Copy link
Contributor

@epelc - here is the issue. If the input field is invalid during page load/initialization, the previous solution does not indicate such. As soon as you touch it and defocus it shows the error style. This is a horrible UX: if the value is invalid show the style immediately. Hence the change(s).

@epelc
Copy link
Contributor Author

epelc commented Feb 12, 2015

@ThomasBurleson What about for required fields? It makes your form look terrible if half your inputs are red when they load especially for a sign in page. Also the user has a general idea that most things are required why do we have to scream it to them when they show up? Wouldn't it be better to guide them as they fill out the form instead?

As a side note this is also how most forms already work so it breaks what the user has already learned. Ie google's sign up page only warns you about an invalid input after you have touched it or input something invalid.
https://accounts.google.com/SignUp

@grabbou
Copy link

grabbou commented Feb 12, 2015

@ThomasBurleson I believe it should show the error as soon as you touch it. For now - even simple login form is completely red as "required" & "email" are obviously wrong on first page load.

Also - on initial page load, my ng-messages are showing ng-message="required" by default.

screen shot 2015-02-12 at 19 14 27

and without ng-if="form.input.$dirty" (should not be needed)

screen shot 2015-02-12 at 19 15 18

With above approach, doing more complex form of any kind to handle API endpoints is just not going to work.

@AllenSH12
Copy link

FWIW here's what the Material spec says on the topic:
http://www.google.com/design/spec/patterns/errors.html#errors-user-input-errors

@grabbou
Copy link

grabbou commented Feb 12, 2015

+1 @AllenSH12

@ThomasBurleson
Copy link
Contributor

@AllenSH12 - Thank you for the link. I did not realize that Material Design is explicit. Very nice... now I will queue this issues as a definite bug. Thx again.

@ThomasBurleson ThomasBurleson added this to the 0.8.0 milestone Feb 12, 2015
@ThomasBurleson ThomasBurleson self-assigned this Feb 12, 2015
@AllenSH12
Copy link

@ThomasBurleson no problem!

@AllenSH12
Copy link

In the meantime I tweaked the Error demo to demo the behavior more explicitly, PR here: #1491

@ThomasBurleson
Copy link
Contributor

@grabbou - can you provide a CodePen or Plunkr that I can use to verify changes ?

@grabbou
Copy link

grabbou commented Feb 14, 2015

Can do, will update this tomorrow.

@ThomasBurleson
Copy link
Contributor

@AllenSH12 , @epelc - can you check with latest in master to confirm that this issue has been resolved ? Thank you.

@epelc
Copy link
Contributor Author

epelc commented Feb 14, 2015

@ThomasBurleson It seems be to fixed on the latest master. But I also think it's odd how it immediately becomes invalid when you touch it(before you type anything). I think a delay or some other sort of logic to wait until after you type something would be better. But these rules are a bit hard to get right.

I also noticed on google stripe and github they don't even show validation errors on their signin page.

@ThomasBurleson
Copy link
Contributor

@epelc - I followed the Material Design spec which states:

Error text should not appear before user interaction with the field.

For me this means that a click or focus on the element is a "user interaction". Thus the immediate indication that the error condition exists.

@epelc
Copy link
Contributor Author

epelc commented Feb 14, 2015

@ThomasBurleson I think you did it right. It seems correct on every other form I have besides the login page. Perhaps because so many sites don't validate your input on login pages I've been tricked into expecting this.

@AllenSH12
Copy link

@ThomasBurleson looks good thank you again

@AllenSH12
Copy link

(sorry accidentally submitted last comment before ready, continuing...)

@ThomasBurleson is there anything I can do to have my PRs merged instead of included indirectly in the future?

@ThomasBurleson
Copy link
Contributor

@AllenSH12 - I asked you in your PR if you had signed the CLA. You did not reply. So I copied your changes. I will only merge valid changes from contributors if a the required CLA has been signed.

@AllenSH12
Copy link

@ThomasBurleson that makes sense, thank you for clarifying.

@fuhlig
Copy link

fuhlig commented Feb 16, 2015

The error stil exists (as shown below on the demo site). Seems like .md-input-invalid is not toggled correctly on <md-input-container>
md-input_error

@ThomasBurleson
Copy link
Contributor

@fuhlig - re-opening to fix.

@grabbou
Copy link

grabbou commented Feb 16, 2015

Yep, can confirm that @fuhlig issue appears in mine project as well. Weird, as material demo seems to handle that properly.

@mbellezi
Copy link

I think this problem is related: If you use the browser memorized values for common fields (Name, city, etc.), the fields are filled with the values but the md-input-container does not update properly. Using a e-mail validated field, for example, when autocompleted, you have to type several times for the update in the container takes place. You can test here:

http://codepen.io/mbellezi/pen/ByxdJB

@ThomasBurleson
Copy link
Contributor

Fixed, Closed with SHA 747eb9c

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants