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

Octicon remediation: aria-label #4811

Merged
merged 10 commits into from
Aug 20, 2024
5 changes: 5 additions & 0 deletions .changeset/cool-lobsters-nail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Octicon: Add aria-label to the Icon instead of it's container
15 changes: 10 additions & 5 deletions packages/react/src/CircleBadge/CircleBadge.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,21 @@ import type {Meta, StoryFn} from '@storybook/react'
import CircleBadge from './CircleBadge'
import {ZapIcon} from '@primer/octicons-react'

export default {
const meta: Meta<typeof CircleBadge> = {
title: 'Components/CircleBadge',
component: CircleBadge,
} as Meta<typeof CircleBadge>
}
export default meta

export const Default = () => (
siddharthkp marked this conversation as resolved.
Show resolved Hide resolved
<CircleBadge>
<CircleBadge.Icon icon={ZapIcon} />
<CircleBadge.Icon icon={ZapIcon} aria-label="User badge" />
</CircleBadge>
)

export const Playground: StoryFn<typeof CircleBadge> = args => (
export const Playground: StoryFn<typeof CircleBadge> = ({'CircleBadge.Icon aria-label': iconAriaLabel, args}) => (
<CircleBadge {...args}>
<CircleBadge.Icon icon={ZapIcon} />
<CircleBadge.Icon icon={ZapIcon} aria-label={iconAriaLabel} />
</CircleBadge>
)

Expand All @@ -25,6 +26,7 @@ Playground.args = {
size: null,
inline: false,
as: 'div',
'CircleBadge.Icon aria-label': undefined,
}

Playground.argTypes = {
Expand All @@ -44,4 +46,7 @@ Playground.argTypes = {
type: 'boolean',
},
},
'CircleBadge.Icon aria-label': {
type: 'string',
},
}
55 changes: 25 additions & 30 deletions packages/react/src/CircleOcticon/CircleOcticon.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,59 +1,54 @@
import React from 'react'
import type {Meta, StoryFn} from '@storybook/react'
import CircleOcticon from './CircleOcticon'
import type {CircleOcticonProps} from './CircleOcticon'
import {CheckIcon} from '@primer/octicons-react'
// eslint-disable-next-line import/no-namespace
import * as Icons from '@primer/octicons-react'

export default {
const meta: Meta<typeof CircleOcticon> = {
title: 'Components/CircleOcticon',
component: CircleOcticon,
} as Meta<typeof CircleOcticon>
}
export default meta

export const Default = () => (
<CircleOcticon icon={CheckIcon} size={32} sx={{backgroundColor: 'success.emphasis', color: 'fg.onEmphasis'}} />
<CircleOcticon
icon={CheckIcon}
size={32}
sx={{backgroundColor: 'success.emphasis', color: 'fg.onEmphasis'}}
aria-label="Changes approved"
/>
)

export const Playground: StoryFn<typeof CircleOcticon> = args => <CircleOcticon {...args} />
type PlaygroundTypes = Omit<CircleOcticonProps, 'icon'> & {icon: keyof typeof Icons}
export const Playground: StoryFn<PlaygroundTypes> = ({icon: iconName, 'aria-label': ariaLabel, ...args}) => (
<CircleOcticon icon={Icons[iconName]} aria-label={ariaLabel ? ariaLabel : undefined} {...args} />
)

Playground.args = {
icon: CheckIcon,
size: 32,
icon: 'CheckIcon',
'aria-label': undefined,
sx: {backgroundColor: 'success.emphasis', color: 'fg.onEmphasis'},
}

Playground.argTypes = {
icon: {
controls: false,
table: {
disable: true,
},
},
size: {
controls: {
type: 'number',
},
},
sx: {
controls: false,
table: {
disable: true,
},
},
as: {
controls: false,
table: {
disable: true,
icon: {
control: {
type: 'select',
},
options: Object.keys(Icons),
},
ref: {
controls: false,
table: {
disable: true,
},
'aria-label': {
type: 'string',
},
theme: {
sx: {
controls: false,
table: {
disable: true,
},
},
}
4 changes: 2 additions & 2 deletions packages/react/src/CircleOcticon/CircleOcticon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export type CircleOcticonProps = {
} & BoxProps

function CircleOcticon(props: CircleOcticonProps) {
const {size = 32, as, icon: IconComponent, bg, ...rest} = props
const {size = 32, as, icon: IconComponent, bg, 'aria-label': ariaLabel, ...rest} = props
return (
<Box
as={as}
Expand All @@ -23,7 +23,7 @@ function CircleOcticon(props: CircleOcticonProps) {
borderColor="border.default"
>
<Box display="flex" as={as} size={size} {...rest} alignItems="center" justifyContent="center">
<IconComponent size={size} />
<IconComponent size={size} aria-label={ariaLabel} />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for reviewer: Moved aria-label from the container to the icon component

</Box>
</Box>
)
Expand Down
15 changes: 10 additions & 5 deletions packages/react/src/Octicon/Octicon.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,20 @@ import type {Meta, StoryFn} from '@storybook/react'
import Octicon from './Octicon'
import {HeartFillIcon} from '@primer/octicons-react'

export default {
const meta: Meta<typeof Octicon> = {
title: 'Components/Octicon',
component: Octicon,
} as Meta<typeof Octicon>
}
export default meta

export const Default = () => <Octicon icon={HeartFillIcon} size={32} />
export const Default = () => <Octicon icon={HeartFillIcon} aria-label="Like" size={32} />

export const Playground: StoryFn<typeof Octicon> = args => <Octicon icon={HeartFillIcon} {...args} />
export const Playground: StoryFn<typeof Octicon> = ({'aria-label': ariaLabel, ...args}) => (
<Octicon icon={HeartFillIcon} aria-label={ariaLabel ? ariaLabel : undefined} {...args} />
)

Playground.args = {
ariaLabel: 'Heart',
'aria-label': 'Heart',
size: 32,
}

Expand All @@ -25,6 +28,8 @@ Playground.argTypes = {
},
verticalAlign: {
type: 'string',
control: {type: 'select'},
options: ['middle', 'text-bottom', 'text-top', 'top', 'unset'],
},
icon: {
controls: false,
Expand Down
Loading