Skip to content

fix: let nested menus handle Escape before closing Dropdown - #275

Open
nrps9909 wants to merge 1 commit into
react-component:masterfrom
nrps9909:codex/fix-submenu-escape
Open

nrps9909 wants to merge 1 commit into
react-component:masterfrom
nrps9909:codex/fix-submenu-escape

Conversation

@nrps9909

@nrps9909 nrps9909 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Problem and behavior

A window Escape listener should leave Escape inside a nested menu to that menu instead of closing its surrounding Dropdown first. Identify the root overlay menu (Menu's list ref, a DOM ref, or a wrapped menu), and skip outer close/focus when the event comes from a different, portaled menu. Root-menu Escape and non-menu overlays keep their existing behavior.

Refs #274 and react-component/menu#899. The Menu companion consumes Escape after closing a submenu; both changes preserve nested-menu behavior when #273 moves this listener to capture. This PR is based on current master and deliberately does not include #273's listener-phase changes. It can be reviewed independently; this guard alone is not a complete fix with menu 1.6.0, whose Escape still bubbles to the portal listener.

Validation

  • npm test -- --runInBand --no-cache: 6 suites, 33 tests, 1 snapshot passed.
  • Two new hook regressions cover portaled submenu events with a Menu ref and wrapped DOM overlay, and verify root Escape still closes and focuses the trigger. Both fail with the original hook (existing Tab test passes).
  • TypeScript and compile pass. Lint exits successfully: 0 errors / 1 existing example warning.
  • With only fix: use capture phase for keydown listener in useAccessibility #273's add/remove-listener capture flags temporarily applied, the new hook tests plus the earlier synchronous-teardown focus regression pass: 4 tests / 2 suites. The flags are not part of this diff.
  • Actual Chrome with this guard + fix: use capture phase for keydown listener in useAccessibility #273 capture flags + menu#899: Child → Escape returns focus to Submenu without closing the Dropdown; a second Escape closes it and returns trigger focus. Three-level Leaf → Inner → Outer → trigger unwind also works; single-level Escape retains fix: use capture phase for keydown listener in useAccessibility #273's trigger-focus restoration.

Same resolved dependency tree used for controls (React/ReactDOM19.3.1, trigger3.10.1, portal2.2.1); source Menu uses one aliased React instance. No npm-release inclusion, remote-CI result, or screen-reader conformance claimed by these local checks.

AI assistance disclosure: Codex assisted implementation, regression tests and native-browser verification; results were executed locally. No duplicate implementation of #273 is included.

Summary by CodeRabbit

  • 修复
    • 在嵌套菜单中按下 Escape 时,不再关闭外层弹出层,焦点会保留在子菜单项。
    • 在根菜单中按下 Escape 时,仍会关闭弹出层并将焦点移回触发器。

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c0f3e8f1-494f-495c-aee1-884a25ec6ffa
📥 Commits

Reviewing files that changed from the base of the PR and between b4ea62f and a7c05d2.

📒 Files selected for processing: 2
  • src/hooks/useAccessibility.ts
  • tests/useAccessibility.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

Escape 事件来自非根级嵌套菜单时,外层弹出层不会关闭,焦点保持不变。根菜单收到 Escape 时,仍会关闭弹出层并将焦点返回触发器。新增测试覆盖这两种情况。

Changes

菜单 Escape 处理

Layer / File(s) Summary
菜单层级判断与测试
src/hooks/useAccessibility.ts, tests/useAccessibility.test.tsx
handleKeyDown 将事件标注为 KeyboardEvent。处理 Escape 时,根据事件目标最近的菜单和根菜单判断是否关闭外层弹出层。测试覆盖嵌套菜单和根菜单的关闭及焦点行为。

Priority: ➖ Normal

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

Change: Bug fix

Possibly related issues

  • react-component/dropdown#274:该问题描述的嵌套菜单 Escape 行为与本次变更一致,包括跳过外层关闭和焦点处理。

Suggested reviewers: qdyanbing

Merge Risk: ⚪ Minimal · up to a7c05

Nested-menu Escape can leave the Dropdown open while root-menu Escape retains its close behavior. No merge-blocking issue was established.

Pre-merge checks: Passed 4 · Failed 1

Failed 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 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed 标题“fix: let nested menus handle Escape before closing Dropdown”准确描述了主要变更:当 Escape 来自另一个传送(portal)的嵌套菜单时,Dropdown 保持打开,由嵌套菜单先处理该事件。原始摘要显示 src/hooks/useAccessibility.ts 中的 handleKeyDown 增加了对非根级嵌套菜单事件目标的…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

标题“fix: let nested menus handle Escape before closing Dropdown”准确描述了主要变更:当 Escape 来自另一个传送(portal)的嵌套菜单时,Dropdown 保持打开,由嵌套菜单先处理该事件。原始摘要显示 src/hooks/useAccessibility.ts 中的 handleKeyDown 增加了对非根级嵌套菜单事件目标的判断,且新增测试覆盖了子菜单 Escape 不关闭 owner、根菜单 Escape 仍关闭并将焦点返回 trigger 的行为。目标描述(PR #275)与此一致。标题长度为 59 个字符、9 个单词,简洁且具体,符合标题规范。

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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 键,
子菜单轻轻合上帘。
外层弹窗仍安然,
根菜单关门不迟延。
焦点回到触发点,
胡萝卜旁继续向前。

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.23%. Comparing base (b4ea62f) to head (a7c05d2).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #275      +/-   ##
==========================================
+ Coverage   99.17%   99.23%   +0.06%     
==========================================
  Files           5        5              
  Lines         121      131      +10     
  Branches       36       44       +8     
==========================================
+ Hits          120      130      +10     
  Misses          1        1              

☔ 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.

This branch has not been deployed

No deployments
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.

1 participant