From afd99dafab3a66c110bf57fa3daec9f68a5a3b54 Mon Sep 17 00:00:00 2001 From: shaharzil Date: Sun, 5 May 2024 20:30:19 +0300 Subject: [PATCH 1/2] feat: add a11y props for search and combobox --- .../core/src/components/Combobox/Combobox.tsx | 11 +- .../Combobox/__stories__/combobox.mdx | 2 +- .../Combobox/__stories__/combobox.stories.js | 102 ++++++++++++++++-- .../Combobox/components/ComboboxConstants.ts | 1 + .../ComboboxItems/ComboboxItems.tsx | 10 +- .../core/src/components/Search/Search.tsx | 6 +- .../src/components/Search/Search.types.ts | 8 ++ .../components/Search/__stories__/Search.mdx | 2 +- .../Search/__stories__/Search.stories.tsx | 25 ++++- 9 files changed, 147 insertions(+), 20 deletions(-) diff --git a/packages/core/src/components/Combobox/Combobox.tsx b/packages/core/src/components/Combobox/Combobox.tsx index 93ccf1f661..3628cffe30 100644 --- a/packages/core/src/components/Combobox/Combobox.tsx +++ b/packages/core/src/components/Combobox/Combobox.tsx @@ -17,7 +17,12 @@ import { StickyCategoryHeader } from "./components/StickyCategoryHeader/StickyCa import { useItemsData, useKeyboardNavigation } from "./ComboboxHelpers/ComboboxHelpers"; import { getOptionId } from "./helpers"; import { ElementContent, SubIcon, VibeComponentProps, withStaticProps } from "../../types"; -import { IComboboxCategoryMap, IComboboxItem, IComboboxOption } from "./components/ComboboxConstants"; +import { + IComboboxCategoryMap, + IComboboxItem, + IComboboxOption, + COMBOBOX_LISTBOX_ID +} from "./components/ComboboxConstants"; import styles from "./Combobox.module.scss"; export interface ComboboxProps extends VibeComponentProps { @@ -297,6 +302,9 @@ const Combobox: React.FC & { autoFocus={autoFocus} loading={loading} searchIconName={searchIcon} + ariaExpanded={hasFilter || hasResults} + ariaHasPopup="listbox" + searchResultsContainerId={id ? `${id}-listbox` : COMBOBOX_LISTBOX_ID} /> {stickyCategories && } & { renderOnlyVisibleOptions={renderOnlyVisibleOptions} maxOptionsWithoutScroll={maxOptionsWithoutScroll} visualFocusItemIndex={visualFocusItemIndex} + id={id ? `${id}-listbox` : COMBOBOX_LISTBOX_ID} /> {hasFilter && !hasResults && !loading && renderNoResults()} diff --git a/packages/core/src/components/Combobox/__stories__/combobox.mdx b/packages/core/src/components/Combobox/__stories__/combobox.mdx index 36e82aa152..8bcb786990 100644 --- a/packages/core/src/components/Combobox/__stories__/combobox.mdx +++ b/packages/core/src/components/Combobox/__stories__/combobox.mdx @@ -1,4 +1,4 @@ -import { Canvas, Meta } from "@storybook/blocks"; +import { Meta } from "@storybook/blocks"; import { Link } from "vibe-storybook-components"; import Combobox from "../Combobox"; import DialogContentContainer from "../../DialogContentContainer/DialogContentContainer"; diff --git a/packages/core/src/components/Combobox/__stories__/combobox.stories.js b/packages/core/src/components/Combobox/__stories__/combobox.stories.js index a08efc3374..5005978cfc 100644 --- a/packages/core/src/components/Combobox/__stories__/combobox.stories.js +++ b/packages/core/src/components/Combobox/__stories__/combobox.stories.js @@ -61,6 +61,11 @@ export const Overview = { onClick: () => alert("clicked"), placeholder: "Placeholder text here", clearFilterOnSelection: true + }, + parameters: { + docs: { + liveEdit: { isEnabled: false } + } } }; @@ -88,6 +93,13 @@ export const Default = { }, name: "Default", + parameters: { + docs: { + liveEdit: { + scope: { useMemo } + } + } + }, play: defaultPlaySuite }; @@ -126,7 +138,14 @@ export const ComboboxInsideADialog = { ); }, - name: "Combobox inside a dialog" + name: "Combobox inside a dialog", + parameters: { + docs: { + liveEdit: { + scope: { useMemo } + } + } + } }; export const Sizes = { @@ -172,7 +191,14 @@ export const Sizes = { ); }, - name: "Sizes" + name: "Sizes", + parameters: { + docs: { + liveEdit: { + scope: { useMemo } + } + } + } }; export const WithCategories = { @@ -274,7 +300,14 @@ export const WithCategories = { ); }, - name: "With categories" + name: "With categories", + parameters: { + docs: { + liveEdit: { + scope: { useMemo, StoryDescription } + } + } + } }; export const WithIcons = { @@ -317,7 +350,14 @@ export const WithIcons = { ); }, - name: "With icons" + name: "With icons", + parameters: { + docs: { + liveEdit: { + scope: { useMemo, Wand, ThumbsUp, Time, Update, Upgrade } + } + } + } }; export const WithOptionRenderer = { @@ -353,7 +393,14 @@ export const WithOptionRenderer = { ); }, - name: "With optionRenderer" + name: "With optionRenderer", + parameters: { + docs: { + liveEdit: { + scope: { useMemo, Person } + } + } + } }; export const WithButton = { @@ -399,7 +446,14 @@ export const WithButton = { ); }, - name: "With Button" + name: "With Button", + parameters: { + docs: { + liveEdit: { + scope: { useMemo, Wand, ThumbsUp, Time, Update, Upgrade, Edit } + } + } + } }; export const WithCreation = { @@ -426,7 +480,14 @@ export const WithCreation = { ); }, - name: "With Creation" + name: "With Creation", + parameters: { + docs: { + liveEdit: { + scope: { useState } + } + } + } }; export const WithVirtualizationOptimization = { @@ -643,7 +704,14 @@ export const WithVirtualizationOptimization = { ); }, - name: "With virtualization optimization" + name: "With virtualization optimization", + parameters: { + docs: { + liveEdit: { + scope: { useMemo, StoryDescription } + } + } + } }; export const LoadingState = { @@ -657,7 +725,14 @@ export const LoadingState = { ); }, - name: "Loading state" + name: "Loading state", + parameters: { + docs: { + liveEdit: { + scope: { useMemo } + } + } + } }; export const ComboboxAsPersonPicker = { @@ -735,5 +810,12 @@ export const ComboboxAsPersonPicker = { ); }, - name: "Combobox as person picker" + name: "Combobox as person picker", + parameters: { + docs: { + liveEdit: { + scope: { useMemo, person1, person2, person3, optionRenderer } + } + } + } }; diff --git a/packages/core/src/components/Combobox/components/ComboboxConstants.ts b/packages/core/src/components/Combobox/components/ComboboxConstants.ts index 934f3e5102..dc916702e5 100644 --- a/packages/core/src/components/Combobox/components/ComboboxConstants.ts +++ b/packages/core/src/components/Combobox/components/ComboboxConstants.ts @@ -5,6 +5,7 @@ import { MutableRef } from "preact/hooks"; export const COMBOBOX_DIVIDER_ITEM = "combobox-divider"; export const COMBOBOX_CATEGORY_ITEM = "combobox-category"; export const COMBOBOX_OPTION_ITEM = "combobox-option"; +export const COMBOBOX_LISTBOX_ID = "combobox-listbox"; export enum ComboboxOptionIconType { DEFAULT = "default", diff --git a/packages/core/src/components/Combobox/components/ComboboxItems/ComboboxItems.tsx b/packages/core/src/components/Combobox/components/ComboboxItems/ComboboxItems.tsx index 321f847b5f..109849ecb5 100644 --- a/packages/core/src/components/Combobox/components/ComboboxItems/ComboboxItems.tsx +++ b/packages/core/src/components/Combobox/components/ComboboxItems/ComboboxItems.tsx @@ -27,6 +27,7 @@ export interface ComboboxItemsProps extends IComboboxOptionEvents { maxOptionsWithoutScroll?: number; itemsMap?: Map; stickyCategories?: boolean; + id?: string; } export const ComboboxItems: React.FC = forwardRef( @@ -48,7 +49,8 @@ export const ComboboxItems: React.FC = forwardRef( onActiveCategoryChanged, maxOptionsWithoutScroll, itemsMap, - stickyCategories + stickyCategories, + id }, ref: RefObject ) => { @@ -128,17 +130,19 @@ export const ComboboxItems: React.FC = forwardRef( className={cx(styles.optionsContainer, className)} items={options} itemRenderer={createItemElementRenderer} - role="treegrid" + role="listbox" scrollableClassName={styles.scrollableContainer} onItemsRendered={onItemsRender} style={style} + id={id} /> ); } else { itemsElements = (
diff --git a/packages/core/src/components/Search/Search.tsx b/packages/core/src/components/Search/Search.tsx index 21c8fbdbc6..a0107fda64 100644 --- a/packages/core/src/components/Search/Search.tsx +++ b/packages/core/src/components/Search/Search.tsx @@ -34,6 +34,8 @@ const Search = forwardRef( onFocus, onBlur, className, + ariaExpanded, + ariaHasPopup, id, "data-testid": dataTestId }: SearchProps, @@ -115,8 +117,10 @@ const Search = forwardRef( inputRole={searchResultsContainerId ? "combobox" : undefined} aria-label={inputAriaLabel} aria-busy={loading} - aria-owns={searchResultsContainerId} + aria-controls={ariaExpanded ? searchResultsContainerId : undefined} aria-activedescendant={currentAriaResultId} + aria-haspopup={ariaHasPopup} + aria-expanded={ariaExpanded} /> ); } diff --git a/packages/core/src/components/Search/Search.types.ts b/packages/core/src/components/Search/Search.types.ts index 4d61166a09..785dc50057 100644 --- a/packages/core/src/components/Search/Search.types.ts +++ b/packages/core/src/components/Search/Search.types.ts @@ -53,6 +53,14 @@ export interface SearchProps extends VibeComponentProps { * Aria-label for the search input, important for accessibility. */ inputAriaLabel?: React.AriaAttributes["aria-label"]; + /** + * aria to be set if the popup is open. + */ + ariaExpanded?: React.AriaAttributes["aria-expanded"]; + /** + * aria to be set the sarch result popup's type. + */ + ariaHasPopup?: React.AriaAttributes["aria-haspopup"]; /** * Rate at which search input changes are debounced. */ diff --git a/packages/core/src/components/Search/__stories__/Search.mdx b/packages/core/src/components/Search/__stories__/Search.mdx index fa3c794d2a..c686e03aae 100644 --- a/packages/core/src/components/Search/__stories__/Search.mdx +++ b/packages/core/src/components/Search/__stories__/Search.mdx @@ -1,4 +1,4 @@ -import { Canvas, Meta } from "@storybook/blocks"; +import { Meta } from "@storybook/blocks"; import { StorybookLink, Tip, UsageGuidelines } from "vibe-storybook-components"; import { COMBOBOX, diff --git a/packages/core/src/components/Search/__stories__/Search.stories.tsx b/packages/core/src/components/Search/__stories__/Search.stories.tsx index b40c793a01..02434dd0f2 100644 --- a/packages/core/src/components/Search/__stories__/Search.stories.tsx +++ b/packages/core/src/components/Search/__stories__/Search.stories.tsx @@ -35,7 +35,12 @@ export const Overview: Story = { name: "Overview", args: { placeholder: "Placeholder text here" }, - decorators: [withFixedWidth] + decorators: [withFixedWidth], + parameters: { + docs: { + liveEdit: { isEnabled: false } + } + } }; export const Sizes: Story = { @@ -65,7 +70,14 @@ export const WithAdditionalAction: Story = { /> ), - decorators: [withFixedWidth] + decorators: [withFixedWidth], + parameters: { + docs: { + liveEdit: { + scope: { FilterIcon } + } + } + } }; const options = [ @@ -102,5 +114,12 @@ export const FilterInCombobox: Story = { ), withFixedWidth - ] + ], + parameters: { + docs: { + liveEdit: { + scope: { options } + } + } + } }; From 3aaed5f65592faa946011c493196f289b0554508 Mon Sep 17 00:00:00 2001 From: shaharzil Date: Sun, 5 May 2024 21:11:33 +0300 Subject: [PATCH 2/2] fix tests --- .../core/src/components/Combobox/Combobox.tsx | 2 +- .../combobox-snapshot-tests.test.js.snap | 115 +++++++++++++----- .../Combobox/__tests__/combobox-tests.test.js | 4 +- .../Search/__tests__/Search.test.tsx | 6 +- 4 files changed, 89 insertions(+), 38 deletions(-) diff --git a/packages/core/src/components/Combobox/Combobox.tsx b/packages/core/src/components/Combobox/Combobox.tsx index 3628cffe30..7aef24369a 100644 --- a/packages/core/src/components/Combobox/Combobox.tsx +++ b/packages/core/src/components/Combobox/Combobox.tsx @@ -287,7 +287,7 @@ const Combobox: React.FC & { data-testid={dataTestId || getTestId(ComponentDefaultTestId.COMBOBOX, id)} ellipsis={false} > -
+
@@ -54,7 +56,8 @@ exports[`Combobox renders correctly when disabled 1`] = `
@@ -68,7 +71,6 @@ exports[`Combobox renders correctly with another noResultsMessage 1`] = ` >
@@ -114,7 +119,8 @@ exports[`Combobox renders correctly with another noResultsMessage 1`] = `
@@ -128,7 +134,6 @@ exports[`Combobox renders correctly with autoFocus 1`] = ` >
@@ -174,7 +182,8 @@ exports[`Combobox renders correctly with autoFocus 1`] = `
@@ -188,7 +197,6 @@ exports[`Combobox renders correctly with className 1`] = ` >
@@ -234,7 +245,8 @@ exports[`Combobox renders correctly with className 1`] = `
@@ -248,7 +260,6 @@ exports[`Combobox renders correctly with custom search icon 1`] = ` >
@@ -294,7 +308,8 @@ exports[`Combobox renders correctly with custom search icon 1`] = `
@@ -308,7 +323,6 @@ exports[`Combobox renders correctly with divider 1`] = ` >
@@ -354,7 +372,8 @@ exports[`Combobox renders correctly with divider 1`] = `
@@ -512,7 +534,8 @@ exports[`Combobox renders correctly with divider and colored categories 1`] = `
@@ -680,7 +705,8 @@ exports[`Combobox renders correctly with empty props 1`] = `
@@ -694,7 +720,6 @@ exports[`Combobox renders correctly with id 1`] = ` >
@@ -740,7 +768,8 @@ exports[`Combobox renders correctly with id 1`] = `
@@ -754,7 +783,6 @@ exports[`Combobox renders correctly with loading 1`] = ` >
@@ -828,7 +859,8 @@ exports[`Combobox renders correctly with loading 1`] = `
@@ -842,7 +874,6 @@ exports[`Combobox renders correctly with optionClassName 1`] = ` >
@@ -888,7 +922,8 @@ exports[`Combobox renders correctly with optionClassName 1`] = `
@@ -902,7 +937,6 @@ exports[`Combobox renders correctly with optionLineHeight 1`] = ` >
@@ -948,7 +985,8 @@ exports[`Combobox renders correctly with optionLineHeight 1`] = `
@@ -962,7 +1000,6 @@ exports[`Combobox renders correctly with optionRenderer 1`] = ` >
@@ -1008,7 +1049,8 @@ exports[`Combobox renders correctly with optionRenderer 1`] = `
@@ -1121,7 +1165,8 @@ exports[`Combobox renders correctly with optionsListHeight 1`] = `
@@ -1135,7 +1180,6 @@ exports[`Combobox renders correctly with placeholder 1`] = ` >
@@ -1181,7 +1228,8 @@ exports[`Combobox renders correctly with placeholder 1`] = `
@@ -1195,7 +1243,6 @@ exports[`Combobox renders correctly with size 1`] = ` >
@@ -1241,7 +1291,8 @@ exports[`Combobox renders correctly with size 1`] = `
diff --git a/packages/core/src/components/Combobox/__tests__/combobox-tests.test.js b/packages/core/src/components/Combobox/__tests__/combobox-tests.test.js index 576a249d19..72a863dd80 100644 --- a/packages/core/src/components/Combobox/__tests__/combobox-tests.test.js +++ b/packages/core/src/components/Combobox/__tests__/combobox-tests.test.js @@ -71,8 +71,8 @@ describe("Combobox tests", () => { ); const input = getByTestId("search_combobox-search"); expect(input.value).toBe(defaultFilter); - const treegrid = getByRole("treegrid"); - expect(treegrid.childElementCount).toBe(1); + const listbox = getByRole("listbox"); + expect(listbox.childElementCount).toBe(1); const orangeOption = getByLabelText(mockOptions[0].label); expect(orangeOption).toBeTruthy(); }); diff --git a/packages/core/src/components/Search/__tests__/Search.test.tsx b/packages/core/src/components/Search/__tests__/Search.test.tsx index 7f46cd7d90..15518c437d 100644 --- a/packages/core/src/components/Search/__tests__/Search.test.tsx +++ b/packages/core/src/components/Search/__tests__/Search.test.tsx @@ -118,9 +118,9 @@ describe("Search", () => { expect(getByRole("combobox")).toBeInTheDocument(); }); - it("should set aria-owns when searchResultsContainerId is provided", () => { - const { getByRole } = renderSearch({ searchResultsContainerId: "search-results" }); - expect(getByRole("combobox")).toHaveAttribute("aria-owns", "search-results"); + it("should set aria-controls when searchResultsContainerId is provided and ariaExpanded is true", () => { + const { getByRole } = renderSearch({ searchResultsContainerId: "search-results", ariaExpanded: true }); + expect(getByRole("combobox")).toHaveAttribute("aria-controls", "search-results"); }); it("should set aria-activedescendant when activeDescendant is provided", () => {