Skip to content

Commit

Permalink
Clear field error state if the field is cleared (#2424)
Browse files Browse the repository at this point in the history
## Summary:
Previously, if a MultiSelect component had shortcuts enabled, if it was in an error state, the "select all" and "select none" options do not clear the error state. Changing the selected values using these shortcuts now clear the error.

Issue: WB-1846

## Test plan:
### MultiSelect
#### Select none
1. Navigate to `?path=/story/packages-dropdown-multiselect--error-from-validation` and confirm that the shortcuts control is enabled
2. Press the "Select none" option on the field that has an error already
3. Verify that the error message is cleared, even after blurring the field (since Jupiter is not selected)

#### Select all
1. Navigate to `?path=/story/packages-dropdown-multiselect--error-from-validation` and confirm that the shortcuts control is enabled
2. Press the "Select all" option on the field that has an error already
3. Verify that the error message is cleared. After blurring the field, the error message should return since Jupiter is selected

Author: beaesguerra

Reviewers: beaesguerra, jandrade

Required Reviewers:

Approved By: jandrade

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ⏹️  [cancelled] Chromatic - Get results on regular PRs, ✅ gerald, ⏹️  [cancelled] Test / Test (ubuntu-latest, 20.x, 2/2), ⏹️  [cancelled] Test / Test (ubuntu-latest, 20.x, 1/2), ⏹️  [cancelled] Lint / Lint (ubuntu-latest, 20.x), ⏹️  [cancelled] Check build sizes (ubuntu-latest, 20.x), ⏹️  [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️  [cancelled] Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏹️  [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ gerald, ⏭️  dependabot

Pull Request URL: #2424
  • Loading branch information
beaesguerra authored Jan 16, 2025
1 parent 48aea76 commit b2df9d3
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/twenty-ducks-double.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-dropdown": patch
---

MultiSelect: Clear error state when "Select none" or "Select all" shortcuts are used
3 changes: 3 additions & 0 deletions __docs__/wonder-blocks-dropdown/multi-select.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,9 @@ export const ErrorFromValidation: StoryComponentType = {
</View>
);
},
args: {
shortcuts: true,
},
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2583,6 +2583,106 @@ describe("MultiSelect", () => {
"true",
);
});

it("should call onValidate with null if values are cleared using the 'select none' shortcut", async () => {
// Arrange
const errorMessage = "Error message";
const onValidate = jest.fn();
const {userEvent} = doRender(
<ControlledMultiSelect
validate={(values) =>
values.includes("1") ? errorMessage : undefined
}
selectedValues={["1"]}
onValidate={onValidate}
shortcuts={true}
/>,
);
await userEvent.click(await screen.findByRole("button")); // Open the dropdown
onValidate.mockClear(); // Clear any calls

// Act
await userEvent.click(await screen.findByText("Select none")); // Select none

// Assert
expect(onValidate).toHaveBeenCalledExactlyOnceWith(null);
});

it("should not be in an error state if values are cleared using the 'select none' shortcut", async () => {
// Arrange
const errorMessage = "Error message";
const {userEvent} = doRender(
<ControlledMultiSelect
validate={(values) =>
values.includes("1") ? errorMessage : undefined
}
selectedValues={["1"]}
shortcuts={true}
/>,
);
await userEvent.click(await screen.findByRole("button")); // Open the dropdown

// Act
await userEvent.click(await screen.findByText("Select none")); // Select none

// Assert
expect(await screen.findByRole("button")).toHaveAttribute(
"aria-invalid",
"false",
);
});

it("should call onValidate with null if values are changed using the 'select all' shortcut", async () => {
// Arrange
const errorMessage = "Error message";
const onValidate = jest.fn();
const {userEvent} = doRender(
<ControlledMultiSelect
validate={(values) =>
values.includes("1") ? errorMessage : undefined
}
selectedValues={["1"]}
onValidate={onValidate}
shortcuts={true}
/>,
);
await userEvent.click(await screen.findByRole("button")); // Open the dropdown
onValidate.mockClear(); // Clear any calls

// Act
await userEvent.click(
await screen.findByText("Select all (2)"),
); // Select all

// Assert
expect(onValidate).toHaveBeenCalledExactlyOnceWith(null);
});

it("should not be in an error state if values are changed using the 'select all' shortcut", async () => {
// Arrange
const errorMessage = "Error message";
const {userEvent} = doRender(
<ControlledMultiSelect
validate={(values) =>
values.includes("1") ? errorMessage : undefined
}
selectedValues={["1"]}
shortcuts={true}
/>,
);
await userEvent.click(await screen.findByRole("button")); // Open the dropdown

// Act
await userEvent.click(
await screen.findByText("Select all (2)"),
); // Select all

// Assert
expect(await screen.findByRole("button")).toHaveAttribute(
"aria-invalid",
"false",
);
});
});

describe("required", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,12 @@ const MultiSelect = (props: Props) => {
.filter((option) => !!option && !option.props.disabled)
.map((option) => option.props.value);
onChange(selected);
onSelectedValuesChangeValidation();
};

const handleSelectNone = () => {
onChange([]);
onSelectedValuesChangeValidation();
};

const getMenuText = (
Expand Down

0 comments on commit b2df9d3

Please sign in to comment.