Skip to content

Commit

Permalink
feat: use "informative" and "decorative" as the type of icons
Browse files Browse the repository at this point in the history
Instead of "img" and "presentation". Now that we're using aria-hidden
instead of role="presentation", our "role" prop is decoupled from the
actual HTML attributes anyway. I figured we may as well complete the
decoupling and not even pretend anymore like we're using an HTML/ARIA
role.

The names "informative" and "decorative" also may better indicate their
use cases.
  • Loading branch information
ahuth committed Jul 29, 2021
1 parent 7e9a42e commit e9444b1
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 22 deletions.
4 changes: 2 additions & 2 deletions packages/components/src/Banner/Banner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export const BannerIcon = ({ color }: { color: Color }) => {

return (
<div className={clsx(styles.icon, iconAssets.style)}>
<iconAssets.icon role="img" title={iconAssets.title} />
<iconAssets.icon type="informative" title={iconAssets.title} />
</div>
);
};
Expand All @@ -101,7 +101,7 @@ const DismissButton = ({ color, onDismiss }: DismissButtonProps) => (
onClick={onDismiss}
variant="link"
>
<CloseIcon role="img" size="28px" title="close" />
<CloseIcon type="informative" size="28px" title="close" />
</Button>
);

Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/Button/button.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const linkWithIcon = Template.bind(null);
linkWithIcon.args = {
children: (
<>
Link with icon <CheckCircle role="presentation" />
Link with icon <CheckCircle type="decorative" />
</>
),
color: "success",
Expand All @@ -87,7 +87,7 @@ export const outlineWithIcon = Template.bind(null);
outlineWithIcon.args = {
children: (
<>
Outline with icon <CheckCircle role="presentation" />
Outline with icon <CheckCircle type="decorative" />
</>
),
color: "warning",
Expand Down
14 changes: 7 additions & 7 deletions packages/components/src/SvgIcon/SvgIcon.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ export default {
options: Object.keys(allIcons),
},
},
role: {
type: {
control: {
type: "radio",
options: ["img", "presentation"],
options: ["informative", "decorative"],
},
},
size: {
Expand Down Expand Up @@ -55,7 +55,7 @@ const Template: Story<Args> = ({ icon, color, ...rest }) => {

const defaultArgs = {
icon: Object.keys(allIcons)[0] as keyof typeof allIcons,
role: "presentation" as const,
role: "decorative" as const,
};

export const Small = Template.bind(null);
Expand Down Expand Up @@ -106,11 +106,11 @@ export const InText = ({ icon, ...rest }: Args) => {
return (
<Text size="h1">
The svg icon defaults to the surrounding text size (
<Icon {...rest} role="img" title="icon with 1em line height" />, 1em), but
often looks better with the line height (
<Icon {...rest} type="informative" title="icon with 1em line height" />,
1em), but often looks better with the line height (
<Icon
{...rest}
role="img"
type="informative"
title="icon with 2em line height"
size="2rem"
/>
Expand All @@ -136,7 +136,7 @@ export const AllIcons = () => {
<ul className={styles.allIcons}>
{Object.entries(allIcons).map(([name, Icon]: [string, any]) => (
<li className={styles.iconCell} key={name}>
<Icon block role="presentation" size="2rem" />
<Icon block type="decorative" size="2rem" />
<Text as="span" size="sm">
{name}
</Text>
Expand Down
29 changes: 21 additions & 8 deletions packages/components/src/SvgIcon/SvgIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,33 @@ interface IconPropsBase {
viewBox?: string;
}

interface FunctionalIconProps extends IconPropsBase {
role: "img";
interface InformativeIconProps extends IconPropsBase {
/**
* The role of the icon.
*
* Use "informative" when the icon **_does_** provide additional meaning to other text on the
* page. You'll be required to pass in a title to label the icon.
*/
type: "informative";
title: string;
}

interface PresentationalIconProps extends IconPropsBase {
role: "presentation";
interface DecorativeIconProps extends IconPropsBase {
/**
* The role of the icon.
*
* Use "decorative" when the icon **_does not_** provide any additional context or meaning to
* associated text. Basically the icon is for show and people don't need it to understand what's
* on the page.
*/
type: "decorative";
}

export type SvgIconProps = PresentationalIconProps | FunctionalIconProps;
export type SvgIconProps = DecorativeIconProps | InformativeIconProps;

export type IconProps =
| Omit<PresentationalIconProps, "children">
| Omit<FunctionalIconProps, "children">;
| Omit<DecorativeIconProps, "children">
| Omit<InformativeIconProps, "children">;

interface SvgStyle extends React.CSSProperties {
"--svg-icon-size"?: string;
Expand Down Expand Up @@ -87,7 +100,7 @@ function SvgIcon(props: SvgIconProps) {
xmlns: "http://www.w3.org/2000/svg",
};

if (props.role === "img") {
if (props.type === "informative") {
return (
<svg {...svgCommonProps} role="img">
<title>{props.title}</title>
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/Tag/Tag.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ OutlineVariants.args = {
export const WithIcon = Template.bind(null);
WithIcon.args = {
...defaultArgs,
icon: <WarningIcon key="icon" role="img" title="warning" />,
icon: <WarningIcon key="icon" type="informative" title="warning" />,
};

export const WithLongTextAndIcon = Template.bind(null);
WithLongTextAndIcon.args = {
...defaultArgs,
children: "This tag has a really long text message",
icon: <WarningIcon key="icon" role="img" title="warning" />,
icon: <WarningIcon key="icon" type="informative" title="warning" />,
};
2 changes: 1 addition & 1 deletion packages/components/src/Toast/Toast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const DismissButton = ({ color, onDismiss }: DismissButtonProps) => (
onClick={onDismiss}
variant="link"
>
<CloseIcon role="img" size="32px" title="close" />
<CloseIcon type="informative" size="32px" title="close" />
</Button>
);

Expand Down

0 comments on commit e9444b1

Please sign in to comment.