BranchName: Add leadingVisual, trailingAction and description#7993
BranchName: Add leadingVisual, trailingAction and description#7993siddharthkp wants to merge 22 commits into
leadingVisual, trailingAction and description#7993Conversation
Demonstrate a trailing action pattern (copy button) for BranchName, serving as a reference for future implementation work. Add WithTrailingActionMenu story with SelectPanel Demonstrates branch switching via SelectPanel anchored to BranchName, showing the 'Change base branch' pattern. Remove overflow:hidden to prevent focus ring clipping
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
- Add BranchName.LeadingVisual for branch/tag icons - Add BranchName.TrailingAction for copy/menu buttons - Add description prop for tooltip (aria-describedby) - Update stories to use new compound API - Add comprehensive tests for new API
…e-trailing-action-component # Conflicts: # packages/react/src/BranchName/BranchName.features.stories.tsx
leadingVisual, trailingAction and description
- Merge BranchName.new-api.test.tsx into BranchName.test.tsx - Document description/className props and LeadingVisual/TrailingAction subcomponents - Refresh story ids in docs.json
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
…wrapping LeadingVisual and TrailingAction now accept rest props and merge className. Collapse the description-tooltip branches into a single ConditionalTooltip render path.
|
🤖 Formatting issues have been automatically fixed and committed to this PR. |
There was a problem hiding this comment.
Pull request overview
This PR evolves BranchName (in packages/react) from a simple link/text pill into a compound component API that supports a leading visual, a trailing action button, and an optional description tooltip, with corresponding styling, Storybook examples, test coverage, VRT snapshots, and a release changeset.
Changes:
- Add
BranchName.LeadingVisualandBranchName.TrailingActionslot subcomponents plus adescriptiontooltip prop inBranchName. - Update CSS to support icon alignment and a trailing-action container layout.
- Update Storybook “Features” stories, unit/type tests, and e2e VRT coverage; add a changeset.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/BranchName/BranchName.tsx | Implements compound slot API (LeadingVisual, TrailingAction) and conditional description tooltip rendering. |
| packages/react/src/BranchName/BranchName.module.css | Switches to flex layout; adds styling for leading visual spacing and trailing-action wrapper/divider. |
| packages/react/src/BranchName/BranchName.features.stories.tsx | Updates stories to demonstrate the new compound API, description tooltip, and menu anchoring via anchorRef. |
| packages/react/src/BranchName/BranchName.docs.json | Documents the new description prop and subcomponents; expands referenced Storybook story IDs. |
| packages/react/src/BranchName/tests/BranchName.types.test.tsx | Updates type-level assertions to reflect the new as constraints and ref/event typing. |
| packages/react/src/BranchName/tests/BranchName.test.tsx | Adds unit tests for leading visual, trailing action behavior, and tooltip/aria-describedby behavior. |
| e2e/components/BranchName.test.ts | Expands VRT story coverage (including focus snapshots) for the new variants. |
| .changeset/whole-rings-glow.md | Adds a release note entry for the updated BranchName API. |
Copilot's findings
- Files reviewed: 8/71 changed files
- Comments generated: 6
| const {as: Component = 'a', className, children, description, ...rest} = props as BranchNameProps<'a'> | ||
|
|
||
| const [slots, textChildren] = useSlots(children, { | ||
| leadingVisual: LeadingVisual, | ||
| trailingAction: TrailingAction, | ||
| }) | ||
|
|
||
| const link = ( | ||
| <ConditionalTooltip description={description}> | ||
| <Component | ||
| {...rest} | ||
| ref={ref as React.Ref<HTMLAnchorElement>} | ||
| className={clsx(className, classes.BranchName)} |
| export type BranchNameProps<As extends 'span' | 'a'> = PolymorphicProps< | ||
| As, | ||
| 'a', | ||
| { | ||
| className?: string | ||
| /** Description tooltip text (renders as aria-describedby) */ | ||
| description?: string | ||
| } |
There was a problem hiding this comment.
Wondering about this, I found a few instances of other elements in dotcom
| "name": "ref", | ||
| "type": "React.RefObject<HTMLAnchorElement>" | ||
| }, | ||
| { | ||
| "name": "as", |
| { | ||
| "name": "className", | ||
| "type": "string", | ||
| "required": false, | ||
| "description": "A custom class name that is merged with the leading visual's class. Additional `span` props are forwarded to the underlying element.", | ||
| "defaultValue": "" | ||
| } |
| "@primer/react": minor | ||
| --- | ||
|
|
||
| BranchName: Add `leadingVisual`, `trailingAction` and `description` prop |
| --- | ||
| "@primer/react": minor | ||
| --- | ||
|
|
||
| BranchName: Add `leadingVisual`, `trailingAction` and `description` prop |
francinelucca
left a comment
There was a problem hiding this comment.
few comments/questions. Also noticed a mild css diff in the actionmenu story in the active state, not sure which one's correct:
Screen.Recording.2026-06-22.at.10.32.12.AM.mov
There was a problem hiding this comment.
noticed some of these are displaying tooltips and some aren't, just want to make sure this doesn't become flaky vrt 👀
| export type BranchNameProps<As extends 'span' | 'a'> = PolymorphicProps< | ||
| As, | ||
| 'a', | ||
| { | ||
| className?: string | ||
| /** Description tooltip text (renders as aria-describedby) */ | ||
| description?: string | ||
| } |
There was a problem hiding this comment.
Wondering about this, I found a few instances of other elements in dotcom
|
|
||
| // With a trailing action, render the action as a sibling of the link | ||
| return ( | ||
| <span className={classes.BranchNameWithTrailingAction} data-component="BranchName.WrapperWithAction"> |
There was a problem hiding this comment.
we don't do wrappers so this data-component is probably not needed
| <Stack direction="horizontal" gap="condensed" align="center"> | ||
| <Octicon icon={GitBranchIcon} /> | ||
| branch_name | ||
| </Stack> |
There was a problem hiding this comment.
is the previous way still supported? just making sure this isn't a breaking change
| <span className={styles.BranchNameWithTrailingAction}> | ||
| <BranchName href={`https://github.com/${repo}/tree/${selected.text}`} className={styles.BranchNameTransparent}> |
There was a problem hiding this comment.
any dead css as a result of this we can cleanup?
| // Note: the story was renamed to "With Leading Visual" but we keep the | ||
| // snapshot title as "With A Branch Icon" so the VRT diff stays usable. |
There was a problem hiding this comment.
double-checking that the change in spacing here is expected
|
Integration test results from github/github-ui PR:
CI check runs linting, type checking, and unit tests. Check the workflow logs for specific failures. Need help? If you believe this failure is unrelated to your changes, please reach out to the Primer team for assistance. |
API proposal document for upstreaming patterns from production usage into the
BranchNamecomponent:owner/repo:branchcontextThe proposal recommends Option A — Compound component based on analysis of github-ui usage patterns.
Changelog
New
packages/react/src/BranchName/BranchName.api-proposal.mdChanged
None
Removed
None
Rollout strategy
Testing & Reviewing
Review the proposal document for API design feedback.
Merge checklist