Skip to content

fix: consume Escape when returning from a submenu - #899

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

Problem and behavior

When an item in a popup submenu has focus, Escape already closes that submenu and focuses its parent item, but the event continues to window-level popup listeners. A surrounding Dropdown then closes too. Consume Escape only when a real parent menuitem was found. Escape at the root remains available to the surrounding popup; inline behavior is unchanged.

Refs react-component/dropdown#274. This complements #893 (Escape on the root container); it does not copy or replace that patch or #898's focus changes.

Validation

  • npm test -- --runInBand --no-cache: 13 suites, 146 tests, 20 snapshots passed.
  • Two new regression cases cover one and two submenu levels, parent-item focus, open keys, and root Escape propagation. With only the hook restored to base 8dfdef453a24cee1bda115e3249b17289356829d, both regressions fail (10 existing keyboard tests pass).
  • TypeScript, compile, and lint pass; lint reports 0 errors / 11 existing warnings. Existing isolated dependency tree reused, Node 24.15.0.
  • Actual Chrome click → Tab → ArrowRight → Child → Escape: submenu closes, parent Dropdown remains open, focus returns to Submenu. The unmodified control closes the parent Dropdown. Also verified three-level unwind with the companion Dropdown guard plus build(deps): update ismobilejs requirement from ^0.5.1 to ^1.0.2 #273's capture-phase change, and root Escape still closes and returns trigger focus.

The companion react-component/dropdown#275 lets nested menu Escape reach this handler when react-component/dropdown#273 uses a window capture listener; the current bubble-listener Dropdown works with this Menu fix alone. Native bundles use the same dependency tree/React instance with source controls; this is not release integration or a screen-reader conformance claim.

AI assistance disclosure: Codex assisted tracing, implementation, regression tests and browser verification; the reported tests and real key-input results were executed locally.

Summary by CodeRabbit

  • Bug 修复
    • 在子菜单中按下 Escape 键时,当前菜单会关闭,焦点返回对应的父菜单项,且事件不会继续冒泡。
    • 在根菜单中按下 Escape 键时,事件仍会正常冒泡。
  • 测试
    • 补充了不同子菜单层级及根菜单的 Escape 键行为测试。

@vercel

vercel Bot commented Oct 23, 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 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: 4975b03a-43db-4e36-9383-a378194ae707
📥 Commits

Reviewing files that changed from the base of the PR and between 8dfdef4 and e00e0c2.

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

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

Walkthrough

子菜单收到 Escape 时,现在会停止事件传播。子菜单仍会关闭,焦点仍会返回父菜单项。根菜单项上的 Escape 事件仍会传播至 window。新增测试覆盖子菜单深度为 1、2 的情况及根菜单行为。

Changes

子菜单 Escape 处理

Layer / File(s) Summary
处理 Escape 传播与焦点返回
src/hooks/useAccessibility.ts, tests/Keyboard.spec.tsx
存在父菜单项时,Escape 事件会停止传播;子菜单关闭和焦点返回行为不变。测试验证不同深度的子菜单行为,以及根菜单 Escape 事件继续传播至 window。

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Possibly related issues

  • react-component/dropdown issue 274:描述 Escape 会同时关闭子菜单和外层 Dropdown;本次更改会在子菜单处理路径中停止 Escape 传播,并将焦点返回父菜单项。
Merge Risk: ⚪ Minimal · up to e00e0

The submenu Escape change has no identified merge-blocking issue; it is ready for normal merge checks.

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 2 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: consume Escape when returning from a submenu”准确概括了主要变更:从子菜单返回时消费 Escape 事件,避免事件继续传播到外部 Dropdown。标题简洁、明确,并与代码和测试变更一致。
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.
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 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.

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