Release Multi Select (#4367)

* Fix accessible name for clear all button * Move stories from staging to components * Add changeset * Export MultiSelect from components/index * Update tests after clear all accessible name change * Uncomment tests for clear all on MultiSelectToggle spec * Update selector for single clear button * Adjust wording to Remove all options from {label} * Remove space from name in stories * Move focus to dialog on open * Disable focus lock by default on Popover * Move focus shift out of Popover and into MultiSelect * Adjust test to confirm dialog has focus on open * Adjust Select specs back to using toBeVisible * Rename subcomponent story titles * Fix Select tests
cultureamp · Dec 4, 2023 · 02a6069 · 02a6069
1 parent 1e3066a
commit 02a6069
Show file tree

Hide file tree

Showing 20 changed files with 152 additions and 131 deletions.
diff --git a/.changeset/shiny-bobcats-cry.md b/.changeset/shiny-bobcats-cry.md
@@ -0,0 +1,5 @@
+---
+"@kaizen/components": minor
+---
+
+Release Multi Select component
diff --git a/packages/components/src/MultiSelect/MultiSelect.spec.tsx b/packages/components/src/MultiSelect/MultiSelect.spec.tsx
@@ -212,7 +212,9 @@ describe("<MultiSelect />", () => {
         expect(popover).toBeVisible()
       })
 
-      await user.click(getByRole("button", { name: "Clear all waffles" }))
+      await user.click(
+        getByRole("button", { name: "Remove all options from Jalapeno" })
+      )
 
       await waitFor(() => {
         expect(popover).toBeVisible()
@@ -230,7 +232,8 @@ describe("<MultiSelect />", () => {
           />
         )
         const toggleButton = getByRole("button", { name: "Jalapeno" })
-        expect(toggleButton).toHaveFocus()
+        const dialog = getByRole("dialog")
+        expect(dialog).toHaveFocus()
 
         await user.tab()
         await waitFor(() => {
@@ -263,7 +266,7 @@ describe("<MultiSelect />", () => {
           await user.tab()
           await waitFor(() => {
             expect(
-              getByRole("button", { name: "Clear all waffles" })
+              getByRole("button", { name: "Remove all options from Jalapeno" })
             ).toHaveFocus()
           })
         })
@@ -323,7 +326,9 @@ describe("Removing all options", () => {
     const pancakesOption = getByText("Pancakes")
     const waffleOption = getByText("Waffle")
 
-    const clearAllButton = getByRole("button", { name: "Clear all waffles" })
+    const clearAllButton = getByRole("button", {
+      name: "Remove all options from Jalapeno",
+    })
     await user.click(clearAllButton)
     await waitFor(() => {
       expect(pancakesOption).not.toBeInTheDocument()

diff --git a/packages/components/src/MultiSelect/MultiSelect.tsx b/packages/components/src/MultiSelect/MultiSelect.tsx
@@ -115,10 +115,12 @@ export const MultiSelect = ({
           refs={refs}
           id={`${id}--popover`}
           focusOnProps={{
+            enabled: true,
             onClickOutside,
             onEscapeKey: handleClose,
             shards: [toggleButtonRef],
             noIsolation: true,
+            onActivation: (): void => refs.floating?.current?.focus(),
           }}
           classNameOverride={styles.popover}
         >

diff --git a/packages/components/src/MultiSelect/_docs/MultiSelect.stickersheet.stories.tsx b/packages/components/src/MultiSelect/_docs/MultiSelect.stickersheet.stories.tsx
@@ -11,7 +11,7 @@ import { MultiSelect, MultiSelectProps } from "../index"
 const IS_CHROMATIC = isChromatic()
 
 export default {
-  title: "Staging/Multi Select",
+  title: "Components/MultiSelect",
   parameters: {
     chromatic: { disable: false },
     controls: { disable: true },

diff --git a/packages/components/src/MultiSelect/_docs/MultiSelect.stories.tsx b/packages/components/src/MultiSelect/_docs/MultiSelect.stories.tsx
@@ -3,7 +3,7 @@ import { Meta, StoryObj } from "@storybook/react"
 import { MultiSelect, MultiSelectProps } from "../index"
 
 const meta = {
-  title: "Staging/Multi Select",
+  title: "Components/MultiSelect",
   component: MultiSelect,
   argTypes: {
     selectedValues: {

diff --git a/...components/src/MultiSelect/subcomponents/Checkbox/_docs/Checkbox.stickersheet.stories.tsx b/...components/src/MultiSelect/subcomponents/Checkbox/_docs/Checkbox.stickersheet.stories.tsx
@@ -7,7 +7,7 @@ import {
 import { Checkbox, CheckedStatus } from "../index"
 
 export default {
-  title: "Staging/Multi Select/Checkbox",
+  title: "Components/MultiSelect/Checkbox",
   parameters: {
     chromatic: { disable: false },
     controls: { disable: true },

diff --git a/packages/components/src/MultiSelect/subcomponents/Checkbox/_docs/Checkbox.stories.tsx b/packages/components/src/MultiSelect/subcomponents/Checkbox/_docs/Checkbox.stories.tsx
@@ -3,7 +3,7 @@ import { Meta, StoryObj } from "@storybook/react"
 import { Checkbox, CheckedStatus } from "../index"
 
 const meta = {
-  title: "Staging/Multi Select/Checkbox",
+  title: "Components/MultiSelect/Checkbox",
   component: Checkbox,
   args: {
     checkedStatus: "unchecked",

diff --git a/...ubcomponents/MultiSelectOptionField/_docs/MultiSelectOptionField.stickersheet.stories.tsx b/...ubcomponents/MultiSelectOptionField/_docs/MultiSelectOptionField.stickersheet.stories.tsx
@@ -5,7 +5,7 @@ import { StickerSheet } from "~storybook/components/StickerSheet"
 import { MultiSelectOptionField, MultiSelectOptionFieldProps } from "../index"
 
 const meta = {
-  title: "Staging/Multi Select/MultiSelectOptionField",
+  title: "Components/MultiSelect/MultiSelectOptionField",
   parameters: {
     chromatic: { disable: false },
     controls: { disable: true },

diff --git a/...MultiSelect/subcomponents/MultiSelectOptionField/_docs/MultiSelectOptionField.stories.tsx b/...MultiSelect/subcomponents/MultiSelectOptionField/_docs/MultiSelectOptionField.stories.tsx
@@ -5,7 +5,7 @@ import { CheckboxProps } from "../../Checkbox"
 import { MultiSelectOptionField } from "../index"
 
 const meta = {
-  title: "Staging/Multi Select/MultiSelectOptionField",
+  title: "Components/MultiSelect/MultiSelectOptionField",
   component: MultiSelectOptionField,
   args: {
     id: "id--multi-select-option-field",

diff --git a/...Select/subcomponents/MultiSelectOptions/_docs/MultiSelectOptions.stickersheet.stories.tsx b/...Select/subcomponents/MultiSelectOptions/_docs/MultiSelectOptions.stickersheet.stories.tsx
@@ -7,7 +7,7 @@ import {
 import { MultiSelectOptions, MultiSelectOptionsProps } from "../index"
 
 export default {
-  title: "Staging/Multi Select/MultiSelectOptions",
+  title: "Components/MultiSelect/MultiSelectOptions",
   parameters: {
     chromatic: { disable: false },
     controls: { disable: true },

diff --git a/...nts/src/MultiSelect/subcomponents/MultiSelectOptions/_docs/MultiSelectOptions.stories.tsx b/...nts/src/MultiSelect/subcomponents/MultiSelectOptions/_docs/MultiSelectOptions.stories.tsx
@@ -3,7 +3,7 @@ import { Meta, StoryObj } from "@storybook/react"
 import { MultiSelectOptions, MultiSelectOptionsProps } from "../index"
 
 const meta = {
-  title: "Staging/Multi Select/MultiSelectOptions",
+  title: "Components/MultiSelect/MultiSelectOptions",
   component: MultiSelectOptions,
   args: {
     id: "id--multi-select-options",

diff --git a/...ges/components/src/MultiSelect/subcomponents/MultiSelectToggle/MultiSelectToggle.spec.tsx b/...ges/components/src/MultiSelect/subcomponents/MultiSelectToggle/MultiSelectToggle.spec.tsx
@@ -70,42 +70,39 @@ describe("<MultiSelectToggle />", () => {
   })
 
   describe("Has no selected options", () => {
-    // @todo: re-enable test when Clear All functionality implemented
-    // eslint-disable-next-line jest/no-commented-out-tests
-    //   it("does not show the clear all button", () => {
-    //     const { queryByRole } = render(<MultiSelectToggleWrapper />)
-    //     expect(
-    //       queryByRole("button", { name: "Clear all waffles" })
-    //     ).not.toBeInTheDocument()
-    //   })
+    it("does not show the clear all button", () => {
+      const { queryByRole } = render(<MultiSelectToggleWrapper />)
+      expect(
+        queryByRole("button", { name: "Remove all options from Waffle" })
+      ).not.toBeInTheDocument()
+    })
   })
 
   describe("Has selected options", () => {
-    // @todo: re-enable test when Clear option functionality implemented
-    // eslint-disable-next-line jest/no-commented-out-tests
-    // it("does not call onClick when clearing a single selected item", async () => {
-    //   const { getByRole } = render(
-    //     <MultiSelectToggleWrapper
-    //       selectedOptions={[{ value: "waffle", label: "Waffle" }]}
-    //     />
-    //   )
-    //   await user.click(getByRole("button", { name: "Clear waffle" }))
-    //   await waitFor(() => {
-    //     expect(onClick).not.toHaveBeenCalled()
-    //   })
-    // })
-    // @todo: re-enable test when Clear All functionality implemented
-    // eslint-disable-next-line jest/no-commented-out-tests
-    // it("does not call onClick when clearing all selected items", async () => {
-    //   const { getByRole } = render(
-    //     <MultiSelectToggleWrapper
-    //       selectedOptions={[{ value: "waffle", label: "Waffle" }]}
-    //     />
-    //   )
-    //   await user.click(getByRole("button", { name: "Clear all waffles" }))
-    //   await waitFor(() => {
-    //     expect(onClick).not.toHaveBeenCalled()
-    //   })
-    // })
+    it("does not call onClick when clearing a single selected item", async () => {
+      const { getByRole } = render(
+        <MultiSelectToggleWrapper
+          selectedOptions={[{ value: "waffle", label: "Waffle" }]}
+        />
+      )
+      await user.click(getByRole("button", { name: "Remove option: Waffle" }))
+      await waitFor(() => {
+        expect(onClick).not.toHaveBeenCalled()
+      })
+    })
+
+    it("does not call onClick when clearing all selected items", async () => {
+      const { getByRole } = render(
+        <MultiSelectToggleWrapper
+          selectedOptions={[{ value: "waffle", label: "Waffle" }]}
+        />
+      )
+      await user.click(
+        getByRole("button", { name: "Remove all options from Waffle" })
+      )
+      await waitFor(() => {
+        expect(onClick).not.toHaveBeenCalled()
+      })
+    })
   })
 })
diff --git a/packages/components/src/MultiSelect/subcomponents/MultiSelectToggle/MultiSelectToggle.tsx b/packages/components/src/MultiSelect/subcomponents/MultiSelectToggle/MultiSelectToggle.tsx
@@ -1,4 +1,4 @@
-import React, { HTMLAttributes, forwardRef } from "react"
+import React, { HTMLAttributes, forwardRef, useId } from "react"
 import classnames from "classnames"
 import { ClearButton } from "~components/ClearButton"
 import { FieldMessageProps } from "~components/FieldMessage"
@@ -38,84 +38,89 @@ export const MultiSelectToggle = forwardRef<
       ...restProps
     },
     ref
-  ): JSX.Element => (
-    <>
-      {/*
-       * The inner <button> is the actual semantic element, and the onClick here
-       * is just a quality-of-life implementation for mouse users.
-       */}
-      {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
-      <div
-        className={classnames(
-          styles.multiSelectToggle,
-          classNameOverride,
-          status && styles[status]
-        )}
-        onClick={onClick}
-        {...restProps}
-      >
-        <button
-          ref={ref}
-          className={styles.toggleButton}
-          aria-labelledby={ariaLabelledBy}
-          aria-describedby={ariaDescribedBy}
-          aria-controls={ariaControls}
-          aria-expanded={isOpen}
-          aria-haspopup="dialog"
-          type="button"
-          onClick={e => {
-            e.stopPropagation()
-            onClick(e)
-          }}
-        >
-          {isOpen ? (
-            <ChevronUpIcon role="presentation" />
-          ) : (
-            <ChevronDownIcon role="presentation" />
-          )}
-        </button>
-
+  ): JSX.Element => {
+    const clearAllId = useId()
+    return (
+      <>
+        {/*
+         * The inner <button> is the actual semantic element, and the onClick here
+         * is just a quality-of-life implementation for mouse users.
+         */}
+        {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
         <div
           className={classnames(
-            styles.selectedItemsContainer,
-            selectedOptions.length && styles.hasSelectedItems
+            styles.multiSelectToggle,
+            classNameOverride,
+            status && styles[status]
           )}
+          onClick={onClick}
+          {...restProps}
         >
-          {selectedOptions.length > 0 && (
-            <>
-              {/* list-style: none removes role="list" in Safari */}
-              {/* eslint-disable-next-line jsx-a11y/no-redundant-roles */}
-              <ul role="list" className={styles.selectedItems}>
-                {selectedOptions.map(({ label, value }) => (
-                  // This stops the underlying toggle collapsing the popover when interactive with Tags
-                  // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events
-                  <li key={value} onClick={e => e.stopPropagation()}>
-                    <RemovableTag
-                      removeButtonProps={{
-                        ariaLabel: `Remove option: ${label}`,
-                        onClick: () => onRemoveOption(value),
-                      }}
-                    >
-                      {label}
-                    </RemovableTag>
-                  </li>
-                ))}
-              </ul>
+          <button
+            ref={ref}
+            className={styles.toggleButton}
+            aria-labelledby={ariaLabelledBy}
+            aria-describedby={ariaDescribedBy}
+            aria-controls={ariaControls}
+            aria-expanded={isOpen}
+            aria-haspopup="dialog"
+            type="button"
+            onClick={e => {
+              e.stopPropagation()
+              onClick(e)
+            }}
+          >
+            {isOpen ? (
+              <ChevronUpIcon role="presentation" />
+            ) : (
+              <ChevronDownIcon role="presentation" />
+            )}
+          </button>
 
-              <ClearButton
-                aria-label="Clear all waffles"
-                classNameOverride={styles.clearAllButton}
-                onClick={e => {
-                  e.stopPropagation()
-                  onRemoveAllOptions()
-                }}
-              />
-            </>
-          )}
+          <div
+            className={classnames(
+              styles.selectedItemsContainer,
+              selectedOptions.length && styles.hasSelectedItems
+            )}
+          >
+            {selectedOptions.length > 0 && (
+              <>
+                {/* list-style: none removes role="list" in Safari */}
+                {/* eslint-disable-next-line jsx-a11y/no-redundant-roles */}
+                <ul role="list" className={styles.selectedItems}>
+                  {selectedOptions.map(({ label, value }) => (
+                    // This stops the underlying toggle collapsing the popover when interactive with Tags
+                    // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/click-events-have-key-events
+                    <li key={value} onClick={e => e.stopPropagation()}>
+                      <RemovableTag
+                        removeButtonProps={{
+                          ariaLabel: `Remove option: ${label}`,
+                          onClick: () => onRemoveOption(value),
+                        }}
+                      >
+                        {label}
+                      </RemovableTag>
+                    </li>
+                  ))}
+                </ul>
+
+                <ClearButton
+                  id={clearAllId}
+                  aria-label="Remove all options from "
+                  aria-labelledby={`${clearAllId} ${ariaLabelledBy}`}
+                  classNameOverride={styles.clearAllButton}
+                  onClick={e => {
+                    e.stopPropagation()
+                    onRemoveAllOptions()
+                  }}
+                />
+              </>
+            )}
+          </div>
         </div>
-      </div>
-    </>
-  )
+      </>
+    )
+  }
 )
 
 MultiSelectToggle.displayName = "MultiSelectToggle"
diff --git a/...tiSelect/subcomponents/MultiSelectToggle/_docs/MultiSelectToggle.stickersheet.stories.tsx b/...tiSelect/subcomponents/MultiSelectToggle/_docs/MultiSelectToggle.stickersheet.stories.tsx
@@ -7,7 +7,7 @@ import {
 import { MultiSelectToggle, MultiSelectToggleProps } from "../index"
 
 export default {
-  title: "Staging/Multi Select/MultiSelectToggle",
+  title: "Components/MultiSelect/MultiSelectToggle",
   parameters: {
     chromatic: { disable: false },
     controls: { disable: true },

diff --git a/packages/components/src/MultiSelect/subcomponents/Popover/Popover.module.scss b/packages/components/src/MultiSelect/subcomponents/Popover/Popover.module.scss
@@ -5,7 +5,14 @@
 .popover {
   box-sizing: border-box;
   box-shadow: $shadow-large-box-shadow;
+  border: $border-focus-ring-border-width $border-focus-ring-border-style
+    transparent;
   border-radius: $border-solid-border-radius;
   background: $color-white;
   overflow: auto;
+
+  &:focus {
+    outline: none;
+    border-color: $color-blue-500;
+  }
 }