Revert "Filter select accessible label and focus jump fix" (#5111)

* Revert "Filter select accessible label and focus jump fix (#5077)" This reverts commit c036342. * Changeset
cultureamp · Sep 30, 2024 · 91ff2cb · 91ff2cb
1 parent da4e513
commit 91ff2cb
Show file tree

Hide file tree

Showing 9 changed files with 40 additions and 106 deletions.
diff --git a/.changeset/khaki-bulldogs-end.md b/.changeset/khaki-bulldogs-end.md
@@ -0,0 +1,5 @@
+---
+"@kaizen/components": patch
+---
+
+Revert "Filter select accessible label and focus jump fix" released in 1.64.5
diff --git a/packages/components/src/Filter/FilterSelect/FilterSelect.spec.tsx b/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.getByRole("listbox")).toBeVisible()
+      expect(screen.queryByRole("listbox")).toBeVisible()
     })
   })
 
@@ -107,7 +107,7 @@ describe("<FilterSelect>", () => {
         render(<FilterSelectWrapper isOpen />)
         expect(screen.queryByRole("listbox")).toBeVisible()
         await waitFor(() => {
-          expect(screen.getAllByRole("option")[0]).toHaveFocus()
+          expect(screen.queryByRole("listbox")).toHaveFocus()
         })
       })
 

diff --git a/packages/components/src/Filter/FilterSelect/FilterSelect.tsx b/packages/components/src/Filter/FilterSelect/FilterSelect.tsx
@@ -79,12 +79,6 @@ 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} />
@@ -96,23 +90,19 @@ export const FilterSelect = <Option extends SelectOption = SelectOption>({
             selectedValue: state.selectedItem?.textValue || undefined,
             label,
             isOpen,
-            ...renderTriggerButtonProps,
+            ...buttonProps,
           })
         }
         onMount={setTriggerRef}
         classNameOverride={classNameOverride}
       >
-        <>
-          <FilterContents classNameOverride={styles.filterContents}>
-            <SelectProvider<Option> state={state}>
-              <SelectPopoverContents
-                menuProps={{ ...menuProps, "aria-labelledby": buttonProps.id }}
-              >
-                {children}
-              </SelectPopoverContents>
-            </SelectProvider>
-          </FilterContents>
-        </>
+        <FilterContents classNameOverride={styles.filterContents}>
+          <SelectProvider<Option> state={state}>
+            <SelectPopoverContents menuProps={menuProps}>
+              {children}
+            </SelectPopoverContents>
+          </SelectProvider>
+        </FilterContents>
       </Filter>
     </>
   )

diff --git a/packages/components/src/Filter/FilterSelect/_docs/FilterSelect.stories.tsx b/packages/components/src/Filter/FilterSelect/_docs/FilterSelect.stories.tsx
@@ -1,5 +1,4 @@
 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"

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

diff --git a/packages/components/src/__future__/Select/_docs/Select.mdx b/packages/components/src/__future__/Select/_docs/Select.mdx
@@ -98,6 +98,8 @@ 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 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.
+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.
 
 <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.
diff --git a/packages/components/src/__future__/Select/_docs/Select.stories.tsx b/packages/components/src/__future__/Select/_docs/Select.stories.tsx
@@ -1,6 +1,5 @@
 import React from "react"
 import { Meta, StoryObj } from "@storybook/react"
-import { ContextModal } from "~components/Modal"
 import { Select } from "../Select"
 import { SelectOption } from "../types"
 import {
@@ -166,41 +165,25 @@ 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 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
+        <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
             {...args}
-            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>
+            label="Inner portal"
+            selectedKey="batch-brew"
+            id="id--select-inner"
+            portalContainerId={portalContainerId}
+          />
         </div>
       </>
     )

diff --git a/packages/components/src/__future__/Select/subcomponents/ListBox/ListBox.tsx b/packages/components/src/__future__/Select/subcomponents/ListBox/ListBox.tsx
@@ -1,6 +1,5 @@
-import React, { HTMLAttributes, Key, useEffect } from "react"
+import React, { HTMLAttributes } 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"
@@ -15,21 +14,6 @@ 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,
@@ -38,40 +22,13 @@ 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,
-      // This is to ensure that the listbox use React Aria's auto focus feature for Listbox, which creates a visual bug
-      autoFocus: false,
-    },
+    { ...menuProps, disallowEmptySelection: true },
     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)}

diff --git a/packages/components/src/__future__/Select/subcomponents/Overlay/Overlay.tsx b/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={false} restoreFocus>
+      <FocusScope autoFocus restoreFocus>
         <DismissButton onDismiss={state.close} />
         {children}
         <DismissButton onDismiss={state.close} />