Skip to content

feat: implement ADR-023 Part 1 #7724

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.

Sign up for GitHub

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

Open
francinelucca wants to merge 32 commits into
base: main
Choose a base branch
from
Open

feat: implement ADR-023 Part 1 #7724

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
abfe9c4
feat(ActionBar): add data-component attributes for better accessibility
francinelucca Apr 2, 2026
6381280
fix test
francinelucca Apr 2, 2026
091de76
Merge branch 'main' of github.com:primer/react into chore/implement-a…
francinelucca Apr 7, 2026
32d4062
fix(ActionBar): update data-component attributes for IconButton and Menu
francinelucca Apr 7, 2026
c7c0da7
Add data-component for ActionList
francinelucca Apr 8, 2026
4cf2494
fix(ActionList): add data-component attributes for Item and LinkItem
francinelucca Apr 8, 2026
2962363
test(Tooltip): add tests for data-component attributes in Tooltip and…
francinelucca Apr 8, 2026
3f92c21
Merge branch 'main' of github.com:primer/react into chore/implement-a…
francinelucca Apr 9, 2026
273f681
fix: update data-component attribute for GroupHeading to GroupHeading…
francinelucca Apr 9, 2026
8c38035
fix comment
francinelucca Apr 9, 2026
8c3d244
Merge branch 'main' into chore/implement-adr-023
francinelucca Apr 13, 2026
8ca6eac
add data-component attribute to Pagination
francinelucca Apr 13, 2026
ddca738
Merge branch 'chore/implement-adr-023' of github.com:primer/react int…
francinelucca Apr 13, 2026
6f96ebd
Add data-component attributes to TextInput components for improved ac…
francinelucca Apr 13, 2026
17f2bf0
test: add nested ActionList data-component attribute tests for Filter…
francinelucca Apr 13, 2026
ff94d66
Merge branch 'main' into chore/implement-adr-023
francinelucca Apr 14, 2026
79e42d4
Merge branch 'chore/implement-adr-023' of github.com:primer/react int…
francinelucca Apr 14, 2026
b05df83
test: enhance data-component attribute tests for FilteredActionList a…
francinelucca Apr 14, 2026
db67cc2
test: add data-component attribute tests for Table and Pagination com…
francinelucca Apr 14, 2026
e71ce27
test: add data-component attribute tests for Table.Header and Table.S…
francinelucca Apr 14, 2026
f2b0a1f
test: add data-component attribute to Button and ButtonReset components
francinelucca Apr 14, 2026
e94fe52
test: add data-component attribute for LinkButton and its tests
francinelucca Apr 14, 2026
8c886da
test: add data-component attribute to Link component and its tests
francinelucca Apr 14, 2026
76bb50d
test: add data-component attributes for SelectPanel.CloseButton and S…
francinelucca Apr 14, 2026
bba65c1
test: add stable data-component selectors to DataTable, Button, Link,…
francinelucca Apr 14, 2026
9aaead8
rearrange stuff
francinelucca Apr 14, 2026
8429e82
chore: auto-fix lint and formatting issues
francinelucca Apr 14, 2026
a874ad0
Merge branch 'main' of github.com:primer/react into chore/implement-a…
francinelucca Apr 16, 2026
34ac550
chore: update octicon
francinelucca Apr 16, 2026
6e784d9
Merge branch 'chore/implement-adr-023' of github.com:primer/react int…
francinelucca Apr 16, 2026
8e7bf4c
Merge branch 'main' into chore/implement-adr-023
francinelucca Apr 20, 2026
e20e7a9
test(vrt): update snapshots
francinelucca Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions packages/react/src/ActionBar/ActionBar.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,65 @@
expect(document.activeElement).toEqual(menuButton)
})
})

describe('ActionBar data-component attributes', () => {
it('renders ActionBar with data-component attribute', () => {
const {container} = render(
<ActionBar aria-label="Toolbar">
<ActionBar.IconButton icon={BoldIcon} aria-label="Bold" />
</ActionBar>,
)

const actionBar = container.querySelector('[data-component="ActionBar"]')
expect(actionBar).toBeInTheDocument()
})

it('renders ActionBar.IconButton with data-component attribute', () => {
render(
<ActionBar aria-label="Toolbar">
<ActionBar.IconButton icon={BoldIcon} aria-label="Bold" />
</ActionBar>,
)

const iconButton = screen.getByRole('button', {name: 'Bold'})
expect(iconButton).toHaveAttribute('data-component', 'ActionBar.IconButton')
})

it('renders ActionBar.Divider with data-component attribute', () => {
const {container} = render(
<ActionBar aria-label="Toolbar">
<ActionBar.IconButton icon={BoldIcon} aria-label="Bold" />
<ActionBar.Divider />
<ActionBar.IconButton icon={ItalicIcon} aria-label="Italic" />
</ActionBar>,
)

const divider = container.querySelector('[data-component="ActionBar.Divider"]')
expect(divider).toBeInTheDocument()

Check failure on line 454 in packages/react/src/ActionBar/ActionBar.test.tsx

View workflow job for this annotation

GitHub Actions / test (react-19)

[@primer/react (chromium)] src/ActionBar/ActionBar.test.tsx > ActionBar data-component attributes > renders ActionBar.Divider with data-component attribute 

Error: expect(received).toBeInTheDocument()

received value must be an HTMLElement or an SVGElement.
Received has type:  Null
Received has value: null
 ❯ toBeInTheDocument src/ActionBar/ActionBar.test.tsx:454:20

Check failure on line 454 in packages/react/src/ActionBar/ActionBar.test.tsx

View workflow job for this annotation

GitHub Actions / test (react-18)

[@primer/react (chromium)] src/ActionBar/ActionBar.test.tsx > ActionBar data-component attributes > renders ActionBar.Divider with data-component attribute 

Error: expect(received).toBeInTheDocument()

received value must be an HTMLElement or an SVGElement.
Received has type:  Null
Received has value: null
 ❯ toBeInTheDocument src/ActionBar/ActionBar.test.tsx:454:20
})

it('renders ActionBar.Group with data-component attribute', () => {
const {container} = render(
<ActionBar aria-label="Toolbar">
<ActionBar.Group>
<ActionBar.IconButton icon={BoldIcon} aria-label="Bold" />
<ActionBar.IconButton icon={ItalicIcon} aria-label="Italic" />
</ActionBar.Group>
</ActionBar>,
)

const group = container.querySelector('[data-component="ActionBar.Group"]')
expect(group).toBeInTheDocument()
})

it('renders ActionBar.Menu with data-component attribute', () => {
render(
<ActionBar aria-label="Toolbar">
<ActionBar.Menu aria-label="More options" icon={BoldIcon} items={[{label: 'Option 1', onClick: vi.fn()}]} />
</ActionBar>,
)

const menuButton = screen.getByRole('button', {name: 'More options'})
expect(menuButton).toHaveAttribute('data-component', 'ActionBar.Menu')
})
})
15 changes: 12 additions & 3 deletions packages/react/src/ActionBar/ActionBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ export const ActionBar: React.FC<React.PropsWithChildren<ActionBarProps>> = prop

return (
<ActionBarContext.Provider value={{size, isVisibleChild}}>
<div ref={navRef} className={clsx(className, styles.Nav)} data-flush={flush}>
<div ref={navRef} className={clsx(className, styles.Nav)} data-component="ActionBar" data-flush={flush}>
<div
ref={listRef}
role="toolbar"
Expand Down Expand Up @@ -511,6 +511,8 @@ export const ActionBarIconButton = forwardRef(
size={size}
onClick={clickHandler}
{...props}
// TODO: does this make sense? it'll override IconButton's data-component
data-component="ActionBar.IconButton"
variant="invisible"
/>
)
Expand All @@ -532,7 +534,7 @@ export const ActionBarGroup = forwardRef(({children}: React.PropsWithChildren, f

return (
<ActionBarGroupContext.Provider value={{groupId: id}}>
<div className={styles.Group} ref={ref}>
<div className={styles.Group} data-component="ActionBar.Group" ref={ref}>
{children}
</div>
</ActionBarGroupContext.Provider>
Expand Down Expand Up @@ -571,7 +573,14 @@ export const ActionBarMenu = forwardRef(
return (
<ActionMenu anchorRef={ref} open={menuOpen} onOpenChange={setMenuOpen}>
<ActionMenu.Anchor>
<IconButton variant="invisible" aria-label={ariaLabel} icon={icon} {...props} />
<IconButton
variant="invisible"
aria-label={ariaLabel}
icon={icon}
{...props}
// TODO: does this make sense? it'll override IconButton's data-component
Copy link
Copy Markdown
Member Author

@francinelucca francinelucca Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I feel like this indeed makes no sense, but does that mean that subcomponents like ActionBar.Menu don't have a data-component, despite being public subcomponents? how would you target them then?: [data-component="ActionBar"] [data-component="IconButton"] isn't specific enough because ActionBar.IconButton would match this as well 🤔
Should we wrap everything top-level in a span for these cases just so we can attach the data-component attribute? is there any chance for that to mess anything up?

Copy link
Copy Markdown
Member

@siddharthkp siddharthkp Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

In this scenario, i think it makes sense to override the menu IconButton because there are other IconButtons in this component as well. (I'd call it ActionBar.MoreIconButton or ActionBar.MenuIconButton)

It's likely that it will be used with a classname on the ActionBar root .myActionBar [data-component=ActionBar.MoreIconButton]

That said, I don't think <Actionbar.IconButton> should get a data-component at all because

  1. they already have a data-component
  2. you can pass a classname to that

Should we wrap everything top-level in a span for these cases just so we can attach the data-component attribute?

Not a good idea because it will introduce nesting, which in best of cases is unnecessary and in the worst cases breaks dom validation rules or slots constraints

There will be other similar scenarios where we might choose to override the child's data-component when there are two distinct IconButtons that are baked in the component and not accessible in the API, for example the previous and next icon buttons in Pagination.

data-component="ActionBar.Menu"
/>
</ActionMenu.Anchor>
<ActionMenu.Overlay {...(returnFocusRef && {returnFocusRef})}>
<ActionList>{items.map((item, index) => renderMenuItem(item, index))}</ActionList>
Expand Down
Loading