Skip to content

fix: close submenus on root Escape - #893

Merged
yoyo837 merged 1 commit into
react-component:masterfrom
nrps9909:codex/close-submenus-on-root-escape
Nov 3, 2026
Merged

yoyo837 merged 1 commit into
react-component:masterfrom
nrps9909:codex/close-submenus-on-root-escape

Conversation

@nrps9909

@nrps9909 nrps9909 commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • close popup submenus when Escape is pressed while the root menu itself has focus
  • preserve inline-menu behavior and the existing item-focused parent navigation
  • cover the click-triggered submenu regression reported in Ant Design

Fixes ant-design/ant-design#43267

Verification

  • npm test -- --runInBand — 147 tests and 20 snapshots passed on rebased head 41236b0556f84c069d92e3a99c28d7aa39e0e0ce (base ff8a0c4333fd856b522e6ac982ec5053307b8366)
  • Root-Escape regression fails on current-master runtime with the same tests, then passes on this head
  • npm run tsc
  • npm run compile and changed-file Prettier check
  • npm run lint — 0 errors (11 existing warnings)
  • Prior Ant Design 6.6.2 integration reproduction (not rerun in this rebase): the parent Dropdown closes, the submenu loses its open state, and its popup enters the leave motion

AI assistance disclosure: Codex was used to trace the Ant Design/rc-menu event path, build the regression test, implement the fix, and run verification. The reported behavior and test results were verified locally.

Summary by CodeRabbit

  • Bug 修复

    • 在非内联菜单模式下,当焦点位于菜单容器并按下 Esc 时,现可关闭所有已展开的子菜单。
    • 菜单关闭状态会同步更新,确保展开项被正确清空。
  • 测试

    • 增加了通过键盘 Esc 关闭子菜单的行为验证。

@vercel

vercel Bot commented Aug 28, 2026

Copy link
Copy Markdown

@nrps9909 is attempting to deploy a commit to the afc163's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Caution

Review failed

The pull request is closed.

No actionable comments were generated in the recent review.

Walkthrough

新增无障碍关闭回调。非 inline 模式下,根菜单容器接收 Escape 时会清空所有打开状态并转发原始键盘事件。新增测试验证子菜单关闭,并确认 onOpenChange([]) 被调用。

Changes

无障碍关闭流程

Layer / File(s) Summary
关闭回调与钩子 wiring
src/Menu.tsx, src/hooks/useAccessibility.ts
Menu 在存在已打开菜单项时调用 triggerOpenKeys(EMPTY_LIST, true),并将关闭回调传入 useAccessibility。
Escape 关闭处理与测试
src/hooks/useAccessibility.ts, tests/SubMenu.spec.tsx
非 inline 模式下,菜单容器直接接收 Escape 时会关闭菜单、转发原始键盘事件。测试验证所有子菜单关闭,且 onOpenChange 收到空数组。

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

  • react-component/menu#899:该 PR 也修改 useAccessibility 的 Escape 处理,并涉及焦点返回和事件传播。
Merge Risk: Moderate · up to 41236

Escape now closes submenus when the root container has focus, but not when a root submenu title has focus. Complete that path before merging; visible persistence after the parent dropdown closes depends on its mounting behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题“fix: close submenus on root Escape”准确概括了根菜单按下 Escape 时关闭子菜单的主要变更。
Linked Issues check ✅ Passed PR 实现了问题 #43267 的核心要求:当根菜单获得焦点并触发 Escape 时,通过 triggerAccessibilityClose 清空打开状态,从而关闭弹出子菜单。测试覆盖了鼠标打开子菜单后在根菜单触发 Escape 的场景。
Out of Scope Changes check ✅ Passed 变更仅涉及 Menu.tsx、useAccessibility.ts 和对应测试。新增关闭回调、处理根菜单 Escape,以及验证子菜单关闭,均直接服务于问题 #43267。未发现无关代码变更。
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

小兔按下 Escape 键
打开的子菜单随之关闭
空数组更新菜单状态
原始键盘事件继续传递
根菜单恢复清晰状态

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ad9b3cb-6d36-4f5a-8c5c-a1e27c4d66be

📥 Commits

Reviewing files that changed from the base of the PR and between 5e2a3e2 and 3baedee.

📒 Files selected for processing (3)
  • src/Menu.tsx
  • src/hooks/useAccessibility.ts
  • tests/SubMenu.spec.tsx

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

@yoyo837

yoyo837 commented Nov 2, 2026

Copy link
Copy Markdown
Member

Rebase please

@yoyo837

yoyo837 commented Nov 2, 2026

Copy link
Copy Markdown
Member

/rebase

@nrps9909
nrps9909 force-pushed the codex/close-submenus-on-root-escape branch from 3baedee to 41236b0 Compare November 3, 2026 02:05
@codecov

codecov Bot commented Nov 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.73%. Comparing base (ff8a0c4) to head (41236b0).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #893   +/-   ##
=======================================
  Coverage   99.72%   99.73%           
=======================================
  Files          26       26           
  Lines         738      745    +7     
  Branches      205      207    +2     
=======================================
+ Hits          736      743    +7     
  Misses          2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nrps9909

nrps9909 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current master ff8a0c4333fd856b522e6ac982ec5053307b8366 as requested. Signed/GitHub-Verified head: 41236b0556f84c069d92e3a99c28d7aa39e0e0ce; the same three-file root-Escape fix is retained, including the newly merged #899 submenu Escape consumption on master.

The root-Escape regression fails with the current-master runtime (one failure / 23 skipped) and passes after the rebase. Full current-head validation passes 13 suites / 147 tests / 20 snapshots, TypeScript, lint (0 errors / 11 existing warnings), ESM/CJS/declaration/Less compilation, and changed-file formatting. Dependencies were resolved from the declared ranges on Node 24.15.0 with legacy-peer-deps and lifecycle scripts disabled, without manifest/lockfile changes.

Pushed with an expected-old-head lease, verified the remote SHA/signature and all three changed source blobs, and updated the verification section. No new Ant Design/native-browser integration run in this rebase. AI-assisted rebase and validation; source and outputs checked.

@yoyo837
yoyo837 merged commit 4bdff45 into react-component:master Nov 3, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Menu - Submenu remains visible even after closing the parent menu

2 participants