From 927fe5a42bc8c3b309942b0c751090f8b3a83c78 Mon Sep 17 00:00:00 2001 From: Kirk Swenson Date: Wed, 25 Feb 2026 22:19:48 -0800 Subject: [PATCH 01/10] fix MobX warning by wrapping observable mutations in runInAction Co-Authored-By: Claude Opus 4.6 --- v3/src/hooks/use-key-states.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/v3/src/hooks/use-key-states.ts b/v3/src/hooks/use-key-states.ts index 0e1bf71dcc..0abe263d46 100644 --- a/v3/src/hooks/use-key-states.ts +++ b/v3/src/hooks/use-key-states.ts @@ -1,4 +1,4 @@ -import { observable } from "mobx" +import { observable, runInAction } from "mobx" import { useEffect } from "react" /** @@ -30,9 +30,10 @@ export const isKeyDown = (key: string) => keysDown.has(key) */ export const useKeyStates = () => { useEffect(() => { - const handleKeyDown = (e: KeyboardEvent) => keysDown.add(e.key) - const handleKeyUp = (e: KeyboardEvent) => keysDown.delete(e.key) - const updateKey = (key: string, isDown: boolean) => isDown ? keysDown.add(key) : keysDown.delete(key) + const handleKeyDown = (e: KeyboardEvent) => runInAction(() => keysDown.add(e.key)) + const handleKeyUp = (e: KeyboardEvent) => runInAction(() => keysDown.delete(e.key)) + const updateKey = (key: string, isDown: boolean) => + runInAction(() => isDown ? keysDown.add(key) : keysDown.delete(key)) const handleMouseEvent = (e: MouseEvent) => { const keys: Record = { Shift: e.shiftKey, Alt: e.altKey, Meta: e.metaKey, Control: e.ctrlKey From cc186c938a62d6e2538208951712acbe1ff6736e Mon Sep 17 00:00:00 2001 From: Kirk Swenson Date: Wed, 25 Feb 2026 22:20:13 -0800 Subject: [PATCH 02/10] CODAP-572: improve accessibility of graph axis/legend attribute menus Add aria-label to MenuButton describing axis orientation and current attribute. Add visible focus indicator and dropdown arrow affordance on axis labels. Scroll focused menu items into view for magnification/zoom users. Add keyboard navigation for collection submenus (ArrowRight opens, ArrowLeft/Escape closes). Handle ArrowRight at MenuList level since Chakra MenuItem with as="div" does not forward onKeyDown to user handlers. Add tests for aria-label, focus scroll, menu rendering, and item selection. Co-Authored-By: Claude Opus 4.6 --- .../axis-or-legend-attribute-menu.scss | 35 +++ .../axis-or-legend-attribute-menu.test.tsx | 219 ++++++++++++++++++ .../axis-or-legend-attribute-menu.tsx | 84 ++++++- v3/src/utilities/translation/lang/en-US.json5 | 4 + 4 files changed, 339 insertions(+), 3 deletions(-) create mode 100644 v3/src/components/axis/components/axis-or-legend-attribute-menu.test.tsx diff --git a/v3/src/components/axis/components/axis-or-legend-attribute-menu.scss b/v3/src/components/axis/components/axis-or-legend-attribute-menu.scss index 9f5436442d..a315a5dc46 100644 --- a/v3/src/components/axis/components/axis-or-legend-attribute-menu.scss +++ b/v3/src/components/axis/components/axis-or-legend-attribute-menu.scss @@ -2,6 +2,41 @@ .attribute-label-menu { touch-action: none; + + button:focus-visible { + outline: 2px solid vars.$focus-outline-color; + outline-offset: 2px; + border-radius: 2px; + } + + .axis-label-dropdown-arrow { + position: absolute; + right: -14px; + top: 50%; + transform: translateY(-50%); + width: 12px; + height: 12px; + pointer-events: none; + opacity: 0; + transition: opacity 0.15s ease; + } + + &:hover .axis-label-dropdown-arrow, + &:focus-within .axis-label-dropdown-arrow { + opacity: 1; + } +} + +// For vertical axes, position the arrow below instead of to the right +.axis-legend-attribute-menu.left, +.axis-legend-attribute-menu.rightCat, +.axis-legend-attribute-menu.rightNumeric { + .attribute-label-menu .axis-label-dropdown-arrow { + right: 50%; + top: auto; + bottom: -14px; + transform: translateX(50%); + } } .collection-menu-button { diff --git a/v3/src/components/axis/components/axis-or-legend-attribute-menu.test.tsx b/v3/src/components/axis/components/axis-or-legend-attribute-menu.test.tsx new file mode 100644 index 0000000000..7d485f90ad --- /dev/null +++ b/v3/src/components/axis/components/axis-or-legend-attribute-menu.test.tsx @@ -0,0 +1,219 @@ +/* eslint-disable testing-library/no-node-access */ +import { 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 = { + 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 = { + 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() + let containerDiv: 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( + + +
+ +
+ + + ) + } + + beforeEach(() => { + containerDiv = document.createElement("div") + document.body.appendChild(containerDiv) + jest.clearAllMocks() + }) + + afterEach(() => { + document.body.removeChild(containerDiv) + }) + + 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) + 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 */ diff --git a/v3/src/components/axis/components/axis-or-legend-attribute-menu.tsx b/v3/src/components/axis/components/axis-or-legend-attribute-menu.tsx index 5ef36123fb..edcb682cc2 100644 --- a/v3/src/components/axis/components/axis-or-legend-attribute-menu.tsx +++ b/v3/src/components/axis/components/axis-or-legend-attribute-menu.tsx @@ -1,7 +1,7 @@ import { clsx } from "clsx" import { observer } from "mobx-react-lite" import { Menu, MenuItem, MenuList, MenuButton, MenuDivider, Portal } from "@chakra-ui/react" -import React, { CSSProperties, useCallback, useRef, useState } from "react" +import React, { CSSProperties, useCallback, useEffect, useRef, useState } from "react" import { useDocumentContainerContext } from "../../../hooks/use-document-container-context" import { useFreeTileLayoutContext } from "../../../hooks/use-free-tile-layout-context" import { IUseDraggableAttribute, useDraggableAttribute } from "../../../hooks/use-drag-drop" @@ -19,6 +19,7 @@ import { GraphPlace } from "../../axis-graph-shared" import { graphPlaceToAttrRole } from "../../data-display/data-display-types" import { useDataConfigurationContext } from "../../data-display/hooks/use-data-configuration-context" +import DropdownArrow from "../../../assets/icons/arrow.svg" import RightArrow from "../../../assets/icons/arrow-right.svg" import "./axis-or-legend-attribute-menu.scss" @@ -61,27 +62,54 @@ interface ICollectionMenuProps { maxMenuHeight: string onCancelPendingHover?: () => void onChangeAttribute: (place: GraphPlace, dataSet: IDataSet, attrId: string) => void + onCloseSubmenu?: () => void + onMenuItemFocus?: React.FocusEventHandler onPointerOver?: React.PointerEventHandler place: GraphPlace } const CollectionMenu = observer(function CollectionMenu({ collectionInfo, containerRef, isAttributeAllowed, isOpen, maxMenuHeight, onCancelPendingHover, - onChangeAttribute, onPointerOver, place + onChangeAttribute, onCloseSubmenu, onMenuItemFocus, onPointerOver, place }: ICollectionMenuProps) { const { collection } = collectionInfo const submenuRef = useRef(null) + const collectionItemRef = useRef(null) const adjustedMaxHeight = useMenuHeightAdjustment({ menuRef: submenuRef, containerRef, isOpen }) const handleSubmenuPointerEnter = () => { onCancelPendingHover?.() } + // When submenu opens, focus the first item + useEffect(() => { + if (isOpen && submenuRef.current) { + requestAnimationFrame(() => { + const firstItem = submenuRef.current?.querySelector('[role="menuitem"]') as HTMLElement + firstItem?.focus() + }) + } + }, [isOpen]) + + // ArrowLeft or Escape in submenu closes it and returns focus to collection item + const handleSubmenuKeyDown = useCallback((e: React.KeyboardEvent) => { + if (e.key === 'ArrowLeft' || e.key === 'Escape') { + e.preventDefault() + e.stopPropagation() + onCloseSubmenu?.() + requestAnimationFrame(() => { + collectionItemRef.current?.focus() + }) + } + }, [onCloseSubmenu]) + return ( <> { + cancelPendingHover() + setOpenCollectionId(collectionId) + }, [cancelPendingHover]) + + // Close the current collection submenu (for keyboard navigation) + const handleCloseSubmenu = useCallback(() => { + cancelPendingHover() + setOpenCollectionId(null) + }, [cancelPendingHover]) + // Enable snapToCursor for vertical (Y) axis labels since the rotated label's bounding box // causes the drag overlay to appear far from the mouse position const isVerticalAxis = ['left', 'rightCat', 'rightNumeric'].includes(place) @@ -241,6 +283,36 @@ export const AxisOrLegendAttributeMenu = observer(function AxisOrLegendAttribute } const clickLabel = place === 'legend' ? `β€”${t("DG.LegendView.attributeTooltip")}` : t("DG.AxisView.labelTooltip", { vars: [orientation]}) + const ariaLabel = place === 'legend' + ? attribute?.name + ? t("DG.AxisView.legendAriaLabel", { vars: [attribute.name] }) + : t("DG.AxisView.emptyLegendAriaLabel") + : attribute?.name + ? t("DG.AxisView.axisAriaLabel", { vars: [orientation, attribute.name] }) + : t("DG.AxisView.emptyAxisAriaLabel", { vars: [orientation] }) + + // Scroll the focused menu item into view for magnification/zoom users + const handleMenuItemFocus = useCallback((e: React.FocusEvent) => { + const focusedElement = e.target as HTMLElement + if (focusedElement.getAttribute('role') === 'menuitem') { + focusedElement.scrollIntoView({ block: 'nearest' }) + } + }, []) + + // Handle ArrowRight on collection items to open submenus. + // Chakra MenuItem with as="div" doesn't forward onKeyDown to the user's handler, + // so we catch the bubbled event at the MenuList level instead. + const handleMainMenuKeyDown = useCallback((e: React.KeyboardEvent) => { + if (e.key === 'ArrowRight') { + const menuItem = (e.target as HTMLElement).closest('[data-collection-id]') + const collectionId = menuItem instanceof HTMLElement ? menuItem.dataset.collectionId : undefined + if (collectionId) { + e.preventDefault() + e.stopPropagation() + handleOpenSubmenu(collectionId) + } + } + }, [handleOpenSubmenu]) const handleChangeAttribute = (_place: GraphPlace, data: IDataSet, _attrId: string) => { onChangeAttribute(_place, data, _attrId) @@ -274,6 +346,8 @@ export const AxisOrLegendAttributeMenu = observer(function AxisOrLegendAttribute maxMenuHeight={maxMenuHeight} onCancelPendingHover={cancelPendingHover} onChangeAttribute={handleChangeAttribute} + onCloseSubmenu={handleCloseSubmenu} + onMenuItemFocus={handleMenuItemFocus} onPointerOver={() => handleCollectionHover(collection.id)} place={place} /> @@ -295,12 +369,16 @@ export const AxisOrLegendAttributeMenu = observer(function AxisOrLegendAttribute
- + {attribute?.name} +