-
Notifications
You must be signed in to change notification settings - Fork 638
TreeView: Add count
to TreeView
trailing actions
#6919
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: aada341 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
TreeViewSecondaryActions
interface to include optionalcount
andclassName
properties - Updates the trailing action rendering logic to conditionally use
Button
with 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed semicolon to hyphen in visually hidden text. This appears to be an unintended character change that should be reverted.
- {shortcutText} | |
; {shortcutText} |
Copilot uses AI. Check for mistakes.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The focus management logic is duplicated between the Button and IconButton implementations. Consider extracting this into a shared function to reduce code duplication.
Copilot uses AI. Check for mistakes.
{count ? ( | ||
<ActionList.TrailingVisual> | ||
{count} | ||
<VisuallyHidden>items</VisuallyHidden> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Copilot uses AI. Check for mistakes.
Adds
className
andcount
support toTreeView
trailing actions. This swaps usage ofIconButton
withButton
at times when utilizingcount
.Changelog
New
count
andclassName
withTreeView
TrailingActions.Rollout strategy
Testing & Reviewing
Merge checklist