Filter select accessible label and focus jump fix (#5077)

* update aria-properties to ensure correct label is passed into ListBox

* update focus strategy for Listbox and overlay to be manually controlled

* clean up stories for FilterSelect and future Select

* update future select portal story example

.changeset/red-pillows-begin.md

@@ -0,0 +1,5 @@
+---
+"@kaizen/components": patch
+---
+
+fix focus jump issue for FilterSelect and Future Select and accessibility bugs.

packages/components/src/Filter/FilterSelect/FilterSelect.spec.tsx

@@ -48,7 +48,7 @@ describe("<FilterSelect>", () => {
   it("shows the options initially when isOpen is true", async () => {
     render(<FilterSelectWrapper isOpen />)
     await waitFor(() => {
-      expect(screen.queryByRole("listbox")).toBeVisible()
+      expect(screen.getByRole("listbox")).toBeVisible()
     })
   })
 
@@ -107,7 +107,7 @@ describe("<FilterSelect>", () => {
         render(<FilterSelectWrapper isOpen />)
         expect(screen.queryByRole("listbox")).toBeVisible()
         await waitFor(() => {
-          expect(screen.queryByRole("listbox")).toHaveFocus()
+          expect(screen.getAllByRole("option")[0]).toHaveFocus()
         })
       })
 

packages/components/src/Filter/FilterSelect/FilterSelect.tsx

@@ -79,6 +79,12 @@ export const FilterSelect = <Option extends SelectOption = SelectOption>({
 
   const { buttonProps } = useButton(triggerProps, triggerRef)
 
+  // The id is being remapped because the buttonProps id points to nowhere. This should ideally be refactored but will point the aria attributes tot he right components
+  const renderTriggerButtonProps = {
+    ...buttonProps,
+    "aria-labelledby": undefined,
+    "aria-controls": menuProps.id,
+  }
   return (
     <>
       <HiddenSelect label={label} state={state} triggerRef={triggerRef} />
@@ -90,19 +96,23 @@ export const FilterSelect = <Option extends SelectOption = SelectOption>({
             selectedValue: state.selectedItem?.textValue || undefined,
             label,
             isOpen,
-            ...buttonProps,
+            ...renderTriggerButtonProps,
           })
         }
         onMount={setTriggerRef}
         classNameOverride={classNameOverride}
       >
-        <FilterContents classNameOverride={styles.filterContents}>
-          <SelectProvider<Option> state={state}>
-            <SelectPopoverContents menuProps={menuProps}>
-              {children}
-            </SelectPopoverContents>
-          </SelectProvider>
-        </FilterContents>
+        <>
+          <FilterContents classNameOverride={styles.filterContents}>
+            <SelectProvider<Option> state={state}>
+              <SelectPopoverContents
+                menuProps={{ ...menuProps, "aria-labelledby": buttonProps.id }}
+              >
+                {children}
+              </SelectPopoverContents>
+            </SelectProvider>
+          </FilterContents>
+        </>
       </Filter>
     </>
   )

packages/components/src/Filter/FilterSelect/_docs/FilterSelect.stories.tsx

@@ -1,4 +1,5 @@
 import React, { useState } from "react"
+import { Story } from "@storybook/blocks"
 import { Meta, StoryObj } from "@storybook/react"
 import { fn } from "@storybook/test"
 import { renderTriggerControls } from "~components/Filter/_docs/controls/renderTriggerControls"

packages/components/src/__future__/Select/Select.spec.tsx

@@ -194,11 +194,13 @@ describe("<Select />", () => {
       })
 
       describe("Given the menu is opened", () => {
-        it("focuses the listbox initially", async () => {
-          const { getByRole } = render(<SelectWrapper defaultOpen />)
+        it("focuses on the first item", async () => {
+          const { getByRole, getAllByRole } = render(
+            <SelectWrapper defaultOpen />
+          )
           expect(getByRole("listbox")).toBeVisible()
           await waitFor(() => {
-            expect(getByRole("listbox")).toHaveFocus()
+            expect(getAllByRole("option")[0]).toHaveFocus()
           })
         })
         it("is closed when hits the escape key", async () => {

packages/components/src/__future__/Select/_docs/Select.mdx

@@ -98,8 +98,6 @@ Set `isFullWidth` to `true` to have the Select span the full width of its contai
 
 By default, the Select's popover will attach itself to the `body` of the document using React's `createPortal`.
 
-You can change the default behaviour by providing a `portalContainerId` to attach this to different element in the DOM. This can help to resolve issues that may arise with `z-index` or having a Select in a modal.
+You can change the default behavior by providing a `portalContainerId` to attach this to different element in the DOM. This can help to resolve issues that may arise with `z-index` or having a Select in a modal.
 
 <Canvas of={SelectStories.PortalContainer} />
-
-There is currently a known issue whereby a selected option will cause the page to scroll to the top of the window on open (click on [default to see example](https://cultureamp.design/?path=/docs/components-select-future--docs#portals)). This can be solved by setting a `portalContainerId` to the closest parent of the Select.

packages/components/src/__future__/Select/_docs/Select.stories.tsx

@@ -1,5 +1,6 @@
 import React from "react"
 import { Meta, StoryObj } from "@storybook/react"
+import { ContextModal } from "~components/Modal"
 import { Select } from "../Select"
 import { SelectOption } from "../types"
 import {
@@ -165,25 +166,41 @@ export const FullWidth: Story = {
 export const PortalContainer: Story = {
   render: args => {
     const portalContainerId = "id--portal-container"
+
+    const [isOpen, setIsOpen] = React.useState(false)
+
+    const handleOpen = (): void => setIsOpen(true)
+    const handleClose = (): void => setIsOpen(false)
     return (
       <>
-        <div
-          id={portalContainerId}
-          className="flex gap-24 bg-gray-200 p-12 overflow-hidden h-[200px] relative"
-        >
-          <Select
-            {...args}
-            label="Default"
-            selectedKey="batch-brew"
-            id="id--select-default"
-          />
-          <Select
+        <div className=" h-[500px] mb-24 block bg-gray-100 flex flex-col gap-16 justify-center items-center">
+          Page content
+          <button
+            type="button"
+            className="border border-gray-500"
+            onClick={handleOpen}
+          >
+            Open Modal
+          </button>
+          <ContextModal
             {...args}
-            label="Inner portal"
-            selectedKey="batch-brew"
-            id="id--select-inner"
-            portalContainerId={portalContainerId}
-          />
+            isOpen={isOpen}
+            onConfirm={handleClose}
+            onDismiss={handleClose}
+            title="Select test"
+          >
+            <div
+              className="flex gap-24 bg-gray-200 p-12 h-[500px] relative"
+              id={portalContainerId}
+            >
+              <Select
+                {...args}
+                label="Select within a modal"
+                id="id--select-inner"
+                portalContainerId={portalContainerId}
+              />
+            </div>
+          </ContextModal>
         </div>
       </>
     )

packages/components/src/__future__/Select/subcomponents/ListBox/ListBox.tsx

@@ -1,5 +1,6 @@
-import React, { HTMLAttributes } from "react"
+import React, { HTMLAttributes, Key, useEffect } from "react"
 import { AriaListBoxOptions, useListBox } from "@react-aria/listbox"
+import { SelectState } from "@react-stately/select"
 import classnames from "classnames"
 import { OverrideClassName } from "~components/types/OverrideClassName"
 import { useSelectContext } from "../../context"
@@ -14,6 +15,21 @@ export type SingleListBoxProps<Option extends SelectOption> = OverrideClassName<
   menuProps: AriaListBoxOptions<SelectItem<Option>>
 }
 
+/** A util to retrieve the key of the correct focusable items based of the focus strategy
+ * This is used to determine which element from the collection to focus to on open base on the keyboard event
+ * ie: UpArrow will set the focusStrategy to "last"
+ */
+const getOptionKeyFromCollection = (
+  state: SelectState<SelectItem<any>>
+): Key | null => {
+  if (state.selectedItem) {
+    return state.selectedItem.key
+  } else if (state.focusStrategy === "last") {
+    return state.collection.getLastKey()
+  }
+  return state.collection.getFirstKey()
+}
+
 export const ListBox = <Option extends SelectOption>({
   children,
   menuProps,
@@ -22,13 +38,40 @@ export const ListBox = <Option extends SelectOption>({
 }: SingleListBoxProps<Option>): JSX.Element => {
   const { state } = useSelectContext<Option>()
   const ref = React.useRef<HTMLUListElement>(null)
+  const [isListboxReady, setListboxReady] = React.useState(false)
   const { listBoxProps } = useListBox(
-    { ...menuProps, disallowEmptySelection: true },
+    {
+      ...menuProps,
+      disallowEmptySelection: true,
+      // This is to ensure that the listbox use React Aria's auto focus feature for Listbox, which creates a visual bug
+      autoFocus: false,
+    },
     state,
     ref
   )
 
+  /**
+   * This is a slightly hacky way to ensure the Listbox is aware of its position without using timeout.
+   * This solves the page from refocusing to the top of the DOM when it is opened for the first time with keyboard.
+   */
+  useEffect(() => {
+    setListboxReady(true)
+  }, [])
+
+  useEffect(() => {
+    if (isListboxReady) {
+      const optionKey = getOptionKeyFromCollection(state)
+      const focusToElement = document.querySelector(
+        `[data-key="${optionKey}"]`
+      ) as HTMLElement
+      if (focusToElement) {
+        focusToElement.focus()
+      }
+    }
+  }, [isListboxReady])
+
   return (
+    // eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions
     <ul
       ref={ref}
       className={classnames(styles.listBox, classNameOverride)}

packages/components/src/__future__/Select/subcomponents/Overlay/Overlay.tsx

@@ -36,7 +36,7 @@ export const Overlay = <Option extends SelectOption>({
       {...restProps}
     >
       {/* eslint-disable-next-line jsx-a11y/no-autofocus */}
-      <FocusScope autoFocus restoreFocus>
+      <FocusScope autoFocus={false} restoreFocus>
         <DismissButton onDismiss={state.close} />
         {children}
         <DismissButton onDismiss={state.close} />