TextField and TextArea: add error prop
#2347
Conversation
…ively. This is useful for validation that happens after a form submission
…tively. This is useful for validation that happens after a form submission
🦋 Changeset detectedLatest commit: 3240bef The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Size Change: +38 B (+0.04%) Total Size: 101 kB
ℹ️ View Unchanged
|
| @@ -10,8 +10,6 @@ import {TextArea} from "@khanacademy/wonder-blocks-form"; | |||
| /** | |||
| * The following stories are used to generate the pseudo states for the | |||
| * TextArea component. This is only used for visual testing in Chromatic. | |||
| * | |||
| * Note: Error state is not shown on initial render if the TextArea value is empty. | |||
There was a problem hiding this comment.
Since we can explicitly put the component in an error state, the variants stories can properly show the error styling in all cases now! Previously, we weren't able to show an empty field with an error state since validation wouldn't be triggered yet in that case.
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-piwkdvgisf.chromatic.com/ Chromatic results:
|
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (ad3dd5d) and published all packages with changesets to npm. You can install the packages in webapp by running: ./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2347"Packages can also be installed manually by running: yarn add @khanacademy/wonder-blocks-<package-name>@PR2347 |
| /** | ||
| * Whether the textarea is in an error state. | ||
| */ | ||
| error?: boolean; |
There was a problem hiding this comment.
I feel that we could get into a tricky situation here.
Have you found a situation where a component instance needs to use both error and validate?
I'm thinking if we need to add more guardrails to prevent a case where these two props could collide.
It's tricky from the API perspective that the validation function would pass but the user would still see an error, and I wonder if this case is expected or not (and if not, then we should prevent from this to happen from the dev perspective).
Here's an example:
https://5e1bf4b385e3fb0020b7073c-kwxacxtfdq.chromatic.com/iframe.html?args=error:!true&id=packages-form-textarea--error-from-validation&viewMode=story
The email pattern is now valid, but the input is still showing its error state.
I wonder if/how we need to cover this case. wdyt?
There was a problem hiding this comment.
Great question! I think this scenario is behaving as I would have expected (remaining in an error state after the email is in a valid format because the error prop is set to true). An example use case for this could be a sign up form where even if the email is valid, there could still be an error after form submission like the email provided is already associated with an account.
The workaround I used for backend validation errors before was to keep track of whether there is an error in the component state, check for that error in the validate function, and then also set the key on the component to whether there was an error so that the component would re-mount to trigger the validate function. Here is an example from webapp where I ran into this issue!
Part of the confusion could also be there is no error message in the story. The error message isn't part of the component, but it is shown in the story for example purposes. I'll update the story so another error message is shown if the error control is set!
Let me know what you think or if you have some ideas on other alternatives!
There was a problem hiding this comment.
oh I see, thanks for the context.
Given your example, I still see that the user could get into an state where they use an email that is not associated with an account and they use valid email. Should the input update to its valid state? or should it remain using the error state?
There was a problem hiding this comment.
The input would return to a valid state when the error prop is set to false (since in that case the email is fine to use).
I'll put together another story that has both error and validate props configured with logic so we can see how it will behave!
There was a problem hiding this comment.
The input would return to a valid state when the error prop is set to false (since in that case the email is fine to use).
That's the point I'm trying to get into.... this could lead into an "make impossible states impossible" situation. We would need to make sure that both error and validate are in sync and there might be cases where it would be confusing for the developer implementing a feature with these characteristics.
I wonder if there's a way to refactor this API to make it more predictable (even if it means changing validate/onValidate).
There was a problem hiding this comment.
Thanks for the feedback, Juan! From our pairing session, we decided that:
- it makes sense to support both
errorandvalidateprops in the TextField and TextArea components since there are use cases for using both - we will need documentation around when to use these props
- we will leave validation up to the field components instead of having LabeledField pass it down. This means LabeledField will only have an error prop for the error message and the validate prop will stay on TextField/TextArea
- it will be helpful to have a shared utility/hook for the shared logic around validation: when to trigger validation, support for async validation, etc (will be explored in another PR).
I've added some more documentation around when to use error vs validate props, and have added examples for when both are used (TextField example and TextArea example)! I'll update the implementation spec as well!
Let me know what you think!
… ControlledTextArea to show a different error message if the error control is enabled
…e ErrorRender exampleto show a different error message if the error control is enabled
…rror prop is set to true. also updating docs to focus more on the error state rather than error messages (since the error messages are part of LabeledField)
beaesguerra
force-pushed
the
form-error-states
branch
from
adaf196
to
3240bef
Compare
Summary:
Adds an
errorprop to TextField and TextArea components so they can be put in an error state declaratively. This is useful for validation that happens after a form submission.Related to the LabeledField work:
errorprop totrueif it is provided with an error message (will do this in a separate PR for LabeledField work)Issue: WB-1777
Test plan:
TextField
errorprop puts the component in an error state (aria-invalidis set totrueand error styling is applied) (?path=/story/packages-form-textfield--error)?path=/story/packages-form-textfield--error-from-validation)?path=/story/packages-form-textfield--required)TextArea
errorprop puts the component in an error state (aria-invalidis set totrueand error styling is applied) (?path=/story/packages-form-textarea--error)?path=/story/packages-form-textarea--error-from-validation)?path=/story/packages-form-textarea--required)