Repository navigation
fix(frontend): モーダルが重なっているときに ESC で下のモーダルまで閉じる問題を修正 - #18066
Syogo-Suganoya wants to merge 2 commits into
Conversation
MkModal は ESC を document で拾っているため、確認ダイアログなどを重ねて 表示していると下のモーダル（投稿フォームなど）にも ESC が届いていた。 表示中のモーダルのうち最前面のものだけが ESC を受け取るようにした。 Fix misskey-dev#14388 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyH5iDXHDUwSpDhjuPGC9W
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VyH5iDXHDUwSpDhjuPGC9W
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
表示状態の追跡とESCイベント制御 packages/frontend/src/components/MkModal.vue |
表示中モーダルのz-indexをSetで管理します。非表示時とアンマウント時に登録を解除します。ESCキーでは、登録中の最大z-indexを持つモーダルだけがescイベントを発火します。 |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Suggested labels: packages/frontend
Suggested reviewers: syuilo
Merge Risk: 
a84ba
-
No actionable merge-blocking risk was identified in the modal ESC handling change.
Pre-merge checks: 
-
Check name Status Explanation Title check タイトルは、重なったモーダルでESCを押すと下のモーダルまで閉じる問題の修正を具体的に示しています。主な変更内容を正確に表しています。 Linked Issues check [ #14388] 要件: 「引用として添付しますか？」ダイアログで ESC を押したとき、ダイアログだけを閉じ、投稿フォームは閉じないこと。
実装: PR はMkModal.vueに、表示中モーダルの z-index を登録する Set を追加します。ESC は、登録された z-index の最大値と一致するモーダルだけがescイベントを発火します。モーダルは表示時に Set へ…Out of Scope Changes check 変更は MkModal.vueの ESC 処理に限定されます。ESC の振る舞いは共通コンポーネントで決まるため、変更は全モーダルに及びます。ただし、これは#14388の原因箇所への修正です。全モーダルに及ぶことは、設計上の影響であり、無関係な変更ではありません。
作者の説明では、公開範囲メニューなどのポップアップでも最前面のものだけが ESC で閉じるとされています。これは同じ仕組み…Description check PRの説明はテンプレートの必須項目である「What」「Why」「Additional info」「Checklist」を含んでいます。変更内容、修正理由、再現状況、確認結果、未実施のテストを具体的に記載しています。 Full details: Linked Issues check
-
Explanation
[
#14388] 要件: 「引用として添付しますか？」ダイアログで ESC を押したとき、ダイアログだけを閉じ、投稿フォームは閉じないこと。実装: PR は
MkModal.vueに、表示中モーダルの z-index を登録する Set を追加します。ESC は、登録された z-index の最大値と一致するモーダルだけがescイベントを発火します。モーダルは表示時に Set へ登録され、非表示時とアンマウント時に削除されます。これは根本原因（v-hotkey.globalが document の keydown として ESC を拾うこと）への対処です。確認結果: 作者はローカル環境で動作を確認したと報告しています。Before/After のスクリーンショットでは、1 回目の ESC でダイアログだけが閉じ、投稿フォームが残ります。2 回目の ESC で投稿フォームが閉じます。
確認できなかった点:
- 判断は PR 概要と作者の説明に基づきます。差分の全文は確認していません。
- 同じ z-index 値を持つモーダルが複数ある場合、それらすべてが ESC を受け取る可能性があります。また、そのうち 1 つが閉じたときに値が Set から削除されると、残りのモーダルが ESC を受け取れなくなる可能性があります。（推測）作者の確認結果から、引用ダイアログと投稿フォームは異なる z-index で表示されていると推定します。（推測）
- テストは追加されていません。ただし、リンク issue はテストを要求していないため、本判定には影響しません。
判定: 要件の振る舞いは、提示された証拠から満たされると判断します。
Full details: Out of Scope Changes check
-
Explanation
変更は
MkModal.vueの ESC 処理に限定されます。ESC の振る舞いは共通コンポーネントで決まるため、変更は全モーダルに及びます。ただし、これは#14388の原因箇所への修正です。全モーダルに及ぶことは、設計上の影響であり、無関係な変更ではありません。作者の説明では、公開範囲メニューなどのポップアップでも最前面のものだけが ESC で閉じるとされています。これは同じ仕組みによる結果です。独立した機能追加は確認されません。
テストと CHANGELOG の更新は PR チェックリストに記載されています。CHANGELOG.md は ignored_files に含まれるため、評価対象外です。テストは、リンク issue の要件ではありません。
判定: 無関係な変更は確認されませんでした。全モーダルへの影響は設計上の結果として留意してください。
-
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.
モーダルの順を記録して
ESCの行き先をひとつに
表示中ならSetに加え
閉じたら記録を外します
最前面だけが応答します
Comment @coderabbitai help to get the list of available commands.
| <script lang="ts"> | ||
| // 表示中のモーダルの z-index。ESC は最前面のモーダルだけが受け取るようにするために使う | ||
| const visibleModalZIndexes = new Set<number>(); | ||
| </script> |
There was a problem hiding this comment.
全 MkModal インスタンスで共有する状態なので、
<script setup> ではなく通常の <script> ブロック（モジュールスコープ）に置いています。
「最前面」の判定には z-index を使っています。
os.claimZIndex() は呼ぶたびに +100 した値を返し、
priority（low/middle/high）ごとに帯が分かれているので、
z-index 最大＝画面上で一番上のモーダル、と一致します。
Generated by Claude Code
| callback: () => { | ||
| // keydown は document で拾っているため、下に重なっているモーダルにも届いてしまう | ||
| if (zIndex !== Math.max(...visibleModalZIndexes)) return; | ||
| emit('esc'); | ||
| }, |
There was a problem hiding this comment.
v-hotkey.global は document に keydown リスナーを登録するので、
同じ要素上のリスナーは stopPropagation() では止まりません
（登録順＝下のモーダルから順に呼ばれる）。
そのため各モーダルが「自分が最前面か」を自分で判定して、
違えば何もしないようにしました。
There was a problem hiding this comment.
stopImmediatePropagation() で止める案も考えましたが、
登録順が下→上なので上のモーダルが先に処理できず、使えませんでした。
There was a problem hiding this comment.
stopImmediatePropagation() で止める案も考えましたが、
登録順が下→上なので上のモーダルが先に処理できず、使えませんでした。
| watch([showing, () => props.manualShowing], ([showing, manualShowing]) => { | ||
| if (manualShowing === true || (manualShowing == null && showing === true)) { | ||
| visibleModalZIndexes.add(zIndex); | ||
| if (modalRootEl.value != null) { | ||
| const { release } = focusTrap(modalRootEl.value, props.hasInteractionWithOtherFocusTrappedEls); | ||
| releaseFocusTrap = release; | ||
| modalRootEl.value.focus(); | ||
| } | ||
| } else { | ||
| visibleModalZIndexes.delete(zIndex); | ||
| releaseFocusTrap?.(); |
There was a problem hiding this comment.
既存の「表示/非表示」を判定している watch（focusTrap の付け外しと同じ場所）で
登録・解除しています。
manualShowing で一時的に隠しているモーダル（MkPopupMenu の
子メニュー表示中、MkEmojiPickerDialog の非表示時）も
「表示中」から外れるので、
隠れているモーダルが ESC に反応することはなくなります。
close() で showing が false になった時点で外れるため、
閉じるアニメーション中のモーダルは最前面扱いされず、次の ESC は下のモーダルに届きます。
Generated by Claude Code
| }); | ||
| onUnmounted(() => { | ||
| visibleModalZIndexes.delete(zIndex); |
There was a problem hiding this comment.
閉じアニメーションを経ずにアンマウントされた場合（親ごと消えた等）、
z-index が Set に残り続けると、
以降どのモーダルも最前面と判定されず ESC が効かなくなるので、
onUnmounted でも必ず外しています。
Generated by Claude Code
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #18066 +/- ##
============================================
- Coverage 26.73% 12.79% -13.94%
============================================
Files 1162 227 -935
Lines 39864 11135 -28729
Branches 10977 3686 -7291
============================================
- Hits 10658 1425 -9233
+ Misses 23432 7631 -15801
+ Partials 5774 2079 -3695 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🖥 Frontend Diagnostics Report(No significant changes) Requests by resource type
V8 heap snapshot statistics
📦 Bundle StatsChunk size diff (0 updated, 0 added, 0 removed)
Startup chunk size (0 updated, 0 added, 0 removed)
Startup chunks are the Vite entry for
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@syuilo z-indexを見る形で問題無さそうかしら |
|
うーむ |
|
z-indexに依存しない方が良さそうではありますね |
|
根本的にはhotkey側をどうにかした方が良い可能性を考えています |
|
hotkey 側で直す版を #18090 として作ってみました。
|
What
MkModalで、表示中のモーダルのうち最前面（z-index が最大）のものだけが ESC を受け取るようにした。Why
Fix #14388
投稿フォームに URL を貼ったときの
「引用として添付しますか？」ダイアログで ESC を押すと、
ダイアログだけを閉じるつもりが、下の投稿フォームまで閉じてしまうため。
原因は
MkModalがv-hotkey.globalでESC を
documentの keydown として拾っていること。モーダルが重なっていると、開いているモーダル全部に ESC が届く。
投稿フォームだけで対処せず
MkModal側で直したので、ほかの重なったモーダル（ポップアップメニューなど）でも ESC で閉じるのは最前面だけになる。
Additional info (optional)
ローカル環境で確認した。
引用ダイアログで ESC を 1 回押す
もう 1 回 ESC を押すと投稿フォームが閉じる（単独のモーダルの動きは変わらない）。
そのほかの確認
ESC を押す前（引用ダイアログ表示中）
公開範囲メニュー（ポップアップ）で ESC を 1 回押す
Checklist
🤖 Generated with Claude Code