Skip to content

fix(Treevieew):do not add aria-describedby attribute when empty leading/trailing visual - #5196

Merged
francinelucca merged 2 commits into
mainfrom
francinelucca/4457-treeview-remove-aria-describedby-when-leading-and-trailing-visuals-do-not-exist
Nov 5, 2024
Merged

fix(Treevieew):do not add aria-describedby attribute when empty leading/trailing visual#5196
francinelucca merged 2 commits into
mainfrom
francinelucca/4457-treeview-remove-aria-describedby-when-leading-and-trailing-visuals-do-not-exist

Conversation

@francinelucca

@francineluccafrancinelucca commented Oct 31, 2024

Copy link
Copy Markdown
Member

Closes#4457

Currently a leadingVisualId and TrailingVisualId are generated and attached to the TreeItem's aria-describedby attribute regardless of whether these nodes are supplied to the component or not. This PR refactors the aria-describedby logic in to only add aria-describedby when the nodes (for trailing and leading visuals) are actually present, and mark it as undefined otherwise.

Changelog

New

  • Adds test to cover for new behavior

Changed

  • aria-describedby is only added to TreeItem's when at least one of leading/trailing visual is present and only the present ID(s) is added.

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

Load deployed treeview stories and ensure TreeItem's aria-describedby only contains the Id(s) of the present leading/trailing visual in the various stories.

Merge checklist

@francineluccafrancinelucca added status: review needed accessibility component: TreeView Issues related to the TreeView component staff Author is a staff member labels Oct 31, 2024
@changeset-bot

changeset-botBot commented Oct 31, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 03a0e08

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

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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
Copy Markdown
Contributor

👋 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-actionsgithub-actionsBot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Oct 31, 2024
@github-actions

github-actionsBot commented Oct 31, 2024

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
packages/react/dist/browser.esm.js97.36 KB (-0.17% 🔽)
packages/react/dist/browser.umd.js97.73 KB (+0.11% 🔺)

@primer-integration

Copy link
Copy Markdown

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

@primer-integration

Copy link
Copy Markdown

🟢 golden-jobs completed with status success.

@francinelucca
francinelucca marked this pull request as ready for review October 31, 2024 15:31
@francinelucca
francinelucca requested a review from a team as a code ownerOctober 31, 2024 15:31

@keithamuskeithamus left a comment

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.

Cool 👍

@francineluccafrancinelucca 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 status: review needed labels Nov 5, 2024
@francinelucca
francinelucca added this pull request to the merge queueNov 5, 2024
Merged via the queue into main with commit 49cbff2Nov 5, 2024
@francinelucca
francinelucca deleted the francinelucca/4457-treeview-remove-aria-describedby-when-leading-and-trailing-visuals-do-not-exist branch November 5, 2024 17:07
@primerprimerBot mentioned this pull request Nov 5, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibilitycomponent: TreeViewIssues related to the TreeView componentintegration-tests: passingChanges in this PR do NOT cause breaking changes in gh/ghstaffAuthor is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TreeView: Remove aria-describedby when leading and trailing visuals do not exist

2 participants

@francinelucca@keithamus