feat: implement ADR-023 Part 1 by francinelucca · Pull Request #7724 · primer/react

francinelucca · 2026-04-02T03:04:46Z

Towards https://github.com/github/primer/issues/6497

Changelog

New

Changed

Removed

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

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews (Storybook)
Changes are SSR compatible
Tested in Chrome
Tested in Firefox
Tested in Safari
Tested in Edge
(GitHub staff only) Integration tests pass at github/github-ui (Learn more about how to run integration tests)

changeset-bot · 2026-04-02T03:04:51Z

⚠️ No Changeset found

Latest commit: c7c0da7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2026-04-02T03:05:15Z

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

francinelucca · 2026-04-02T03:06:57Z

packages/react/src/ActionBar/ActionBar.tsx

+            aria-label={ariaLabel}
+            icon={icon}
+            {...props}
+            // TODO: does this make sense? it'll override IconButton's data-component


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?

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

they already have a data-component

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.

…dr-023

francinelucca · 2026-04-07T22:08:16Z

packages/react/src/ActionBar/ActionBar.tsx

+            icon={icon}
+            {...props}
+            // overriding IconButton's data-component so that the ActionBar's "More Menu" Icon can be targetted specifically
+            data-component="ActionBar.Menu.IconButton"


should this be:

ActionBar.MenuIconButton

ActionBar.Menu

ActionBar.Menu.IconButton
?

I feel like this is the IconButton that belongs to the Menu that is a subcomponent of ActionBar, which is why ActionBar.Menu.IconButton makes sense to me 🤔. ActionBar.Menu seems disingenuous because really this is not the menu itself, just the Icon trigger. But I could see the case for ActionBar.MenuIconButton

I vote for ActionBar.Menu.IconButton too!

tl;dr: both ActionBar.MenuIconButton and ActionBar.Menu.IconButton are fine, i like one more than the other

ActionBar.Menu 👎 that's for the menu, not the button

ActionBar.Menu.IconButton it's fine, but the double dots feels weird, idk why. Maybe because ActionBar.Menu is actually a shorthand for ActionMenu... but ActionBar.ActionMenu.IconButton is worse.

ActionBar.MenuIconButton: 👍 I like this the most. consistent pattern everywhere

francinelucca · 2026-04-08T02:47:15Z

packages/react/src/ActionList/Group.tsx

        <HeadingWrap
          role="presentation"
          className={groupClasses.GroupHeadingWrap}
          aria-hidden="true"
          data-variant={variant}
-          data-component="GroupHeadingWrap"
+          data-component="ActionList.GroupHeading"
          as={headingWrapElement}
          {...props}
        >
          <span className={clsx(className, groupClasses.GroupHeading)} id={groupHeadingId}>
            {_internalBackwardCompatibleTitle ?? children}
          </span>


Here the actual user-supplied className is in the span within the "HeadingWrap";
Should the data-component always be where the className is? or does it make sense for it to go on the outermost element?
if so:
For this case, should we do: data-component="GroupHeading.Wrapper" and data-component="GroupHeading"?
If so:
does that mean we should always have data-component for wrappers?

primer · 2026-04-08T02:50:09Z

🤖 Lint issues have been automatically fixed and committed to this PR.

feat(ActionBar): add data-component attributes for better accessibility

abfe9c4

github-actions bot added staff Author is a staff member integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Apr 2, 2026

francinelucca commented Apr 2, 2026

View reviewed changes

francinelucca deployed to github-pages April 2, 2026 03:15 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7724 April 2, 2026 03:15 Inactive

fix test

6381280

francinelucca deployed to github-pages April 2, 2026 03:31 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7724 April 2, 2026 03:31 Inactive

francinelucca added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Apr 2, 2026

francinelucca added 2 commits April 7, 2026 17:59

Merge branch 'main' of github.com:primer/react into chore/implement-a…

091de76

…dr-023

fix(ActionBar): update data-component attributes for IconButton and Menu

32d4062

francinelucca commented Apr 7, 2026

View reviewed changes

francinelucca deployed to github-pages April 7, 2026 22:15 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-7724 April 7, 2026 22:16 Inactive

Add data-component for ActionList

c7c0da7

francinelucca commented Apr 8, 2026

View reviewed changes

francinelucca deployed to github-pages April 8, 2026 02:53 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-7724 April 8, 2026 02:53 View deployment

francinelucca changed the title ~~feat(ActionBar): add data-component attributes for better accessibility~~ feat: implement ADR-023 Part 1 Apr 8, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement ADR-023 Part 1#7724

feat: implement ADR-023 Part 1#7724
francinelucca wants to merge 5 commits intomainfrom
chore/implement-adr-023

francinelucca commented Apr 2, 2026 •

edited

Loading

Uh oh!

changeset-bot bot commented Apr 2, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 2, 2026

Uh oh!

francinelucca Apr 2, 2026 •

edited

Loading

Uh oh!

siddharthkp Apr 2, 2026 •

edited

Loading

Uh oh!

francinelucca Apr 7, 2026

Uh oh!

llastflowers Apr 7, 2026

Uh oh!

siddharthkp Apr 8, 2026 •

edited

Loading

Uh oh!

francinelucca Apr 8, 2026

Uh oh!

primer bot commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

francinelucca commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog

New

Changed

Removed

Rollout strategy

Testing & Reviewing

Merge checklist

Uh oh!

changeset-bot bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

github-actions bot commented Apr 2, 2026

⚠️ Action required

Uh oh!

francinelucca Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

siddharthkp Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

francinelucca Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

llastflowers Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

siddharthkp Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

francinelucca Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

primer bot commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

francinelucca commented Apr 2, 2026 •

edited

Loading

changeset-bot bot commented Apr 2, 2026 •

edited

Loading

francinelucca Apr 2, 2026 •

edited

Loading

siddharthkp Apr 2, 2026 •

edited

Loading

siddharthkp Apr 8, 2026 •

edited

Loading