WB-1665: MultiSelect: Don't select disabled items when select all is …

…selected (#2186)

## Summary:

We are currently selecting disabled items when the "Select all" option is
selected. Even that this is not a common use case, it is still a bug in the
sense that it doesn't match how `<select>` works.

In this PR we are trying to match the behavior of `<select>` by not selecting
disabled option items when "Select all" is selected.

Another change is that we are counting the number of available options to
select in the `Select All (n)` label.

Issue: WB-1665

## Test plan:

1. Navigate to /?path=/story/dropdown-multiselect--shortcuts.
2. Open the dropdown and select "Select all".
3. Verify that the disabled items are not selected.


https://github.com/Khan/wonder-blocks/assets/843075/199ae0d9-f59e-4ffe-bc15-fc1da44b9555

Author: jandrade

Reviewers: jeresig

Required Reviewers:

Approved By: jeresig

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ codecov/project, ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Lint (ubuntu-latest, 20.x), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), 🚫 Chromatic - Get results on regular PRs, ✅ Test (ubuntu-latest, 20.x, 2/2), ✅ Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Lint (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ 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), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️  dependabot

Pull Request URL: #2186

.changeset/pink-islands-grin.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/wonder-blocks-dropdown": patch
+---
+
+Don't select disabled items using the "Select all" shortcut

packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx

@@ -14,7 +14,7 @@ import type {Labels} from "../multi-select";
 
 const defaultLabels: Labels = {
     ...builtinLabels,
-    selectAllLabel: (numOptions: any) => `Sellect all (${numOptions})`,
+    selectAllLabel: (numOptions: any) => `Select all (${numOptions})`,
     noneSelected: "Choose",
     someSelected: (numSelectedValues: any) =>
         numSelectedValues > 1 ? `${numSelectedValues} students` : "1 student",
@@ -456,6 +456,47 @@ describe("MultiSelect", () => {
             expect(opener).toHaveTextContent("item 3");
         });
 
+        it("selects all the enabled items when the 'All items' shortcut is selected", () => {
+            // Arrange
+            const ControlledMultiSelect = function (
+                props: Props,
+            ): React.ReactElement {
+                const [selectedValues, setSelectedValues] = React.useState([]);
+                const handleChange = (newValues: any) => {
+                    setSelectedValues(newValues);
+                };
+
+                return (
+                    <MultiSelect
+                        labels={labels}
+                        onChange={handleChange}
+                        opened={props.opened}
+                        onToggle={props.onToggle}
+                        selectedValues={selectedValues}
+                        shortcuts={true}
+                    >
+                        <OptionItem label="item 1" value="1" />
+                        {/* The second option item shouldn't be selectable */}
+                        <OptionItem label="item 2" value="2" disabled={true} />
+                        <OptionItem label="item 3" value="3" />
+                    </MultiSelect>
+                );
+            };
+
+            render(<ControlledMultiSelect />);
+
+            const opener = screen.getByRole("button");
+            userEvent.click(opener);
+
+            // Act
+            // Select all of the items
+            const selectAll = screen.getByRole("option", {name: /Select all/i});
+            userEvent.click(selectAll);
+
+            // Assert
+            expect(opener).toHaveTextContent("All fruits");
+        });
+
         it("selects all the items when the 'All items' shortcut is selected", () => {
             // Arrange
             render(<ControlledComponent shortcuts={true} />);
@@ -558,6 +599,32 @@ describe("MultiSelect", () => {
             expect(screen.getAllByRole("option")).toHaveLength(1);
         });
 
+        it("Allows selecting all the enabled items", () => {
+            // Arrange
+            render(
+                <MultiSelect
+                    onChange={() => {}}
+                    selectedValues={[]}
+                    labels={defaultLabels}
+                    shortcuts={true}
+                >
+                    <OptionItem label="item 1" value="1" />
+                    {/* The second option item shouldn't be selectable */}
+                    <OptionItem label="item 2" value="2" disabled={true} />
+                    <OptionItem label="item 3" value="3" />
+                </MultiSelect>,
+            );
+
+            // Act
+            // open the dropdown menu
+            userEvent.click(screen.getByRole("button"));
+
+            // Assert
+            expect(
+                screen.getByRole("option", {name: "Select all (2)"}),
+            ).toBeInTheDocument();
+        });
+
         it("Hides shortcuts when there are any text in search text input", () => {
             // Arrange
             render(

packages/wonder-blocks-dropdown/src/components/multi-select.tsx

@@ -283,9 +283,12 @@ export default class MultiSelect extends React.Component<Props, State> {
 
     handleSelectAll: () => void = () => {
         const {children, onChange} = this.props;
-        const selected = React.Children.toArray(children)
-            .filter(Boolean)
-            // @ts-expect-error [FEI-5019] - TS2339 - Property 'props' does not exist on type 'ReactChild | ReactFragment | ReactPortal'.
+        const allChildren = React.Children.toArray(
+            children,
+        ) as Array<React.ReactElement>;
+
+        const selected = allChildren
+            .filter((option) => !!option && !option.props.disabled)
             .map((option) => option.props.value);
         onChange(selected);
     };
@@ -298,6 +301,9 @@ export default class MultiSelect extends React.Component<Props, State> {
     getMenuText(children: OptionItemComponentArray): string {
         const {implicitAllEnabled, selectedValues} = this.props;
         const {noneSelected, someSelected, allSelected} = this.state.labels;
+        const numSelectedAll = children.filter(
+            (option) => !option.props.disabled,
+        ).length;
 
         // When implicit all enabled, use `labels.allSelected` when no selection
         // otherwise, use the `labels.noneSelected` value
@@ -327,7 +333,7 @@ export default class MultiSelect extends React.Component<Props, State> {
                 }
 
                 return noSelectionText;
-            case children.length:
+            case numSelectedAll:
                 return allSelected;
             default:
                 return someSelected(selectedValues.length);
@@ -505,7 +511,9 @@ export default class MultiSelect extends React.Component<Props, State> {
         const {noneSelected} = this.state.labels;
 
         const menuText = this.getMenuText(allChildren);
-        const numOptions = allChildren.length;
+        const numOptions = allChildren.filter(
+            (option) => !option.props.disabled,
+        ).length;
 
         const dropdownOpener = opener ? (
             <DropdownOpener
@@ -557,7 +565,9 @@ export default class MultiSelect extends React.Component<Props, State> {
                 React.ReactElement<React.ComponentProps<typeof OptionItem>>
             >
         ).filter(Boolean);
-        const numOptions = allChildren.length;
+        const numEnabledOptions = allChildren.filter(
+            (option) => !option.props.disabled,
+        ).length;
         const filteredItems = this.getMenuItems(allChildren);
         const opener = this.renderOpener(allChildren);
 
@@ -571,7 +581,10 @@ export default class MultiSelect extends React.Component<Props, State> {
                     dropdownStyle,
                 ]}
                 isFilterable={isFilterable}
-                items={[...this.getShortcuts(numOptions), ...filteredItems]}
+                items={[
+                    ...this.getShortcuts(numEnabledOptions),
+                    ...filteredItems,
+                ]}
                 light={light}
                 onOpenChanged={this.handleOpenChanged}
                 open={open}