fix(recip-selector): cleanup esc handling in recip selector - #5626
Merged
Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughEscape handling now closes recipient menus, clears recipient searches, and blurs recipient inputs. Channel compose passes a recipient selector debug name. Desktop and mobile email subject fields blur on Escape. The channel input caches inline-menu state and logs development diagnostics before returning that state. 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/features/block-email/component/BaseInput.tsx`:
- Around line 1818-1822: Update both subject input handlers and the hotkey
dispatcher so Escape on a subject input cannot trigger the “Close reply” action;
prefer excluding subject inputs in the dispatcher’s capture-phase logic, while
preserving the existing preventDefault and blur behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: de4e1d21-be53-4fbf-8414-e2eedca826ff
📒 Files selected for processing (5)
apps/web/src/features/block-channel/component/Compose.tsxapps/web/src/features/block-email/component/BaseInput.tsxapps/web/src/features/block-email/component/compose/ComposeSubject.tsxapps/web/src/features/channel/Input/ChannelInput.tsxapps/web/src/lib/core/component/RecipientSelector.tsx
Comment on lines
+1818
to
+1822
| onKeyDown={(e) => { | ||
| if (e.key !== 'Escape') return; | ||
| e.preventDefault(); | ||
| e.currentTarget.blur(); | ||
| }} |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 15 'registerHotkey|useHotkeyDOMScope|runWithInputFocused' apps/web --glob '*.{ts,tsx}'Repository: macro-inc/macro
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== BaseInput symbols and relevant ranges ==='
rg -n -C 12 "runWithInputFocused|hotkey: ['\"]escape|Close reply|onKeyDown=|on:keydown" apps/web/src/features/block-email/component/BaseInput.tsx
echo '=== hotkey implementation files ==='
rg -l "function registerHotkey|const registerHotkey|export .*registerHotkey|addEventListener\\(['\"]keydown|onKeyDown" apps/web/src/lib apps/web/src/features --glob '*.{ts,tsx}' | rg 'hotkey|Hotkey|keyboard|Key'
echo '=== reply Escape registrations ==='
rg -n -C 15 "Close reply|close reply|bottomReplyOpen|replyingToMessageId|hotkey: ['\"]escape" apps/web/src/features/block-email apps/web/src/lib/core --glob '*.{ts,tsx}' | head -n 1200Repository: macro-inc/macro
Length of output: 33308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=apps/web/src/lib/core/hotkey/hotkeys.ts
echo '=== hotkeys outline ==='
ast-grep outline "$file"
echo '=== hotkeys key symbols ==='
rg -n -C 20 "runWithInputFocused|addEventListener|removeEventListener|keydown|stopPropagation|preventDefault|attachHotkey|useHotkeyDOMScope|registerHotkey|dispatch" "$file"
echo '=== BaseInput scope attachment and lifecycle ==='
sed -n '1,180p' apps/web/src/features/block-email/component/BaseInput.tsx
sed -n '1120,1290p' apps/web/src/features/block-email/component/BaseInput.tsxRepository: macro-inc/macro
Length of output: 31161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== runCommand implementation and hotkey types ==='
rg -n -C 30 "export function runCommand|function runCommand|runCommand\\(" apps/web/src/lib/core/hotkey --glob '*.{ts,tsx}'
echo '=== compose scope declaration and attachment ==='
rg -n -C 25 "composeHotkeyScope|attachComposeHotkeys|useHotkeyDOMScope" apps/web/src/features/block-email/component/BaseInput.tsx
echo '=== all native keydown capture listeners near hotkey root ==='
rg -n -C 8 "addEventListener\\(['\"]keydown|capture: true|on:keydown" apps/web/src/lib/core apps/web/src/features/block-email --glob '*.{ts,tsx}'Repository: macro-inc/macro
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '=== runCommand body ==='
sed -n '435,535p' apps/web/src/lib/core/hotkey/utils.ts
echo '=== BaseInput document capture listener ==='
sed -n '560,645p' apps/web/src/features/block-email/component/BaseInput.tsx
echo '=== hotkey event finalization ==='
sed -n '700,750p' apps/web/src/lib/core/hotkey/hotkeys.ts
echo '=== static behavioral verifier ==='
python3 - <<'PY'
from pathlib import Path
base = Path("apps/web/src/features/block-email/component/BaseInput.tsx").read_text()
hotkeys = Path("apps/web/src/lib/core/hotkey/hotkeys.ts").read_text()
utils = Path("apps/web/src/lib/core/hotkey/utils.ts").read_text()
checks = {
"subject handlers lack stopPropagation": base.count("e.currentTarget.blur();") >= 2
and base.count("e.stopPropagation();") == 0,
"close reply runs with focused input": "description: 'Close reply'" in base
and "runWithInputFocused: true" in base[base.index("description: 'Close reply'"):base.index("description: 'Close reply'") + 500],
"global keydown listener uses capture": "document.addEventListener('keydown', handleKeyDown, { capture: true });" in hotkeys,
"close reply returns true": "return true;" in base[base.index("description: 'Close reply'"):base.index("description: 'Close reply'") + 900],
"dispatcher invokes preventDefault and stopPropagation": "e.preventDefault();" in hotkeys and "e.stopPropagation();" in hotkeys,
"dispatcher runs commands when input focused": "command.runWithInputFocused || !isEditableFocused" in hotkeys,
}
for name, ok in checks.items():
print(f"{name}: {'PASS' if ok else 'FAIL'}")
if not all(checks.values()):
raise SystemExit(1)
print("Conclusion: a target/bubble subject handler cannot suppress a document-capture hotkey listener that already dispatched Close reply.")
PYRepository: macro-inc/macro
Length of output: 8538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
base_path = Path("apps/web/src/features/block-email/component/BaseInput.tsx")
hotkey_path = Path("apps/web/src/lib/core/hotkey/hotkeys.ts")
utils_path = Path("apps/web/src/lib/core/hotkey/utils.ts")
base = base_path.read_text()
hotkeys = hotkey_path.read_text()
utils = utils_path.read_text()
subject_blocks = re.findall(
r"onKeyDown=\{\(e\) => \{\n"
r"\s+if \(e\.key !== 'Escape'\) return;\n"
r"(?P<body>.*?)"
r"\n\s+\}\}",
base,
re.S,
)
assert len(subject_blocks) == 2, len(subject_blocks)
for i, body in enumerate(subject_blocks, 1):
assert "e.preventDefault();" in body, i
assert "e.currentTarget.blur();" in body, i
assert "e.stopPropagation();" not in body, i
close_start = base.index("description: 'Close reply'")
close_block = base[close_start:base.index("hotkeyToken:", close_start)]
assert "runWithInputFocused: true" in close_block
assert "return true;" in close_block
assert "deleteDraftAndReset();" in close_block
assert "document.addEventListener('keydown', handleKeyDown, { capture: true });" in hotkeys
run_start = utils.index("export function runCommand(")
run_block = utils[run_start:]
assert "command.keyDownHandler?.(e)" in run_block
assert "e?.stopPropagation();" in run_block
print("PASS: both subject handlers blur without stopping propagation.")
print("PASS: Close reply runs with editable inputs and returns true.")
print("PASS: the dispatcher runs in document capture phase before target handlers.")
print("PASS: a captured Close reply command stops the event after its handler runs.")
PYRepository: macro-inc/macro
Length of output: 431
Fix Escape handling for subject inputs.
The hotkey dispatcher runs in document capture phase, before either subject handler. stopPropagation() in the current onKeyDown cannot prevent Close reply from running. Exclude subject inputs from Close reply, or add an earlier capture-phase guard in the dispatcher. Apply this to both subject inputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/features/block-email/component/BaseInput.tsx` around lines 1818
- 1822, Update both subject input handlers and the hotkey dispatcher so Escape
on a subject input cannot trigger the “Close reply” action; prefer excluding
subject inputs in the dispatcher’s capture-phase logic, while preserving the
existing preventDefault and blur behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.