Skip to content

Commit

Permalink
feat(loader): remove progressbar role and add loaderLabel prop
Browse files Browse the repository at this point in the history
Removes role of progressbar in Loader as this fails WCAG 1.3.1.
Adds loaderLabel prop to provide a visually hidden label for the component.
Deprecates the 'aria-label' prop in favour of the new label prop.

fix #6956
  • Loading branch information
nuria1110 committed Jan 24, 2025
1 parent 82596c2 commit 5e8bb45
Show file tree
Hide file tree
Showing 15 changed files with 131 additions and 52 deletions.
2 changes: 1 addition & 1 deletion src/components/confirm/confirm.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export const Confirm = ({
})}
>
{isLoadingConfirm ? (
<Loader isInsideButton isActive />
<Loader data-role="confirm-loader" isInsideButton isActive />
) : (
confirmLabel || l.confirm.yes()
)}
Expand Down
3 changes: 2 additions & 1 deletion src/components/confirm/confirm.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from "./components.test-pw";
import {
getDataElementByValue,
getDataRoleByValue,
icon,
} from "../../../playwright/components/index";
import {
Expand Down Expand Up @@ -240,7 +241,7 @@ test.describe("should render Confirm component", () => {

const button = getDataElementByValue(page, "confirm");
await expect(button).toHaveAttribute("disabled", "");
const loader = page.getByLabel("Loading");
const loader = getDataRoleByValue(page, "confirm-loader");
await expect(loader).toBeAttached();
});

Expand Down
2 changes: 1 addition & 1 deletion src/components/confirm/confirm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ test("should render disabled confirm button with Loader if isLoadingConfirm is s
);

expect(screen.queryByRole("button", { name: "Yes" })).not.toBeInTheDocument();
expect(screen.getByRole("progressbar", { name: "Loading" })).toBeVisible();
expect(screen.getByTestId("confirm-loader")).toBeVisible();
expect(screen.getByTestId("confirm-button")).toBeDisabled();
});

Expand Down
2 changes: 1 addition & 1 deletion src/components/loader/loader-test.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default {
},
},
variant: {
options: ["default", "colorful"],
options: ["default", "gradient"],
control: {
type: "select",
},
Expand Down
26 changes: 22 additions & 4 deletions src/components/loader/loader.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,42 @@ import StyledLoader from "./loader.style";
import StyledLoaderSquare, {
StyledLoaderSquareProps,
} from "./loader-square.style";
import Typography from "../typography";
import Logger from "../../__internal__/utils/logger";

export interface LoaderProps
extends Omit<StyledLoaderSquareProps, "backgroundColor">,
MarginProps,
TagProps {
/** Toggle between the default variant and gradient variant */
variant?: string;
/** Specify a custom accessible name for the Loader component */
/** [Deprecated] Specify a custom accessible name for the Loader component */
"aria-label"?: string;
/**
* Specify a custom accessible label for the Loader.
* This label will be present if the user has `reduce-motion` enabled and will also be available to assistive technologies.
*/
loaderLabel?: string;
}

let deprecateAriaLabelWarnTriggered = false;

export const Loader = ({
variant = "default",
"aria-label": ariaLabel,
size = "medium",
isInsideButton,
isActive = true,
loaderLabel,
...rest
}: LoaderProps) => {
if (!deprecateAriaLabelWarnTriggered && ariaLabel) {
deprecateAriaLabelWarnTriggered = true;
Logger.deprecate(
"The aria-label prop in Loader is deprecated and will soon be removed, please use the `loaderLabel` prop instead to provide an accessible label.",
);
}

const l = useLocale();

const reduceMotion = !useMediaQuery(
Expand All @@ -45,13 +62,11 @@ export const Loader = ({
// FE-6368 has been raised for the below, changed hex values for design tokens (when added)
return (
<StyledLoader
aria-label={ariaLabel || l.loader.loading()}
role="progressbar"
{...tagComponent("loader", rest)}
{...filterStyledSystemMarginProps(rest)}
>
{reduceMotion ? (
l.loader.loading()
loaderLabel || ariaLabel || l.loader.loading()
) : (
<>
{["#13A038", "#0092DB", "#8F49FE"].map((color) => (
Expand All @@ -66,6 +81,9 @@ export const Loader = ({
{...loaderSquareProps}
/>
))}
<Typography data-role="hidden-label" variant="span" screenReaderOnly>
{loaderLabel || ariaLabel || l.loader.loading()}
</Typography>
</>
)}
</StyledLoader>
Expand Down
12 changes: 8 additions & 4 deletions src/components/loader/loader.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,17 @@ This is an example of the large Loader component. The larger size is only used w
This example shows a `Loader` nested inside of a `Button` component. To ensure that the correct styling is applied to the `Loader` component when it is nested inside of the `Button` component,
please remember to pass the `isInsideButton` prop to the `Loader` component.

This example also demonstrates how to ensure that the `Loader` component is accessible when used inside of the `Button` component.
It is important to wrap the `Button` component in a `span` (use when elements are inline) or `div` (use when elements are in a block) tag and pass the `aria-live` attribute with the value of `polite`. This will ensure that the screen reader will reliabily announce the updates of the button to the user.
<Canvas of={LoaderStories.InsideButton} />

### Conditional Rendering

In most cases, Loaders are conditionally rendered on the page and are then replaced by content.
It is important to wrap the relevant content in a `span` (when elements are inline) or a `div` (when elements are in a block) and pass the `aria-live` attribute with the value of `polite`.

Please note that ARIA live regions provide a means for conveying dynamic updates to users who rely on assistive technologies like screen readers.
These updates in this case include changes to the button's content. By incorporating live regions permanently into the DOM, we can ensure that users are consistently informed of relevant changes as they occur.
By incorporating live regions permanently into the DOM, we can ensure that users are consistently informed of relevant changes as they occur.

<Canvas of={LoaderStories.InsideButton} />
<Canvas of={LoaderStories.ConditionalRendering} />

## Props

Expand Down
9 changes: 0 additions & 9 deletions src/components/loader/loader.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,6 @@ test.describe("check props for Loader component test", () => {
expect(await colorVal(2)).toBe(color);
});

test("should render Loader with aria-label prop", async ({ mount, page }) => {
await mount(<Loader aria-label="playwright-aria" />);

await expect(loader(page, 0).locator("..")).toHaveAttribute(
"aria-label",
"playwright-aria",
);
});

test("should render Loader with isActive prop set to false", async ({
mount,
page,
Expand Down
39 changes: 28 additions & 11 deletions src/components/loader/loader.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,35 @@ export const InsideButton: Story = () => {
mimicLoading();
};
const buttonContent = isLoading ? <Loader isInsideButton /> : "Click me";
const ariaContent = isLoading ? "Loading" : "Click me";

return (
<span aria-live="polite">
<Button
m={2}
buttonType="primary"
aria-label={ariaContent}
onClick={handleButtonClick}
>
{buttonContent}
</Button>
</span>
<Button m={2} buttonType="primary" onClick={handleButtonClick}>
{buttonContent}
</Button>
);
};
InsideButton.storyName = "Inside Button";

export const ConditionalRendering = () => {
const [isLoading, setIsLoading] = useState(false);
const mimicLoading = () => {
setIsLoading(true);
setTimeout(() => {
setIsLoading(false);
}, 5000);
};
const handleButtonClick = () => {
mimicLoading();
};

return (
<div aria-live="polite">
<Button m={2} buttonType="primary" onClick={handleButtonClick}>
Render Loader
</Button>

{isLoading ? <Loader /> : "Content to Load"}
</div>
);
};
ConditionalRendering.storyName = "Conditional Rendering";
66 changes: 55 additions & 11 deletions src/components/loader/loader.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,55 @@ import { render, screen } from "@testing-library/react";
import { testStyledSystemMargin } from "../../__spec_helper__/__internal__/test-utils";
import Loader from ".";
import useMediaQuery from "../../hooks/useMediaQuery";
import Logger from "../../__internal__/utils/logger";

jest.mock("../../hooks/useMediaQuery", () => ({
__esModule: true,
default: jest.fn().mockReturnValue(false),
}));

testStyledSystemMargin(
(props) => <Loader {...props} />,
() => screen.getByRole("progressbar"),
(props) => <Loader data-role="loader" {...props} />,
() => screen.getByTestId("loader"),
);

test("throws a deprecation warning if the 'aria-label' prop is set", () => {
const loggerSpy = jest
.spyOn(Logger, "deprecate")
.mockImplementation(() => {});

render(<Loader aria-label="Still loading" />);

expect(loggerSpy).toHaveBeenCalledWith(
"The aria-label prop in Loader is deprecated and will soon be removed, please use the `loaderLabel` prop instead to provide an accessible label.",
);
expect(loggerSpy).toHaveBeenCalledTimes(1);

loggerSpy.mockRestore();
});

test("when the user disallows animations or their preference cannot be determined, alternative loading text is rendered", () => {
render(<Loader />);

expect(screen.getByText("Loading")).toBeInTheDocument();
expect(screen.getByText("Loading")).toBeVisible();
});

test("when the user disallows animations or their preference cannot be determined, the provided `aria-label` is rendered", () => {
const loggerSpy = jest
.spyOn(Logger, "deprecate")
.mockImplementation(() => {});

render(<Loader aria-label="Still loading" />);

expect(screen.getByText("Still loading")).toBeVisible();

loggerSpy.mockRestore();
});

test("when the user disallows animations or their preference cannot be determined, the provided `loaderLabel` is rendered", () => {
render(<Loader loaderLabel="Still loading" />);

expect(screen.getByText("Still loading")).toBeVisible();
});

describe("when the user allows animations", () => {
Expand All @@ -36,17 +70,27 @@ describe("when the user allows animations", () => {
expect(squares).toHaveLength(3);
});

test("root element has accessible name", () => {
render(<Loader />);
test("root element has default accessible name", () => {
render(<Loader data-role="loader" />);

expect(screen.getByTestId("loader")).toHaveTextContent("Loading");
});

test("should set visually hidden label to provided `aria-label`", () => {
const loggerSpy = jest
.spyOn(Logger, "deprecate")
.mockImplementation(() => {});

render(<Loader data-role="loader" aria-label="Still loading" />);

expect(screen.getByTestId("loader")).toHaveTextContent("Still loading");

expect(screen.getByRole("progressbar")).toHaveAccessibleName("Loading");
loggerSpy.mockRestore();
});

test("when custom `aria-label` is passed, set accessible name to its value", () => {
render(<Loader aria-label="Still loading" />);
test("should set visually hidden label to provided `loaderLabel`", () => {
render(<Loader data-role="loader" loaderLabel="Still loading" />);

expect(screen.getByRole("progressbar")).toHaveAccessibleName(
"Still loading",
);
expect(screen.getByTestId("loader")).toHaveTextContent("Still loading");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ const SelectList = React.forwardRef(

const loader = isLoading ? (
<StyledSelectLoaderContainer key="loader">
<Loader />
<Loader data-role="select-list-loader" />
</StyledSelectLoaderContainer>
) : undefined;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ describe("rendered content", () => {
</SelectListWithInput>,
);

expect(screen.getByRole("progressbar", { name: "Loading" })).toBeVisible();
expect(screen.getByTestId("select-list-loader")).toBeVisible();
});

it("renders options in a table layout when multiColumn prop is true", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ beforeEach(() => {
test("when `loading` is true, the correct Loader styles are applied", () => {
render(<Switch onChange={() => {}} loading />);

const loaderElement = screen.getByRole("progressbar");
const loaderElement = screen.getByTestId("switch-slider-loader");

expect(loaderElement).toBeVisible();
expect(loaderElement).toHaveStyle({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ const SwitchSlider = ({

const sliderContent = (
<SwitchSliderPanel data-role="slider-panel" {...sliderPanelStyleProps}>
{loading ? <Loader {...loaderProps} /> : panelContent}
{loading ? (
<Loader data-role="switch-slider-loader" {...loaderProps} />
) : (
panelContent
)}
</SwitchSliderPanel>
);

Expand Down
2 changes: 1 addition & 1 deletion src/components/switch/__internal__/switch-slider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ test("when `checked` is true, only one panel renders", () => {
test("when `loading` is true, Loader component renders in the first panel", () => {
render(<SwitchSlider loading />);

const loader = screen.getByRole("progressbar");
const loader = screen.getByTestId("switch-slider-loader");

expect(loader).toBeVisible();
});
Expand Down
8 changes: 4 additions & 4 deletions src/components/switch/switch.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
fieldHelpPreview,
tooltipPreview,
getDataElementByValue,
getDataRoleByValue,
} from "../../../playwright/components/index";
import {
assertCssValueIsApproximately,
Expand Down Expand Up @@ -94,10 +95,9 @@ test.describe("Prop tests for Switch component", () => {
if (boolVal) {
await expect(switchInput(page)).toBeDisabled();

await expect(page.getByRole("progressbar")).toHaveAttribute(
"data-component",
"loader",
);
await expect(
getDataRoleByValue(page, "switch-slider-loader"),
).toBeVisible();
} else {
await expect(switchInput(page)).not.toBeDisabled();
}
Expand Down

0 comments on commit 5e8bb45

Please sign in to comment.