Skip to content

TextInputWithTokens: announce selected token values for screen readers#7618

Merged
liuliu-dev merged 3 commits intomainfrom
liuliu/fix-selected-tokens-announcement
Mar 4, 2026
Merged

TextInputWithTokens: announce selected token values for screen readers#7618
liuliu-dev merged 3 commits intomainfrom
liuliu/fix-selected-tokens-announcement

Conversation

@liuliu-dev
Copy link
Copy Markdown
Contributor

Related issue: https://github.com/github/accessibility-audits/issues/15031

Changelog

Fixes an accessibility issue so screen readers can announce selected tokens in the input. Added a hidden selected values description use it for aria-describedby when the input role is combobox.

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

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 3, 2026

🦋 Changeset detected

Latest commit: c8a87d4

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 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 Mar 3, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 3, 2026

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Or, apply the integration-tests: skipped manually label to skip these checks.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves accessibility of the deprecated TextInputWithTokens component by adding a screen-reader-only description of currently selected token values and wiring it into aria-describedby when the input is used with role="combobox" (e.g., with Autocomplete).

Changes:

  • Add a VisuallyHidden “Selected: …” description and append its id to aria-describedby when role="combobox".
  • Add a unit test verifying the combobox is described by the selected token values.
  • Add a patch changeset for @primer/react.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/react/src/TextInputWithTokens/TextInputWithTokens.tsx Derives a selected-values description and conditionally links it via aria-describedby for combobox usage.
packages/react/src/TextInputWithTokens/TextInputWithTokens.test.tsx Adds a test asserting selected values are present in the element(s) referenced by aria-describedby.
.changeset/three-suns-move.md Declares a patch release note for the accessibility change.
Comments suppressed due to low confidence (3)

packages/react/src/TextInputWithTokens/TextInputWithTokens.tsx:123

  • selectedTokenTexts/selectedValuesDescription are recomputed on every render (including when role is not combobox). Since this component rerenders on keystrokes, this creates avoidable O(n) work and string allocation proportional to the token count. Consider guarding the computation behind role === 'combobox' and memoizing the derived description (e.g., via useMemo keyed on tokens/role).
  const selectedTokenTexts = tokens
    .map(token => {
      if ('text' in token && typeof token.text === 'string' && token.text.trim().length) {
        return token.text
      }

      return null
    })
    .filter((tokenText): tokenText is string => tokenText !== null)
  const selectedValuesDescription = selectedTokenTexts.length ? `Selected: ${selectedTokenTexts.join(', ')}` : ''

packages/react/src/TextInputWithTokens/TextInputWithTokens.tsx:124

  • The generated aria-describedby text lists every selected token value. With large token sets this can become extremely verbose for screen readers and may negatively affect usability/performance. Consider truncating the announcement (e.g., first N tokens + “and X more”) or aligning it with visibleTokenCount when provided.
  const selectedValuesDescription = selectedTokenTexts.length ? `Selected: ${selectedTokenTexts.join(', ')}` : ''
  const shouldExposeSelectedValuesDescription = role === 'combobox' && Boolean(selectedValuesDescription)

.changeset/three-suns-move.md:5

  • Changeset message references TextInputTokens, but the exported component is TextInputWithTokens (and the PR title uses that name). For clarity in the changelog and searchability, consider updating the changeset text to match the public export name.
TextInputTokens: announce selected token values for screen readers.

Copy link
Copy Markdown
Contributor

@llastflowers llastflowers left a comment

Choose a reason for hiding this comment

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

Looks great! I tested the storybook preview and the SR behavior appears as expected. Just one super minor nitpick in the changeset that I noticed

'@primer/react': patch
---

TextInputTokens: announce selected token values for screen readers.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
TextInputTokens: announce selected token values for screen readers.
TextInputWithTokens: announce selected token values for screen readers.

@primer-integration
Copy link
Copy Markdown

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

@primer-integration
Copy link
Copy Markdown

Integration test results from github/github-ui:

Passed  CI   Passed
Passed  VRT   Passed
Passed  Projects   Passed

All checks passed!

@liuliu-dev liuliu-dev added this pull request to the merge queue Mar 4, 2026
Merged via the queue into main with commit 17a103c Mar 4, 2026
52 checks passed
@liuliu-dev liuliu-dev deleted the liuliu/fix-selected-tokens-announcement branch March 4, 2026 22:33
@primer primer bot mentioned this pull request Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants