Skip to content

ActionMenu: Avoid passive anchor label updates - #8611

Open
alexus37 wants to merge 5 commits into
mainfrom
agents/implement-primer-react-8608-fix
Open

alexus37 wants to merge 5 commits into
mainfrom
agents/implement-primer-react-8608-fix

Conversation

@alexus37

Copy link
Copy Markdown
Contributor

Closes #8608

Moves ActionMenu trigger-label capture out of passive effects and into the open interaction, where React batches it with the existing open update. This avoids an extra commit when closed menus mount while preserving the accessible name on first open.

Adds focused regression coverage for closed-mount render count, first-open labeling, Escape, Tab, selection, and focus restoration. No visual changes are expected.

Changelog

New

  • None.

Changed

  • ActionMenu no longer schedules an avoidable follow-up render to publish its trigger label after mount.
  • ActionMenu retains trigger-derived accessible labeling and existing close/focus behavior.

Removed

  • None.

Rollout strategy

  • No changes needed in github/github-ui
  • Needs changes in github/github-ui
  • Ships with feature flag(s)
  • Not sure
  • None

Testing & Reviewing

  • npm test -- packages/react/src/ActionMenu/ActionMenu.test.tsx — passed (59 tests)
  • npm run build — passed
  • npm test — passed (160 files; 2,902 passed, 3 skipped, 10 todo)
  • npm run type-check — passed
  • npm run lint — passed
  • npm run lint:css — passed
  • npm run format:diff — passed
  • STORYBOOK_URL=http://localhost:6006 npx playwright test --grep @avt — passed (8 tests)

Review the focused tests for the first-open accessible name and the different close/focus paths. The mount profiler assertion should remain at zero update commits after effects flush.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@changeset-bot

changeset-bot Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 21ee8c6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

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

@github-actions github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Oct 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ 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.

@github-actions
github-actions Bot temporarily deployed to storybook-preview-8611 October 22, 2026 15:40 Inactive
@github-actions github-actions Bot added integration-tests: failing Changes in this PR cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Oct 23, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 23a42dde-c59d-4f5a-97ca-4fa6f7c41164
@broccolinisoup
broccolinisoup marked this pull request as ready for review October 26, 2026 09:07
@broccolinisoup
broccolinisoup requested a review from a team as a code owner October 26, 2026 09:07
@broccolinisoup
broccolinisoup requested review from llastflowers and a balanced review from Copilot October 26, 2026 09:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

With native CSS anchor positioning enabled, first-open focus can enter the menu before its accessible label is available.

1 open issue
What changed in this PR

This PR addresses #8608 by avoiding label-state updates when closed ActionMenus mount, with regression coverage for labeling and focus behavior.

Changes:

  • Restricts trigger-label capture to a passive effect while the menu is open.
  • Adds closed-mount render-count, controlled-labeling, and close/focus tests.
  • Adds a patch changeset for @primer/react.
File Description
packages/​react/​src/​ActionMenu/​ActionMenu.tsx Gates trigger-label updates on the open state.
packages/​react/​src/​ActionMenu/​ActionMenu.test.tsx Adds render-count, labeling, and focus regression coverage.
.changeset/​quiet-action-menu-labels.md Documents the closed-mount rendering improvement.

Review effort: Balanced

Comment thread packages/react/src/ActionMenu/ActionMenu.tsx
@primer-integration

Copy link
Copy Markdown

⚠️ Integration PR Outdated

This integration PR does not have the latest commit from the primer/react PR.

Integration PR references: e6416ac07a4d521944a3e015542df668b5502e56
Latest commit on primer/react PR: 21ee8c6c363fff45b22155d32a612ff6a134e4ad

Please update the integration PR to reference the latest commit from the primer/react PR before reviewing workflow results.

This branch was successfully deployed

2 active deployments
storybook-preview-8611 — 21ee8c6c Deployed Oct 26, 2026 by github-actions[bot]
github-pages — 21ee8c6c Deployed Oct 26, 2026 by broccolinisoup via deploy-preview #33895
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: failing Changes in this PR cause breaking changes in gh/gh

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ActionMenu publishes anchor labels from passive effects

4 participants