This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
Initial portions of support for Field validation #2780
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* renames RoomTooltip to be a generic Tooltip (which it is) * hooks it into Field to show validation results * adds onValidate to Field to let Field instances call an arbitrary validation function Rebased from @ara4n's matrix-org#2550 by @jryans. Subsequent commits revise and adapt this work.
* The field border style was previously moved up to the field * Validity colors should be shown regardless of focus state
This checks `onValidate` in `render` to make the linter happy.
Always set some value on the Field's input so that it doesn't flip flop between controlled and uncontrolled.
As part of adding validation to Field, the logic is simpler to follow if we can assume that all usages of Field use it as a controlled component, instead of supporting both controlled and uncontrolled. This converts the uncontrolled usages to controlled.
Field is no longer used as an uncontrolled component, so we can remove some supporting code that we no longer need.
This is example code from @ara4n's work in matrix-org#2550. We're not ready to actually apply validation yet, so removing this for now.
turt2live
approved these changes
Mar 13, 2019
There was a problem hiding this comment.
I'm trusting and assuming 40f16fa is okay - I Haven't bothered to look
Overall the changes look sensible though, and well laid out. It's a bit of a shame to lose uncontrolled inputs, but I can see how annoying it is to deal with :(
jryans
added a commit
to jryans/matrix-react-sdk
that referenced
this pull request
Mar 14, 2019
This aligns the code in `RegistrationForm` with other users of the `Field` component. (In matrix-org#2780, I had thought that this code would be okay to leave alone, but I had missed the usage of the `Field` value getter.) Fixes element-hq/element-web#9172
dbkr
added a commit
that referenced
this pull request
Mar 15, 2019
#2780 renamed RoomTooltip (to Tooltip) but missed the references in the custom tag panel.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reviewer: Most commits in this PR are fairly self-contained, so you may want to review commit by commit, since there are quite a few changes here.
This extends @ara4n's previously reviewed (but unmerged) validation prototype from #2550. (The first commit is that same work rebased, and so it could be ignored / reviewed more lightly.)
This adds a basic facility for showing validation with Fields. However, the UX of the validation itself has still needs to be tweaked and improved. It is also not applied to any Fields in this work. You can see an example of the API from the prototype code.
I am putting this code up for review now since there are already a lot of changes here, so it seems best to review this chunk and then iterate from there.
Part of element-hq/element-web#8170 and element-hq/element-web#8292.