Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TextInput: remove :invalid styling on some browsers #576

Merged
merged 2 commits into from Sep 4, 2019

Conversation

@sohkai
Copy link
Member

commented Sep 4, 2019

Removes the default :invalid styling that some browsers (e.g. Firefox) apply to inputs.

@sohkai sohkai requested a review from bpierre Sep 4, 2019

border-color: ${theme.border};
}
&:invalid {
border-color: ${value ? theme.negative : theme.border};

This comment has been minimized.

Copy link
@sohkai

sohkai Sep 4, 2019

Author Member

This isn't super nice... but it looks like an input is invalid if it's required and has no value (so it always starts by default in invalid).

This comment has been minimized.

Copy link
@bpierre

bpierre Sep 4, 2019

Member

mmm yes, I think it’s a good idea to style these, but relying only on :invalid would probably cause problems. Every browser has its own validation mechanism, and doesn’t integrate really well with JavaScript / React where the validation is happening at another level. We could try using element.setCustomValidity()… but it would mean having the error messages reported in different ways depending on the browser, if supported at all.

What would you think of leaving this style for :invalid (so we stop having the default one when a field is invalid), but also adding an invalid prop, that will apply the same style? Or maybe we need different levels to be represented, in which case we could have something like validity="success / warning / error / default" /cc @dizzypaty

This comment has been minimized.

Copy link
@sohkai

sohkai Sep 4, 2019

Author Member

That sounds good, I'd agree that moving the invalid styling up a level sounds like the least headache down the line.

This comment has been minimized.

Copy link
@sohkai

sohkai Sep 4, 2019

Author Member

Added 00a251c to just remove the default :invalid styling.

Let's review the error states afterwards and add the invalid prop then.

@sohkai sohkai changed the title TextInput: add invalid state styling TextInput: remove :invalid styling on some browsers Sep 4, 2019

@bpierre
bpierre approved these changes Sep 4, 2019

@sohkai sohkai merged commit 96932c9 into newstyle Sep 4, 2019

2 of 5 checks passed

License Compliance FOSSA is analyzing this commit
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
WIP Ready for review
Details
license/cla Contributor License Agreement is signed.
Details

@delete-merged-branch delete-merged-branch bot deleted the input-invalid-state branch Sep 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.