Uh oh!
There was an error while loading. Please reload this page.
TreeView: Add count to TreeView trailing actions - #6919
Conversation
🦋 Changeset detectedLatest commit: f9935cf The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
👋 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! |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the TreeView component by adding count and className support to trailing actions. When a count is provided, the component automatically switches from using IconButton to Button to properly display the count value.
- Extends the
TreeViewSecondaryActionsinterface to include optionalcountandclassNameproperties - Updates the trailing action rendering logic to conditionally use
Buttonwith count support orIconButton - Adds count display functionality to the action dialog component
Reviewed Changes
Copilot reviewed 3 out of 12 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/react/src/TreeView/TreeView.tsx | Main implementation adding count and className support with conditional Button/IconButton rendering |
| packages/react/src/TreeView/TreeView.examples.stories.tsx | Example story demonstrating the new count functionality |
| .changeset/two-pots-battle.md | Changeset documenting the minor version feature addition |
| <> | ||
| <div id={trailingActionId} className={clsx('PRIVATE_VisuallyHidden', classes.TreeViewVisuallyHidden)}> | ||
| ; {shortcutText} | ||
| - {shortcutText} |
There was a problem hiding this comment.
Changed semicolon to hyphen in visually hidden text. This appears to be an unintended character change that should be reverted.
| -{shortcutText} | |
| ;{shortcutText} |
| onKeyDown={() => { | ||
| // hack to send focus back to the tree item after the action is triggered via click | ||
| // this is needed because the trailing action shouldn't be focused, as it does not interact well with | ||
| // the focus management of TreeView | ||
| const parentElement = document.getElementById(itemId) | ||
| parentElement?.focus() | ||
| }} |
There was a problem hiding this comment.
The focus management logic is duplicated between the Button and IconButton implementations. Consider extracting this into a shared function to reduce code duplication.
| {count ? ( | ||
| <ActionList.TrailingVisual> | ||
| {count} | ||
| <VisuallyHidden>items</VisuallyHidden> |
There was a problem hiding this comment.
The hardcoded 'items' text may not be appropriate for all use cases of count. Consider making this text configurable or using more generic language like 'count' to better represent various counting scenarios.
Adds
classNameandcountsupport toTreeViewtrailing actions. This swaps usage ofIconButtonwithButtonat times when utilizingcount.Changelog
New
countandclassNamewithTreeViewTrailingActions.Rollout strategy
Testing & Reviewing
Merge checklist