Skip to content

Commit

Permalink
fix(Icons): pass a11y props to the svg
Browse files Browse the repository at this point in the history
- pass a11y props to the svg element instead of the div container
  - use `splitIconProps` function to do this
  - also refactor focusable and size props to use `splitIconProps`
- fix tests using presentation role and label
  • Loading branch information
zettca committed Aug 18, 2023
1 parent ee184b6 commit c29c45f
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 50 deletions.
24 changes: 9 additions & 15 deletions packages/core/src/components/AppSwitcher/AppSwitcher.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { render } from "@testing-library/react";
import { render, screen } from "@testing-library/react";
import { HvAppSwitcher, HvAppSwitcherProps } from "./AppSwitcher";

describe("<AppSwitcher /> with minimum configuration", () => {
Expand Down Expand Up @@ -33,18 +33,14 @@ describe("<AppSwitcher /> with minimum configuration", () => {
};

it("should render 3 action components", () => {
const { getAllByRole } = render(
<HvAppSwitcher {...mockAppSwitcherProps} />
);
const actions = getAllByRole("listitem");
render(<HvAppSwitcher {...mockAppSwitcherProps} />);
const actions = screen.getAllByRole("listitem");
expect(actions.length).toBe(3);
});

it("should have 2 Info icons rendered", () => {
const { getAllByLabelText } = render(
<HvAppSwitcher {...mockAppSwitcherProps} />
);
const images = getAllByLabelText("Description", { exact: false });
render(<HvAppSwitcher {...mockAppSwitcherProps} />);
const images = screen.getAllByAltText(/Description/i);
expect(images.length).toBe(2);
});

Expand All @@ -57,13 +53,11 @@ describe("<AppSwitcher /> with minimum configuration", () => {
});

it('should have "aria-hidden" for avatars', () => {
const { getAllByRole } = render(
<HvAppSwitcher {...mockAppSwitcherProps} />
);
render(<HvAppSwitcher {...mockAppSwitcherProps} />);

const hiddenAvatar = getAllByRole("listitem")[0].querySelector(
'[aria-hidden="true"]'
);
const hiddenAvatar = screen
.getAllByRole("listitem")[0]
.querySelector('[aria-hidden="true"]');
expect(hiddenAvatar).not.toBeNull();
});
});
4 changes: 2 additions & 2 deletions packages/core/src/components/Avatar/Avatar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ describe("Avatar", () => {
it("renders the icon", () => {
render(
<HvAvatar>
<LogIn aria-label="login" />
<LogIn role="img" aria-label="login" />
</HvAvatar>
);

expect(screen.getByLabelText("login")).toBeInTheDocument();
expect(screen.getByRole("img", { name: "login" })).toBeInTheDocument();
});
});
4 changes: 2 additions & 2 deletions packages/core/src/components/Badge/Badge.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ describe("Badge", () => {
<HvBadge
count={100}
showCount
icon={<Alert role="presentation" aria-label="Alert" />}
icon={<Alert role="img" aria-label="Alert" />}
/>
);
expect(screen.queryByText("99+")).toBeInTheDocument();
expect(screen.getByLabelText("Alert")).toBeInTheDocument();
expect(screen.getByRole("img", { name: "Alert" })).toBeInTheDocument();
});

it("should render correctly with custom label", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,9 @@ import { HvAdornment } from "@core/components";
describe("Adornment", () => {
it("should render the passed icon", () => {
render(
<HvAdornment
icon={<CloseXS role="presentation" aria-label="close icon" />}
/>
<HvAdornment icon={<CloseXS role="img" aria-label="close icon" />} />
);
expect(
screen.getByRole("presentation", { name: "close icon" })
).toBeInTheDocument();
expect(screen.getByRole("img", { name: "close icon" })).toBeInTheDocument();
});

it("should render a button if a 'onClick' is passed", async () => {
Expand Down
26 changes: 15 additions & 11 deletions packages/core/src/components/Kpi/Kpi.test.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { render } from "@testing-library/react";
import { render, screen } from "@testing-library/react";
import { describe, expect, it } from "vitest";
import { HvKpi } from "@core/components";
import { TopXS } from "@hitachivantara/uikit-react-icons";

describe("Kpi", () => {
it("should render all components", () => {
const { getByText, getByTitle } = render(
render(
<HvKpi
indicatorTextVariant="title1"
indicatorUnitTextVariant="title2"
Expand All @@ -16,17 +16,21 @@ describe("Kpi", () => {
comparisonIndicatorInfo: "Comparison indicator info",
}}
visualComparison="Visual comparison"
trendIndicator={<TopXS title="Trend indicator" />}
visualIndicator={<TopXS title="Visual indicator" />}
trendIndicator={<TopXS role="img" title="Trend indicator" />}
visualIndicator={<TopXS role="img" title="Visual indicator" />}
/>
);

expect(getByText("Title")).toBeInTheDocument();
expect(getByText("Indicator")).toBeInTheDocument();
expect(getByText("Unit")).toBeInTheDocument();
expect(getByText("Comparison indicator info")).toBeInTheDocument();
expect(getByText("Visual comparison")).toBeInTheDocument();
expect(getByTitle("Trend indicator")).toBeInTheDocument();
expect(getByTitle("Visual indicator")).toBeInTheDocument();
expect(screen.getByText("Title")).toBeInTheDocument();
expect(screen.getByText("Indicator")).toBeInTheDocument();
expect(screen.getByText("Unit")).toBeInTheDocument();
expect(screen.getByText("Comparison indicator info")).toBeInTheDocument();
expect(screen.getByText("Visual comparison")).toBeInTheDocument();
expect(
screen.getByRole("img", { name: "Trend indicator" })
).toBeInTheDocument();
expect(
screen.getByRole("img", { name: "Visual indicator" })
).toBeInTheDocument();
});
});
46 changes: 45 additions & 1 deletion packages/icons/lib/IconBase.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import styled from "@emotion/styled";
import React, { HTMLAttributes, AllHTMLAttributes, useMemo } from "react";
import { theme } from "@hitachivantara/uikit-styles";
import { isSemantic, isXS } from "./utils";

const getColor = (c: string): string => theme?.colors?.[c] || c;

Expand Down Expand Up @@ -66,6 +67,45 @@ export const getIconColors = (
return colorArray;
};

/** Splits icon `props` between those to be passed to container or svg */
export const splitIconProps = (iconName: string, props: IconBaseProps) => {
const {
role,
title,
iconSize: iconSizeProp,
width,
height,
svgProps,
"aria-label": ariaLabel,
"aria-labelledby": ariaLabelledBy,
"aria-describedby": ariaDescribedBy,
...rest
} = props;
const iconSize = iconSizeProp ?? (isXS(iconName) ? "XS" : "S");
const size = getIconSize(iconSize, isSemantic(iconName), width, height);

const newSvgProps = {
focusable: false,
// pass size props
...size,
// pass a11y props
title,
role,
"aria-label": ariaLabel,
"aria-labelledby": ariaLabelledBy,
"aria-describedby": ariaDescribedBy,
// pass all other `svgProps`
...svgProps,
} satisfies HTMLAttributes<SVGElement>;

const newOtherProps = {
iconSize,
...rest,
} satisfies IconBaseProps;

return [newSvgProps, newOtherProps] as const;
};

export function useIconColor(
palette: string[] = [],
color?: string | string[],
Expand Down Expand Up @@ -154,18 +194,22 @@ export const StyledIconBase = styled("div")(
export const IconBase = ({
children,
palette,
height,
width,
color,
semantic,
inverted = false,
iconSize = "S",
iconName,
style,
...others
}: IconBaseProps & { palette: string[] }) => {
}: IconBaseProps & { palette: string[]; iconName: string }) => {
const colorArray = getIconColors(palette, color, semantic, inverted);
const colorVars = getColorVars(colorArray);

return (
<StyledIconBase
data-name={iconName}
iconSize={iconSize}
style={{ ...colorVars, ...style }}
{...others}
Expand Down
1 change: 1 addition & 0 deletions packages/icons/lib/IconSprite.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export const HvIconSprite = ({

return (
<IconBase
iconName={iconName}
iconSize={iconSize}
color={color}
palette={baseColors}
Expand Down
14 changes: 5 additions & 9 deletions packages/icons/src/utils/converter/generateComponent.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isSemantic, isXS, isSelector } from "../../../lib/utils";
import { isSelector } from "../../../lib/utils";

const replaceColorsWithTheme = (defaultPalette, themePalette) => {
let result = defaultPalette;
Expand Down Expand Up @@ -39,21 +39,17 @@ export const generateComponent = (

return `
import { theme } from "@hitachivantara/uikit-styles";
import { IconBase, IconBaseProps, useIconSize } from "${basePath}/IconBase";
import { IconBase, IconBaseProps, splitIconProps } from "${basePath}/IconBase";
export const ${iconName} = ({
iconSize = "${isXS(iconName) ? "XS" : "S"}",
viewbox = "${defaultSizes.viewBoxRegexp.join(" ")}",
height,
width,
svgProps,
...others
}: IconBaseProps) => {
const size = useIconSize(iconSize, height, width, ${isSemantic(iconName)});
const [svgProps, rest] = splitIconProps("${iconName}", others);
return (
<IconBase iconSize={iconSize} data-name="${iconName}" palette={[${palette}]} {...others}>
${svgOutput.replace("{...other}", "focusable={false} {...svgProps}")}
<IconBase iconName="${iconName}" palette={[${palette}]} {...rest}>
${svgOutput.replace("{...other}", "{...svgProps}")}
</IconBase>
)};
`;
Expand Down
5 changes: 1 addition & 4 deletions packages/icons/src/utils/size.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,5 @@ export const replaceSize = (fileData: string) => {
return fileData
.replace(widthRegexp, ``)
.replace(heightRegexp, ``)
.replace(
viewBoxRegexp,
`viewBox={viewbox} height={size.height} width={size.width}`
);
.replace(viewBoxRegexp, `viewBox={viewbox}`);
};

0 comments on commit c29c45f

Please sign in to comment.