Skip to content
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

feat(loader): remove progressbar role and add loaderLabel prop #7176

Merged
merged 3 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 1 addition & 1 deletion src/components/confirm/confirm.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ test.describe("should render Confirm component", () => {

const button = getDataElementByValue(page, "confirm");
await expect(button).toHaveAttribute("disabled", "");
const loader = page.getByLabel("Loading");
const loader = page.getByTestId("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
14 changes: 12 additions & 2 deletions src/components/loader/loader-test.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,21 @@ export default {
},
},
variant: {
options: ["default", "colorful"],
options: ["default", "gradient"],
control: {
type: "select",
},
},
loaderLabel: {
control: {
type: "text",
},
},
"aria-label": {
control: {
type: "text",
},
},
},
};

Expand All @@ -61,7 +71,7 @@ export const Default = ({
</Button>
);
}
return <Loader size={size} variant={variant} />;
return <Loader size={size} variant={variant} {...args} />;
};

Default.storyName = "default";
Expand Down
29 changes: 25 additions & 4 deletions src/components/loader/loader.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,45 @@ 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 */
/**
* Specify a custom accessible name for the Loader component
* @deprecated - use `loaderLabel` prop instead
*/
"aria-label"?: string;
/**
* Specify a custom accessible label for the Loader.
* This label is visible to users who have enabled the reduce motion setting in their operating system. It is also 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 +65,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 +84,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.
Parsium marked this conversation as resolved.
Show resolved Hide resolved
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
37 changes: 28 additions & 9 deletions src/components/loader/loader.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,37 @@ export const InsideButton: Story = () => {
mimicLoading();
};
const buttonContent = isLoading ? <Loader isInsideButton /> : "Click me";
const ariaContent = isLoading ? "Loading" : "Click me";

return (
<span aria-live="polite">
Parsium marked this conversation as resolved.
Show resolved Hide resolved
<Button
m={2}
buttonType="primary"
aria-label={ariaContent}
onClick={handleButtonClick}
>
<div aria-live="polite">
<Button m={2} buttonType="primary" onClick={handleButtonClick}>
{buttonContent}
</Button>
</span>
</div>
);
};
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("visually hidden label is set to 'Loading' when neither `aria-label` nor `loaderLabel` are passed", () => {
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
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
Loading