-
Notifications
You must be signed in to change notification settings - Fork 44
CODAP-572: Improve accessibility of custom select dropdown menus #2436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kswenson
wants to merge
10
commits into
main
Choose a base branch
from
CODAP-572-accessible-axis-dropdowns
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+709
−44
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
927fe5a
fix MobX warning by wrapping observable mutations in runInAction
kswenson cc186c9
CODAP-572: improve accessibility of graph axis/legend attribute menus
kswenson b79bc2e
CODAP-572: fix ARIA issues found in accessibility audit
kswenson ecbc856
CODAP-572: open collection submenu immediately on click and fix arrow…
kswenson afec5b6
CODAP-572: improve accessibility of case table/card attribute header …
kswenson 5b1c3bb
CODAP-572: improve accessibility of toolbar button menus
kswenson efb9756
CODAP-572: improve accessibility of inspector panel menus
kswenson 19f690b
CODAP-572: add unit tests for StdMenuList accessibility
kswenson 8b8bfbd
CODAP-572: fix translation placeholder comment to use %@ syntax
kswenson 9687fb4
CODAP-572: address Copilot review feedback
kswenson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
215 changes: 215 additions & 0 deletions
215
v3/src/components/axis/components/axis-or-legend-attribute-menu.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,215 @@ | ||
| /* eslint-disable testing-library/no-node-access */ | ||
| import { act, render, screen } from "@testing-library/react" | ||
| import { userEvent } from "@testing-library/user-event" | ||
| import React, { createRef } from "react" | ||
| import { AxisOrLegendAttributeMenu } from "./axis-or-legend-attribute-menu" | ||
| import { DocumentContainerContext } from "../../../hooks/use-document-container-context" | ||
| import { InstanceIdContext } from "../../../hooks/use-instance-id-context" | ||
|
|
||
| // Mock hooks that require DOM measurements or DnD context | ||
| jest.mock("../../../hooks/use-drag-drop", () => ({ | ||
| useDraggableAttribute: () => ({ | ||
| attributes: {}, | ||
| listeners: {}, | ||
| setNodeRef: jest.fn() | ||
| }) | ||
| })) | ||
|
|
||
| jest.mock("../../../hooks/use-overlay-bounds", () => ({ | ||
| useOverlayBounds: () => ({ left: 0, top: 0, width: 100, height: 20 }) | ||
| })) | ||
|
|
||
| jest.mock("../../../hooks/use-outside-pointer-down", () => ({ | ||
| useOutsidePointerDown: jest.fn() | ||
| })) | ||
|
|
||
| jest.mock("../../../hooks/use-menu-height-adjustment", () => ({ | ||
| useMenuHeightAdjustment: () => undefined | ||
| })) | ||
|
|
||
| // Mock shared data utilities that need MST tree context | ||
| const mockDataSet = { | ||
| id: "ds1", | ||
| name: "TestData", | ||
| attrFromID: (id: string) => { | ||
| const attrs: Record<string, any> = { | ||
| attr1: { id: "attr1", name: "Height", type: "numeric", description: "" }, | ||
| attr2: { id: "attr2", name: "Weight", type: "numeric", description: "" }, | ||
| attr3: { id: "attr3", name: "Species", type: "categorical", description: "" } | ||
| } | ||
| return attrs[id] || null | ||
| }, | ||
| collections: [ | ||
| { | ||
| id: "col1", | ||
| name: "Cases", | ||
| attributes: [ | ||
| { id: "attr1", name: "Height", type: "numeric" }, | ||
| { id: "attr2", name: "Weight", type: "numeric" }, | ||
| { id: "attr3", name: "Species", type: "categorical" } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
|
|
||
| jest.mock("../../../models/data/collection", () => ({ | ||
| isCollectionModel: () => true | ||
| })) | ||
|
|
||
| jest.mock("../../../models/shared/shared-data-utils", () => ({ | ||
| getDataSets: () => [mockDataSet], | ||
| getMetadataFromDataSet: () => undefined | ||
| })) | ||
|
|
||
| // Mock data configuration context | ||
| const mockDataConfiguration: Record<string, any> = { | ||
| dataset: mockDataSet, | ||
| attributeID: (role: string) => role === "x" ? "attr1" : "", | ||
| attributeType: (role: string) => role === "x" ? "numeric" : "", | ||
| placeCanAcceptAttributeIDDrop: undefined | ||
| } | ||
|
|
||
| jest.mock("../../data-display/hooks/use-data-configuration-context", () => ({ | ||
| useDataConfigurationContext: () => mockDataConfiguration, | ||
| DataConfigurationContext: React.createContext(undefined) | ||
| })) | ||
|
|
||
| // Mock free tile layout context | ||
| jest.mock("../../../hooks/use-free-tile-layout-context", () => ({ | ||
| useFreeTileLayoutContext: () => ({ height: 300 }) | ||
| })) | ||
|
|
||
| describe("AxisOrLegendAttributeMenu", () => { | ||
| const containerRef = createRef<HTMLDivElement>() | ||
|
|
||
| const defaultProps = { | ||
| place: "bottom" as const, | ||
| target: null as SVGGElement | null, | ||
| portal: null as HTMLElement | null, | ||
| layoutBounds: "", | ||
| onChangeAttribute: jest.fn(), | ||
| onRemoveAttribute: jest.fn(), | ||
| onTreatAttributeAs: jest.fn() | ||
| } | ||
|
|
||
| const renderMenu = (props = {}) => { | ||
| return render( | ||
| <DocumentContainerContext.Provider value={containerRef}> | ||
| <InstanceIdContext.Provider value="test-instance"> | ||
| <div ref={containerRef}> | ||
| <AxisOrLegendAttributeMenu {...defaultProps} {...props} /> | ||
| </div> | ||
| </InstanceIdContext.Provider> | ||
| </DocumentContainerContext.Provider> | ||
| ) | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| // Reset mock data configuration to defaults so tests are independent | ||
| mockDataConfiguration.attributeID = (role: string) => role === "x" ? "attr1" : "" | ||
| mockDataConfiguration.attributeType = (role: string) => role === "x" ? "numeric" : "" | ||
| jest.clearAllMocks() | ||
| }) | ||
|
|
||
| describe("aria-label", () => { | ||
| it("sets aria-label describing the axis when an attribute is assigned", () => { | ||
| renderMenu({ place: "bottom" }) | ||
| const button = screen.getByTestId("axis-legend-attribute-button-bottom") | ||
| expect(button).toHaveAttribute("aria-label") | ||
| const label = button.getAttribute("aria-label")! | ||
| expect(label).toContain("horizontal") | ||
| expect(label).toContain("Height") | ||
| }) | ||
|
|
||
| it("sets aria-label for vertical axis", () => { | ||
| mockDataConfiguration.attributeID = (role: string) => role === "y" ? "attr1" : "" | ||
| mockDataConfiguration.attributeType = (role: string) => role === "y" ? "numeric" : "" | ||
| renderMenu({ place: "left" }) | ||
| const button = screen.getByTestId("axis-legend-attribute-button-left") | ||
| const label = button.getAttribute("aria-label")! | ||
| expect(label).toContain("vertical") | ||
| }) | ||
|
|
||
| it("sets aria-label for legend", () => { | ||
| mockDataConfiguration.attributeID = (role: string) => role === "legend" ? "attr3" : "" | ||
| mockDataConfiguration.attributeType = (role: string) => role === "legend" ? "categorical" : "" | ||
| renderMenu({ place: "legend" }) | ||
| const button = screen.getByTestId("axis-legend-attribute-button-legend") | ||
| const label = button.getAttribute("aria-label")! | ||
| expect(label).toContain("legend") | ||
| }) | ||
|
|
||
| it("sets appropriate aria-label when no attribute is assigned", () => { | ||
| mockDataConfiguration.attributeID = () => "" | ||
| mockDataConfiguration.attributeType = () => "" | ||
| renderMenu({ place: "bottom" }) | ||
| const button = screen.getByTestId("axis-legend-attribute-button-bottom") | ||
| const label = button.getAttribute("aria-label")! | ||
| expect(label).toContain("horizontal") | ||
| // Should not contain an attribute name | ||
| expect(label).not.toContain("Height") | ||
| }) | ||
| }) | ||
|
|
||
| describe("dropdown arrow indicator", () => { | ||
| it("renders a dropdown arrow element", () => { | ||
| // SVG imports are mocked as strings in the test environment (fileMock.js), | ||
| // so we verify the arrow is rendered by checking for its aria-hidden attribute | ||
| // in the overlay div structure. | ||
| renderMenu() | ||
| const overlayDiv = screen.getByTestId("attribute-label-menu-bottom") | ||
| // The DropdownArrow is rendered as a child of the overlay div | ||
| expect(overlayDiv).toBeInTheDocument() | ||
| // The button should be inside the overlay | ||
| expect(overlayDiv.querySelector("[data-testid='axis-legend-attribute-button-bottom']")) | ||
| .toBeInTheDocument() | ||
| }) | ||
| }) | ||
|
|
||
| describe("scroll into view on focus", () => { | ||
| it("calls scrollIntoView when a menu item receives focus", async () => { | ||
| renderMenu({ place: "bottom" }) | ||
|
|
||
| // The MenuList has an onFocus handler that calls scrollIntoView. | ||
| // Chakra renders menu items even when the menu is "closed" (just hidden via CSS). | ||
| // We can verify the handler works by directly focusing a menu item. | ||
| const menuItems = screen.getAllByRole("menuitem", { hidden: true }) | ||
| expect(menuItems.length).toBeGreaterThan(0) | ||
| act(() => { menuItems[0].focus() }) | ||
| // scrollIntoView is mocked in setupTests.ts | ||
| expect(Element.prototype.scrollIntoView).toHaveBeenCalledWith({ block: "nearest" }) | ||
| }) | ||
| }) | ||
|
|
||
| describe("menu rendering", () => { | ||
| it("renders the menu list in the DOM", () => { | ||
| renderMenu({ place: "bottom" }) | ||
| const menuList = screen.getByTestId("axis-legend-attribute-menu-list-bottom") | ||
| expect(menuList).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("includes attribute names as menu items", () => { | ||
| renderMenu({ place: "bottom" }) | ||
| // Chakra renders items even when menu is closed (hidden via CSS) | ||
| const menuItems = screen.getAllByRole("menuitem", { hidden: true }) | ||
| const itemTexts = menuItems.map(item => item.textContent) | ||
| expect(itemTexts).toContain("Height") | ||
| expect(itemTexts).toContain("Weight") | ||
| expect(itemTexts).toContain("Species") | ||
| }) | ||
|
|
||
| it("calls onChangeAttribute when a menu item is clicked", async () => { | ||
| const user = userEvent.setup() | ||
| const onChangeAttribute = jest.fn() | ||
| renderMenu({ place: "bottom", onChangeAttribute }) | ||
|
|
||
| // Click the "Weight" menu item directly (it's in the DOM even if visually hidden) | ||
| const menuItems = screen.getAllByRole("menuitem", { hidden: true }) | ||
| const weightItem = menuItems.find(item => item.textContent === "Weight") | ||
| expect(weightItem).toBeDefined() | ||
| await user.click(weightItem!) | ||
| expect(onChangeAttribute).toHaveBeenCalledWith("bottom", mockDataSet, "attr2") | ||
| }) | ||
| }) | ||
| }) | ||
| /* eslint-enable testing-library/no-node-access */ | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.