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
Merged

Octicon remediation: aria-label #4811

merged 10 commits into from
Aug 20, 2024

Conversation

siddharthkp
Copy link
Member

@siddharthkp siddharthkp commented Aug 2, 2024

Changelog

  • Most of these are changes to storybook playground
  • Except 1 change where Octicon aria-label is now added to the icon instead of the icon container

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

Merge checklist

Copy link

changeset-bot bot commented Aug 2, 2024

🦋 Changeset detected

Latest commit: 83fce5d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@siddharthkp siddharthkp self-assigned this Aug 2, 2024
@github-actions github-actions bot added the staff Author is a staff member label Aug 2, 2024
@@ -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

@siddharthkp siddharthkp marked this pull request as ready for review August 2, 2024 12:45
@siddharthkp siddharthkp requested a review from a team as a code owner August 2, 2024 12:45
Copy link
Contributor

github-actions bot commented Aug 2, 2024

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 96.2 KB (-0.09% 🔽)
packages/react/dist/browser.umd.js 96.54 KB (-0.02% 🔽)

@github-actions github-actions bot temporarily deployed to storybook-preview-4811 August 2, 2024 12:49 Inactive
@siddharthkp siddharthkp added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Aug 2, 2024
Copy link
Contributor

@TylerJDev TylerJDev left a comment

Choose a reason for hiding this comment

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

1 non-blocking comment. Looks good!

@primer-integration
Copy link

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/338296

@siddharthkp siddharthkp removed the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Aug 20, 2024
@siddharthkp siddharthkp added the integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh label Aug 20, 2024
@siddharthkp siddharthkp added this pull request to the merge queue Aug 20, 2024
Merged via the queue into main with commit 5ee8704 Aug 20, 2024
34 checks passed
@siddharthkp siddharthkp deleted the octicon-remediation-1 branch August 20, 2024 14:20
@primer primer bot mentioned this pull request Aug 20, 2024
@primer primer bot mentioned this pull request Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh react staff Author is a staff member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants