Add `instantValidation` prop for TextArea #2355

beaesguerra · 2024-11-06T17:13:11Z

Summary:

Adds instantValidation prop so there is an option to validate on blur. If instantValidation is false, the error is also cleared on change
If textarea is required and instantValidation is false, validate whenever the user tabs away from the field

Next: I'll work on refactoring TextField to a function component first and then will add the same instantValidation prop to TextField

Test plan:

Instant Validation docs are reviewed (/?path=/docs/packages-form-textarea--docs#instant%20validation)
Instant Validation works as expected (see docs for expected behaviour) (/?path=/story/packages-form-textarea--instant-validation)

changeset-bot · 2024-11-06T17:13:16Z

🦋 Changeset detected

Latest commit: bee2c97

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@khanacademy/wonder-blocks-form	Minor
@khanacademy/wonder-blocks-search-field	Patch
@khanacademy/wonder-blocks-dropdown	Patch
@khanacademy/wonder-blocks-birthday-picker	Patch

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

github-actions · 2024-11-06T17:15:05Z

Size Change: +86 B (+0.09%)

Total Size: 101 kB

Filename	Size	Change
`packages/wonder-blocks-form/dist/es/index.js`	6.33 kB	+86 B (+1.38%)

ℹ️ View Unchanged

Filename	Size
`packages/wonder-blocks-accordion/dist/es/index.js`	3.78 kB
`packages/wonder-blocks-banner/dist/es/index.js`	1.53 kB
`packages/wonder-blocks-birthday-picker/dist/es/index.js`	1.77 kB
`packages/wonder-blocks-breadcrumbs/dist/es/index.js`	887 B
`packages/wonder-blocks-button/dist/es/index.js`	4.04 kB
`packages/wonder-blocks-cell/dist/es/index.js`	2.01 kB
`packages/wonder-blocks-clickable/dist/es/index.js`	3.06 kB
`packages/wonder-blocks-core/dist/es/index.js`	3.44 kB
`packages/wonder-blocks-data/dist/es/index.js`	6.24 kB
`packages/wonder-blocks-dropdown/dist/es/index.js`	18.2 kB
`packages/wonder-blocks-grid/dist/es/index.js`	1.36 kB
`packages/wonder-blocks-i18n/dist/es/index.js`	4.76 kB
`packages/wonder-blocks-icon-button/dist/es/index.js`	3 kB
`packages/wonder-blocks-icon/dist/es/index.js`	871 B
`packages/wonder-blocks-labeled-field/dist/es/index.js`	72 B
`packages/wonder-blocks-layout/dist/es/index.js`	1.82 kB
`packages/wonder-blocks-link/dist/es/index.js`	2.28 kB
`packages/wonder-blocks-modal/dist/es/index.js`	5.36 kB
`packages/wonder-blocks-pill/dist/es/index.js`	1.65 kB
`packages/wonder-blocks-popover/dist/es/index.js`	4.87 kB
`packages/wonder-blocks-progress-spinner/dist/es/index.js`	1.52 kB
`packages/wonder-blocks-search-field/dist/es/index.js`	1.3 kB
`packages/wonder-blocks-switch/dist/es/index.js`	1.94 kB
`packages/wonder-blocks-testing-core/dist/es/index.js`	3.74 kB
`packages/wonder-blocks-testing/dist/es/index.js`	1.07 kB
`packages/wonder-blocks-theming/dist/es/index.js`	693 B
`packages/wonder-blocks-timing/dist/es/index.js`	1.8 kB
`packages/wonder-blocks-tokens/dist/es/index.js`	2.36 kB
`packages/wonder-blocks-toolbar/dist/es/index.js`	827 B
`packages/wonder-blocks-tooltip/dist/es/index.js`	7.08 kB
`packages/wonder-blocks-typography/dist/es/index.js`	1.23 kB

_{compressed-size-action}

github-actions · 2024-11-06T17:16:57Z

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-crdgebdozd.chromatic.com/

Chromatic results:

Metric	Total
Captured snapshots	369
Tests with visual changes	0
Total stories	502
Inherited (not captured) snapshots [TurboSnap]	0
Tests on the build	369

beaesguerra · 2024-11-06T17:31:38Z

__docs__/wonder-blocks-form/text-area.stories.tsx

+    render: (args) => {
+        return (
+            <View style={{gap: spacing.small_12}}>
+                <LabelSmall htmlFor="instant-validation-true-not-required">


Will replace with LabeledField after. I'll be merging/releasing the form field updates separately from LabeledField (LabeledField changes are in feature/labeled-field right now, and I'll update it with these form field changes after!)

khan-actions-bot · 2024-11-06T17:44:32Z

Gerald

Required Reviewers

@Khan/wonder-blocks for changes to .changeset/chatty-moles-brush.md, __docs__/wonder-blocks-form/text-area.stories.tsx, packages/wonder-blocks-form/src/components/text-area.tsx, packages/wonder-blocks-form/src/components/__tests__/text-area.test.tsx

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

github-actions · 2024-11-06T17:45:10Z

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (50f71bb) 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="PR2355"

Packages can also be installed manually by running:

yarn add @khanacademy/wonder-blocks-<package-name>@PR2355

jandrade

This looks great, thanks a lot for putting so much detail in the tests and documentation

jandrade · 2024-11-06T18:57:01Z

packages/wonder-blocks-form/src/components/__tests__/text-area.test.tsx

+            });
+
+            describe("instantValidation=true", () => {
+                it("should call validate each time the value changes if the instantValidation prop is true", async () => {


nit: The last part of the test statement is redundant so you could probably remove it if you'd like (no change required). For context, the test should display in the CLI as:

instantValidation=true > should call validate each time the value changes if the instantValidation prop is true

Suggested change

it("should call validate each time the value changes if the instantValidation prop is true", async () => {

it("should call validate each time the value changes", async () => {

Good call! I removed the redundant parts for the test names 😄

jandrade · 2024-11-06T18:59:14Z

packages/wonder-blocks-form/src/components/__tests__/text-area.test.tsx

+                    expect(validate.mock.calls).toStrictEqual([
+                        ["t"],
+                        ["te"],
+                        ["tes"],
+                        ["test"],
+                    ]);


praise: This is a neat way to test multiple things while using the AAA testing pattern 🎉

jandrade · 2024-11-06T19:03:25Z

packages/wonder-blocks-form/src/components/__tests__/text-area.test.tsx

+
+                    // Assert
+                    expect(field).toHaveAttribute("aria-invalid", "true");
+                });


question: I wonder if we need to test what happens in the exact case where the user just focuses on the field and sets instantValidation=true. In theory, the field should be in its default state (aria-invalid=false), so I'm not sure if it is worth testing that case to prevent regressions in the future. wdyt?

Clarification from our discussion offline: This refers to the case where the field is empty, the user focuses on it, and verify that validate is not called when instantValidation=true

As discussed, this case is unrelated to the instantValidation prop since validating on mount depends on if there is a value set in the component! It is covered in these tests:

wonder-blocks/packages/wonder-blocks-form/src/components/__tests__/text-area.test.tsx

Lines 980 to 1012 in d2aeaf8

it("should call the validate function when it is first rendered", async () => {

// Arrange

const validate = jest.fn();

render(

<TextArea

value="text"

onChange={() => {}}

validate={validate}

/>,

defaultOptions,

);

// Act

// Assert

expect(validate).toHaveBeenCalledExactlyOnceWith("text");

});

it("should not call the validate function when it is first rendered if the value is empty", async () => {

// Arrange

const validate = jest.fn();

render(

<TextArea

value=""

onChange={() => {}}

validate={validate}

/>,

defaultOptions,

);

// Act

// Assert

expect(validate).not.toHaveBeenCalled();

});

Let me know if this addresses what you had in mind! :)

thanks for the link, the second test covers the case I was thinking about.

jandrade · 2024-11-06T19:06:13Z

packages/wonder-blocks-form/src/components/__tests__/text-area.test.tsx

+                    await userEvent.type(field, "test");
+                    // Blur will trigger error to be shown
+                    await userEvent.tab();


question: I'm not sure if I'm missing some context here.... how do we signal in the test that this case throws a validation error? Do we need to modify the handleValidate fn to check against test?

The test was checking only the last call on the handleValidate mock since there were other tests making sure it called it with the error (I was trying to avoid multiple assertions!). I updated the assertion though so it checks for all calls so it includes the first call with the error message. Hope that makes it clearer!

This change looks great, thanks!

jandrade · 2024-11-06T19:09:54Z

packages/wonder-blocks-form/src/components/__tests__/text-area.test.tsx

praise: this covers a lot of scenarios, awesome (as usual) 👏

jandrade · 2024-11-06T19:12:52Z

packages/wonder-blocks-form/src/components/text-area.tsx

+                    // If instantValidation is false and there is an error
+                    // message, error needs to be cleared when the user updates
+                    // the value


praise: this context is super helpful!

jandrade · 2024-11-06T19:25:12Z

packages/wonder-blocks-form/src/components/text-area.tsx

+        const handleBlur = (event: React.FocusEvent<HTMLTextAreaElement>) => {
+            // Only handle validation on blur if instantValidation is false
+            if (!instantValidation) {
+                // Handle validation on blur if:
+                // 1. There is a value in the field so a user tabbing through
+                // fields doesn't trigger the error state when the field is left
+                // empty. Or,
+                // 2. The field is required. Tabbing through an empty field that
+                // is required will trigger the error state
+                if (event.target.value || required) {
+                    handleValidation(event.target.value);
+                }
+            }
+
+            if (onBlur) {
+                onBlur(event);
+            }
        };


thought: I know that we discussed this at some point, but maybe related to another prop. I feel that this logic for validation could be abstracted out into a custom hook so other field components could reuse this functionality. Do you think this would be feasible? or do you envision some limitations between form fields? (no changes required for this PR, just food for thought)

Yes good question! I'm going to refactor TextField next so it's a function component, then I'll see how we can share the validation logic between TextArea and TextField. I wanted to work on implementing the validation logic in one of the components first so we can get the functionality working with tests! :)

Sounds good!

marcysutton

What a great PR! So many good tests! I left one suggestion for a test name and a reusable regex utility. This is looking great though, awesome job Bea!

marcysutton · 2024-11-08T00:50:02Z

__docs__/wonder-blocks-form/text-area.stories.tsx

+export const InstantValidation: StoryComponentType = {
+    args: {
+        validate(value: string) {
+            const emailRegex = /^[^@\s]+@[^@\s.]+\.[^@.\s]+$/;


Would this make sense for a test utility so it could be reused?

Yes! I'll have another PR up soon related to shared validation logic for TextField and TextArea so I will update the stories to use a test utility there!

marcysutton · 2024-11-08T00:51:43Z

__docs__/wonder-blocks-form/text-area.stories.tsx

+    parameters: {
+        chromatic: {
+            // Disabling because this doesn't test anything visual.
+            disableSnapshot: true,


Ooh, I didn't know you could do that! Nice.

packages/wonder-blocks-form/src/components/__tests__/text-area.test.tsx

… If instantValidation is false, the error is cleared on change

…e field

…ncludes that info already

… checking the last one

…rror

Co-authored-by: Marcy Sutton-Todd <[email protected]>

## Summary: Refactor TextField to be a function component. This will make it easier to share validation logic from the TextArea instantValidation PR #2355 so that TextField will also have the `instantValidation` prop functionality (I'll be working on this next)! Issue: WB-1781 ## Test plan: Tests pass and TextField looks and functions the same way (`/?path=/docs/packages-form-textfield--docs`) Author: beaesguerra Reviewers: beaesguerra, jandrade, marcysutton, kevinb-khan Required Reviewers: Approved By: jandrade, marcysutton Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot Pull Request URL: #2356

beaesguerra self-assigned this Nov 6, 2024

beaesguerra commented Nov 6, 2024

View reviewed changes

beaesguerra marked this pull request as ready for review November 6, 2024 17:44

khan-actions-bot requested a review from a team November 6, 2024 17:44

jandrade approved these changes Nov 6, 2024

View reviewed changes

beaesguerra requested a review from jandrade November 6, 2024 21:56

jandrade approved these changes Nov 6, 2024

View reviewed changes

beaesguerra mentioned this pull request Nov 6, 2024

Refactor TextField to be a function component #2356

Merged

marcysutton reviewed Nov 8, 2024

View reviewed changes

beaesguerra mentioned this pull request Nov 8, 2024

Add instantValidation prop for TextField & share logic with TextArea #2357

Merged

Base automatically changed from form-error-states to main November 13, 2024 21:04

beaesguerra and others added 8 commits November 13, 2024 14:37

Add instantValidation prop so there is an option to validate on blur.…

911f3d4

… If instantValidation is false, the error is cleared on change

If textarea is required, validate whenever the user tabs away from th…

ff4b525

…e field

add changeset

08dd8ed

update tests

5c5e699

remove redundant description in test cases since the describe block i…

9c89a8d

…ncludes that info already

update test assertion to verify all calls to the mock instead of just…

a2649ea

… checking the last one

set error message to null instead of empty string when clearing the e…

8e1fdbf

…rror

Fix test description typo

bee2c97

Co-authored-by: Marcy Sutton-Todd <[email protected]>

beaesguerra force-pushed the instant-validation-prop branch from 3f056a0 to bee2c97 Compare November 13, 2024 21:38

beaesguerra merged commit 9ed7bd5 into main Nov 13, 2024
14 checks passed

beaesguerra deleted the instant-validation-prop branch November 13, 2024 21:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `instantValidation` prop for TextArea #2355

Add `instantValidation` prop for TextArea #2355

beaesguerra commented Nov 6, 2024 •

edited

Loading

changeset-bot bot commented Nov 6, 2024 •

edited

Loading

github-actions bot commented Nov 6, 2024 •

edited

Loading

github-actions bot commented Nov 6, 2024 •

edited

Loading

beaesguerra Nov 6, 2024

khan-actions-bot commented Nov 6, 2024

github-actions bot commented Nov 6, 2024 •

edited

Loading

jandrade left a comment

jandrade Nov 6, 2024

beaesguerra Nov 6, 2024

jandrade Nov 6, 2024

jandrade Nov 6, 2024 •

edited

Loading

beaesguerra Nov 6, 2024

jandrade Nov 6, 2024

jandrade Nov 6, 2024

beaesguerra Nov 6, 2024

jandrade Nov 6, 2024

jandrade Nov 6, 2024

jandrade Nov 6, 2024

jandrade Nov 6, 2024

beaesguerra Nov 6, 2024

jandrade Nov 6, 2024

marcysutton left a comment

marcysutton Nov 8, 2024

beaesguerra Nov 8, 2024

marcysutton Nov 8, 2024

	it("should call validate each time the value changes if the instantValidation prop is true", async () => {
	it("should call validate each time the value changes", async () => {

	it("should call the validate function when it is first rendered", async () => {
	// Arrange
	const validate = jest.fn();
	render(
	<TextArea
	value="text"
	onChange={() => {}}
	validate={validate}
	/>,
	defaultOptions,
	);
	// Act

	// Assert
	expect(validate).toHaveBeenCalledExactlyOnceWith("text");
	});

	it("should not call the validate function when it is first rendered if the value is empty", async () => {
	// Arrange
	const validate = jest.fn();
	render(
	<TextArea
	value=""
	onChange={() => {}}
	validate={validate}
	/>,
	defaultOptions,
	);
	// Act

	// Assert
	expect(validate).not.toHaveBeenCalled();
	});

Add instantValidation prop for TextArea #2355

Add instantValidation prop for TextArea #2355

Conversation

beaesguerra commented Nov 6, 2024 • edited Loading

Summary:

Test plan:

changeset-bot bot commented Nov 6, 2024 • edited Loading

🦋 Changeset detected

github-actions bot commented Nov 6, 2024 • edited Loading

github-actions bot commented Nov 6, 2024 • edited Loading

A new build was pushed to Chromatic! 🚀

Chromatic results:

Choose a reason for hiding this comment

khan-actions-bot commented Nov 6, 2024

Gerald

github-actions bot commented Nov 6, 2024 • edited Loading

npm Snapshot: Published

jandrade left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jandrade Nov 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcysutton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add `instantValidation` prop for TextArea #2355

Add `instantValidation` prop for TextArea #2355

beaesguerra commented Nov 6, 2024 •

edited

Loading

changeset-bot bot commented Nov 6, 2024 •

edited

Loading

github-actions bot commented Nov 6, 2024 •

edited

Loading

github-actions bot commented Nov 6, 2024 •

edited

Loading

github-actions bot commented Nov 6, 2024 •

edited

Loading

jandrade Nov 6, 2024 •

edited

Loading