diff --git a/changes/46591-script-actions-keyboard b/changes/46591-script-actions-keyboard new file mode 100644 index 0000000000..973021a00b --- /dev/null +++ b/changes/46591-script-actions-keyboard @@ -0,0 +1 @@ +- Fixed Scripts library action buttons (edit, download, delete) being unreachable via keyboard navigation, and added accessible labels so screen readers can distinguish them. diff --git a/frontend/components/ListItem/_styles.scss b/frontend/components/ListItem/_styles.scss index 4cd07980e4..b8911c8531 100644 --- a/frontend/components/ListItem/_styles.scss +++ b/frontend/components/ListItem/_styles.scss @@ -28,5 +28,6 @@ display: flex; justify-content: flex-end; gap: $pad-xsmall; // Padding on buttons + min-width: max-content; // Prevent wrapping on low screen widths } } diff --git a/frontend/components/TooltipTruncatedText/TooltipTruncatedText.stories.tsx b/frontend/components/TooltipTruncatedText/TooltipTruncatedText.stories.tsx index 6c7fae5699..b40310952c 100644 --- a/frontend/components/TooltipTruncatedText/TooltipTruncatedText.stories.tsx +++ b/frontend/components/TooltipTruncatedText/TooltipTruncatedText.stories.tsx @@ -25,7 +25,35 @@ export default meta; type Story = StoryObj; -export const Default: Story = {}; +// Drag the `containerWidth` control in the Storybook addons panel to resize the +// container. The tooltip should only appear when the container is narrow enough +// that the text actually overflows. +export const Default: StoryObj< + React.ComponentProps & { containerWidth: number } +> = { + args: { + containerWidth: 200, + value: "Resize the container to show tooltip only shows on truncation", + }, + argTypes: { + containerWidth: { + control: { type: "range", min: 50, max: 900, step: 10 }, + description: + "Width of the wrapping container in px. Tooltip only shows when the text is truncated.", + }, + }, + render: ({ containerWidth, ...args }) => ( +
+ +
+ ), +}; export const UsedInsideDataSet: Story = { decorators: [ diff --git a/frontend/components/TooltipTruncatedText/TooltipTruncatedText.tests.tsx b/frontend/components/TooltipTruncatedText/TooltipTruncatedText.tests.tsx new file mode 100644 index 0000000000..24670eb0d0 --- /dev/null +++ b/frontend/components/TooltipTruncatedText/TooltipTruncatedText.tests.tsx @@ -0,0 +1,91 @@ +import React from "react"; +import { render } from "@testing-library/react"; + +import TooltipWrapper from "components/TooltipWrapper"; +import { useCheckTruncatedElement } from "hooks/useCheckTruncatedElement"; +import TooltipTruncatedText from "./TooltipTruncatedText"; + +// Mock TooltipWrapper so we can spy on the props TooltipTruncatedText forwards. +// We don't care about TooltipWrapper's internal behavior here — only that +// pass-through props (notably fixedPositionStrategy, disableTooltip, and +// tipContent) reach it correctly. +jest.mock("components/TooltipWrapper", () => ({ + __esModule: true, + default: jest.fn(({ children }) =>
{children}
), +})); + +// Mock useCheckTruncatedElement so we can control isTruncated in tests. +// In jsdom there's no layout computation, so the real hook always returns +// false — mocking lets us assert the disableTooltip wiring in both states. +jest.mock("hooks/useCheckTruncatedElement", () => ({ + useCheckTruncatedElement: jest.fn(), +})); + +const mockedTooltipWrapper = (TooltipWrapper as unknown) as jest.Mock; +const mockedUseCheckTruncatedElement = useCheckTruncatedElement as jest.Mock; + +describe("TooltipTruncatedText", () => { + beforeEach(() => { + mockedTooltipWrapper.mockClear(); + mockedUseCheckTruncatedElement.mockReturnValue(false); + }); + + it("forwards fixedPositionStrategy=true to TooltipWrapper when set", () => { + render(); + + expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual( + expect.objectContaining({ fixedPositionStrategy: true }) + ); + }); + + it("defaults fixedPositionStrategy to false when not provided", () => { + render(); + + expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual( + expect.objectContaining({ fixedPositionStrategy: false }) + ); + }); + + it("disables the tooltip when text is not truncated", () => { + mockedUseCheckTruncatedElement.mockReturnValue(false); + + render(); + + expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual( + expect.objectContaining({ disableTooltip: true }) + ); + }); + + it("enables the tooltip when text is truncated", () => { + mockedUseCheckTruncatedElement.mockReturnValue(true); + + render( + + ); + + expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual( + expect.objectContaining({ disableTooltip: false }) + ); + }); + + it("uses value as the tip content when tooltip prop is not provided", () => { + render(); + + expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual( + expect.objectContaining({ tipContent: "just-the-value" }) + ); + }); + + it("uses tooltip prop as the tip content when provided, overriding value", () => { + render( + + ); + + expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual( + expect.objectContaining({ tipContent: "custom-tip-content" }) + ); + }); +}); diff --git a/frontend/components/TooltipTruncatedText/TooltipTruncatedText.tsx b/frontend/components/TooltipTruncatedText/TooltipTruncatedText.tsx index 06d8b7715a..77d46ee860 100644 --- a/frontend/components/TooltipTruncatedText/TooltipTruncatedText.tsx +++ b/frontend/components/TooltipTruncatedText/TooltipTruncatedText.tsx @@ -12,6 +12,10 @@ interface ITooltipTruncatedTextCellProps { className?: string; tooltipPosition?: "top" | "bottom" | "left" | "right"; isMobileView?: boolean; + /** Pass-through to TooltipWrapper. Set to `true` when the truncated text + * lives inside an `overflow: hidden` ancestor — the default `absolute` + * positioning can misplace the tooltip in that case. */ + fixedPositionStrategy?: boolean; } const baseClass = "tooltip-truncated-text"; @@ -22,6 +26,7 @@ const TooltipTruncatedText = ({ className, tooltipPosition = "top", isMobileView = false, + fixedPositionStrategy = false, }: ITooltipTruncatedTextCellProps): JSX.Element => { const classNames = classnames(baseClass, className); @@ -39,6 +44,7 @@ const TooltipTruncatedText = ({ showArrow tipContent={tooltip ?? value} isMobileView={isMobileView} + fixedPositionStrategy={fixedPositionStrategy} >
{value} diff --git a/frontend/pages/ManageControlsPage/Scripts/_styles.scss b/frontend/pages/ManageControlsPage/Scripts/_styles.scss index cb4949c9d4..34bf8d77cc 100644 --- a/frontend/pages/ManageControlsPage/Scripts/_styles.scss +++ b/frontend/pages/ManageControlsPage/Scripts/_styles.scss @@ -8,7 +8,15 @@ .list-item { &__actions { - display: none; + // opacity/pointer-events (not display:none) so action buttons stay in + // the tab order in both directions. display:none would remove them from + // layout and tab order, breaking Shift+Tab reversal. + display: flex; + justify-content: flex-end; + gap: $pad-medium; + opacity: 0; + pointer-events: none; + transition: opacity 150ms ease-in-out; } } .upload-list__list-item { @@ -16,16 +24,17 @@ padding: 0; .script-list-item { padding: 1rem 1.5rem; - &:hover { - // currently this behavior has only been specified for Script list items + &:hover, + &:focus-within { background-color: $ui-off-white; - cursor: pointer; .list-item__actions { - display: flex; - justify-content: flex-end; - gap: $pad-medium; + opacity: 1; + pointer-events: auto; } } + &:hover { + cursor: pointer; + } } } } diff --git a/frontend/pages/ManageControlsPage/Scripts/components/ScriptListItem/ScriptListItem.tests.tsx b/frontend/pages/ManageControlsPage/Scripts/components/ScriptListItem/ScriptListItem.tests.tsx index 89507714c5..732fff37a7 100644 --- a/frontend/pages/ManageControlsPage/Scripts/components/ScriptListItem/ScriptListItem.tests.tsx +++ b/frontend/pages/ManageControlsPage/Scripts/components/ScriptListItem/ScriptListItem.tests.tsx @@ -61,6 +61,27 @@ describe("ScriptListItem", () => { expect(screen.getByText(/Windows/)).toBeInTheDocument(); }); + it("labels each action button with the script name for screen readers", () => { + render( + + ); + + expect( + screen.getByRole("button", { name: `Edit ${MAC_SCRIPT.name}` }) + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: `Download ${MAC_SCRIPT.name}` }) + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: `Delete ${MAC_SCRIPT.name}` }) + ).toBeInTheDocument(); + }); + it("calls onClickScript when script name is clicked", async () => { const { user } = renderWithSetup( @@ -116,6 +118,7 @@ const ScriptListItem = ({ className={`${baseClass}__action-button`} variant="icon" onClick={onClickDownload} + ariaLabel={`Download ${script.name}`} > @@ -126,6 +129,7 @@ const ScriptListItem = ({ onClick={onClickDelete} className={`${baseClass}__action-button`} variant="icon" + ariaLabel={`Delete ${script.name}`} > @@ -138,7 +142,11 @@ const ScriptListItem = ({ {script.name}} + title={ + + } details={