Skip to content

Conversation

@francinelucca
Copy link
Member

@francinelucca francinelucca commented Sep 25, 2024

Closes https://github.com/github/primer/issues/3381

Adds focus styles for SegmentedControlButton and SegmentedControlIconButton components via calling getGlobalFocusStyles() to ensure focus styles comply with accessibility color contrast. This function explicitly adds focus state using color: colors.accent.fg. This fixes contrast issues when using safari's default focus state.

Before:
image showing SegmentedControl component with focus not meeting color contrast standard

After:
image showing SegmentedControl component with focus meeting color contrast standard

Changelog

Changed

  • Adds call to getGlobalFocusStyles() in SegmentedControlIconButtonStyled and SegmentedControlButtonStyled definitions.

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

  • Test SegmentedControl stories in preview deployment in safari, compare focus states to prod

Merge checklist

@changeset-bot
Copy link

changeset-bot bot commented Sep 25, 2024

🦋 Changeset detected

Latest commit: f3197ef

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

@github-actions
Copy link
Contributor

github-actions bot commented Sep 25, 2024

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 96.75 KB (-0.03% 🔽)
packages/react/dist/browser.umd.js 97.02 KB (-0.07% 🔽)

@github-actions github-actions bot temporarily deployed to storybook-preview-5029 September 25, 2024 18:46 Inactive
@francinelucca francinelucca added the staff Author is a staff member label Sep 25, 2024
…practice-dont-use-the-default-focus-state-style
@primer-integration
Copy link

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

…practice-dont-use-the-default-focus-state-style
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

@github-actions github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Oct 8, 2024
@francinelucca francinelucca added integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Oct 8, 2024
@francinelucca francinelucca added this pull request to the merge queue Oct 9, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2024

Uh oh! @francinelucca, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

Merged via the queue into main with commit 0d444d7 Oct 9, 2024
49 checks passed
@francinelucca francinelucca deleted the francinelucca/3381-prcsegmentedcontrol-best-practice-dont-use-the-default-focus-state-style branch October 9, 2024 16:03
@primer primer bot mentioned this pull request Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: SegmentedControl integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh staff Author is a staff member status: review needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants