Fleet UI: Script action buttons now keyboard accessible (#46720)
This commit is contained in:
@@ -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.
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -25,7 +25,35 @@ export default meta;
|
||||
|
||||
type Story = StoryObj<typeof TooltipTruncatedText>;
|
||||
|
||||
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<typeof TooltipTruncatedText> & { 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 }) => (
|
||||
<div
|
||||
style={{
|
||||
width: containerWidth,
|
||||
padding: "8px",
|
||||
border: "1px dashed #c5c7d1",
|
||||
}}
|
||||
>
|
||||
<TooltipTruncatedText {...args} />
|
||||
</div>
|
||||
),
|
||||
};
|
||||
|
||||
export const UsedInsideDataSet: Story = {
|
||||
decorators: [
|
||||
|
||||
@@ -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 }) => <div>{children}</div>),
|
||||
}));
|
||||
|
||||
// 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(<TooltipTruncatedText value="example" fixedPositionStrategy />);
|
||||
|
||||
expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual(
|
||||
expect.objectContaining({ fixedPositionStrategy: true })
|
||||
);
|
||||
});
|
||||
|
||||
it("defaults fixedPositionStrategy to false when not provided", () => {
|
||||
render(<TooltipTruncatedText value="example" />);
|
||||
|
||||
expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual(
|
||||
expect.objectContaining({ fixedPositionStrategy: false })
|
||||
);
|
||||
});
|
||||
|
||||
it("disables the tooltip when text is not truncated", () => {
|
||||
mockedUseCheckTruncatedElement.mockReturnValue(false);
|
||||
|
||||
render(<TooltipTruncatedText value="short" />);
|
||||
|
||||
expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual(
|
||||
expect.objectContaining({ disableTooltip: true })
|
||||
);
|
||||
});
|
||||
|
||||
it("enables the tooltip when text is truncated", () => {
|
||||
mockedUseCheckTruncatedElement.mockReturnValue(true);
|
||||
|
||||
render(
|
||||
<TooltipTruncatedText value="a very long value that gets truncated" />
|
||||
);
|
||||
|
||||
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(<TooltipTruncatedText value="just-the-value" />);
|
||||
|
||||
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(
|
||||
<TooltipTruncatedText
|
||||
value="display-value"
|
||||
tooltip="custom-tip-content"
|
||||
/>
|
||||
);
|
||||
|
||||
expect(mockedTooltipWrapper.mock.calls[0][0]).toEqual(
|
||||
expect.objectContaining({ tipContent: "custom-tip-content" })
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -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}
|
||||
>
|
||||
<div className={`${baseClass}__text-value`} ref={ref}>
|
||||
{value}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+21
@@ -61,6 +61,27 @@ describe("ScriptListItem", () => {
|
||||
expect(screen.getByText(/Windows/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("labels each action button with the script name for screen readers", () => {
|
||||
render(
|
||||
<ScriptListItem
|
||||
script={MAC_SCRIPT}
|
||||
onDelete={onDelete}
|
||||
onEdit={onEdit}
|
||||
onClickScript={onClickScript}
|
||||
/>
|
||||
);
|
||||
|
||||
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(
|
||||
<ScriptListItem
|
||||
|
||||
+9
-1
@@ -12,6 +12,7 @@ import ListItem from "components/ListItem";
|
||||
import { ISupportedGraphicNames } from "components/ListItem/ListItem";
|
||||
import GitOpsModeTooltipWrapper from "components/GitOpsModeTooltipWrapper";
|
||||
import { HumanTimeDiffWithDateTip } from "components/HumanTimeDiffWithDateTip";
|
||||
import TooltipTruncatedText from "components/TooltipTruncatedText";
|
||||
|
||||
const baseClass = "script-list-item";
|
||||
|
||||
@@ -107,6 +108,7 @@ const ScriptListItem = ({
|
||||
onClick={onClickEdit}
|
||||
className={`${baseClass}__action-button`}
|
||||
variant="icon"
|
||||
ariaLabel={`Edit ${script.name}`}
|
||||
>
|
||||
<Icon name="pencil" />
|
||||
</Button>
|
||||
@@ -116,6 +118,7 @@ const ScriptListItem = ({
|
||||
className={`${baseClass}__action-button`}
|
||||
variant="icon"
|
||||
onClick={onClickDownload}
|
||||
ariaLabel={`Download ${script.name}`}
|
||||
>
|
||||
<Icon name="download" />
|
||||
</Button>
|
||||
@@ -126,6 +129,7 @@ const ScriptListItem = ({
|
||||
onClick={onClickDelete}
|
||||
className={`${baseClass}__action-button`}
|
||||
variant="icon"
|
||||
ariaLabel={`Delete ${script.name}`}
|
||||
>
|
||||
<Icon name="trash" />
|
||||
</Button>
|
||||
@@ -138,7 +142,11 @@ const ScriptListItem = ({
|
||||
<ListItem
|
||||
className={baseClass}
|
||||
graphic={graphicName}
|
||||
title={<Button variant="link">{script.name}</Button>}
|
||||
title={
|
||||
<Button variant="link" className={`${baseClass}__title-button`}>
|
||||
<TooltipTruncatedText value={script.name} fixedPositionStrategy />
|
||||
</Button>
|
||||
}
|
||||
details={
|
||||
<ScriptListItemDetails
|
||||
platform={platform}
|
||||
|
||||
@@ -7,6 +7,36 @@
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
// Constrain the title slot so TooltipTruncatedText can engage its ellipsis
|
||||
// when script names are long. min-width: 0 on the flex children is required
|
||||
// because flex items default to min-width: auto, which refuses to shrink
|
||||
// below content width — without it, __info inherits the long name's width
|
||||
// and bleeds past __main-content's max-width.
|
||||
.list-item__info {
|
||||
min-width: 0;
|
||||
}
|
||||
.list-item__title {
|
||||
display: block;
|
||||
min-width: 0;
|
||||
max-width: 100%;
|
||||
overflow: hidden;
|
||||
}
|
||||
|
||||
&__title-button {
|
||||
max-width: 100%;
|
||||
// children-wrapper is a flex item of the inline-flex Button and needs
|
||||
// min-width: 0 to actually shrink to the parent's max-width. Without it,
|
||||
// overflow: hidden on children-wrapper just visually clips while the
|
||||
// computed width still tracks the content — meaning TooltipTruncatedText's
|
||||
// ref'd __text-value div sees clientWidth == scrollWidth, so
|
||||
// useCheckTruncatedElement returns false and the tooltip never enables.
|
||||
.children-wrapper {
|
||||
max-width: 100%;
|
||||
min-width: 0;
|
||||
overflow: hidden;
|
||||
}
|
||||
}
|
||||
|
||||
&__action-button {
|
||||
width: 40px;
|
||||
height: 40px;
|
||||
|
||||
Reference in New Issue
Block a user