fix(context-menu): fix slash menu dismissal state (SD-2747)#3234
Conversation
The slash command menu had three independent state bugs that combined to break the dismiss-and-retype flow: 1. Backspace and Delete were not handled anywhere — neither the PM plugin's handleKeyDown nor the Vue component's document keydown listener caught them, so pressing Backspace after opening the menu left it open. 2. A 5-second slashCooldown locked out subsequent `/` presses immediately after dismissal. The user typed `/`, dismissed the menu, typed `/` again to retry — and got a literal `/` inserted instead of the menu reopening. 3. Escape closed the menu but did not insert the slash the user originally typed (it had been preventDefault'd on open). Per the requirements that match Google Docs, dismissing with Escape should leave the slash visible while dismissing with Backspace should remove it. Plugin handleKeyDown now handles Backspace / Delete (close, no insert) and Escape / ArrowLeft (close, insert `/` at the original anchor). The 5-second cooldown is gone — subsequent `/` reopens the menu immediately. Focus shifts to the Vue search input when the menu opens, so the PM plugin can't see keys typed there. The Vue handleGlobalKeyDown handler gets the same three branches (Backspace/Delete close without insert, Escape closes and inserts the slash) so the dismissal works whichever element holds focus. Removed the three unit tests that codified the cooldown behavior; added six new tests covering the corrected dismissal contract.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 879d35941b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (props.editor && anchorPos !== null && anchorPos !== undefined) { | ||
| const tr = props.editor.state.tr.insertText('/', anchorPos); | ||
| const insertedAt = anchorPos + 1; | ||
| tr.setSelection(props.editor.state.selection.constructor.near(tr.doc.resolve(insertedAt))); | ||
| props.editor.dispatch(tr); |
There was a problem hiding this comment.
handleGlobalKeyDown always reinserts '/' on Escape whenever the menu is open, but the menu is also opened by right-click (handleRightClick dispatches type: 'open' with a normal cursor position). In that common path there was no suppressed slash keystroke to restore, so pressing Escape now mutates the document by inserting an unexpected / at anchorPos. Please condition this reinsertion on a slash-triggered open only (e.g., based on trigger/context), and keep Escape as close-only for context-menu opens.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… (SD-2747) While the menu is open, focus is on a hidden search input that captures keystrokes for filtering. The user saw no feedback — they typed `intex`, the filter eliminated all items, and the menu collapsed to a zero-height invisible box. Visually it looked like the menu had silently vanished. Two additions, scoped to the same menu: - A "Searching: /<typed text>" header appears at the top of the menu whenever the user has typed any filter characters. The header uses a monospaced font for the slash + query so it reads as "this is what you're literally typing," matching command-palette conventions. - A "No matching commands" empty state renders inside the items list when the filter has eliminated every item, so the menu always has visible content as long as it's open. Existing items, divider rendering, and selection state are unchanged.
luccas-harbour
left a comment
luccas-harbour
left a comment
There was a problem hiding this comment.
hey @tupizz, can you check this one? I was able to reproduce it in the editor. If I dismiss the context menu by pressing escape and it was opened via a right click, a slash character gets added:
- [P2] Gate slash reinsertion to slash-triggered menus — packages/super-editor/src/editors/v1/
components/context-menu/ContextMenu.vue:238-241
When the same ContextMenu is opened via right-click, handleRightClick also stores an anchorPos, but no / key was suppressed. This Escape path now unconditionally inserts /, so
pressing Escape to dismiss a right-click context menu mutates the document. Please only reinsert the slash for slash-triggered menus, and apply the same guard to the PM key
handler that also inserts / on Escape/ArrowLeft.
Also, I noticed that when I press /, the context menu appears and then if I press escape to dismiss it, it will insert the slash but then jump to a new line. And another thing is that it seems I have to press escape twice for the context menu to be dismissed.
Can you check these? Thanks!
| // SD-2747: Escape (or ArrowLeft) closes the menu and inserts a literal `/` at the | ||
| // anchor position — matches Google Docs, where the slash stays visible when the | ||
| // user dismisses the menu without picking an item. | ||
| if (event.key === 'Escape' || event.key === 'ArrowLeft') { | ||
| const { anchorPos } = pluginState; | ||
| event.preventDefault(); | ||
| view.dispatch(view.state.tr.setMeta(ContextMenuPluginKey, { type: 'close' })); | ||
|
|
||
| // Restore cursor position and focus | ||
| if (anchorPos !== null) { | ||
| const tr = view.state.tr.setSelection( | ||
| view.state.selection.constructor.near(view.state.doc.resolve(anchorPos)), | ||
| ); | ||
| view.dispatch(tr); | ||
| const insertTr = view.state.tr.insertText('/', anchorPos); | ||
| const insertedAt = anchorPos + 1; | ||
| insertTr.setSelection(view.state.selection.constructor.near(insertTr.doc.resolve(insertedAt))); | ||
| view.dispatch(insertTr); |
There was a problem hiding this comment.
Found this other problem:
ArrowLeft is lumped with Escape so it now inserts a literal /. Also, handleGlobalKeyDown does not handle ArrowLeft at all, so while the menu is open ArrowLeft is swallowed by the hidden input and never dismisses the menu — making this PM branch unreachable in the primary flow.
| // the slash was preventDefault'd when the menu opened, so we re-insert it here so the | ||
| // user's typed character is preserved when they decline to pick a command. Matches Google | ||
| // Docs' trigger-menu behavior. | ||
| if (event.key === 'Escape' && isOpen.value) { |
There was a problem hiding this comment.
the close+insert-/+selection-near+focus block is now in two places (here and context-menu.js:418-434) and only the Vue one actually runs (hidden input has focus while menu is open). want to drop the PM-side branches now that Vue owns dismissal, or extract a shared dismissSlashMenu(editor, anchorPos, { insertSlash }) helper?
…D-2747) Addresses Codex's P1 and Luccas's review on PR #3234. Pressing Escape (or ArrowLeft) on a context menu opened via right-click previously inserted a literal `/` at the click position, mutating the document — the dismissal path assumed the menu was always opened by a suppressed `/` keystroke. Track which gesture opened the menu and gate slash-reinsertion on `trigger === 'slash'` everywhere. Also wires ArrowLeft into the Vue-side `handleGlobalKeyDown`: the menu's hidden search input owns focus while the menu is open, so the PM plugin's matching branch never fires in the live flow and ArrowLeft was simply swallowed instead of dismissing the menu. Plus three undeclared CSS variables flagged by Codex (`--sd-ui-menu-header-bg`, `--sd-ui-menu-text-muted`, `--sd-ui-font-mono`) now have :root defaults so consumers can theme them and the inline fallbacks in `ContextMenu.vue` are no longer the only source. Changes - `context-menu.js`: new `trigger: 'slash'|'rightClick'|null` field on the plugin state, set from the existing `isRightClick` signal in the 'open' meta and cleared on 'close'. The PM-side Escape/ArrowLeft branch only inserts `/` when `trigger === 'slash'`. - `ContextMenu.vue::handleGlobalKeyDown`: same `trigger === 'slash'` gate applied to the Vue-owned dismissal path. ArrowLeft now joins Escape in this branch. - `variables.css`: declare the three undeclared `--sd-ui-menu-*` tokens at :root with the same fallback values currently inlined in ContextMenu.vue. TDD - 5 new failing unit tests before the fix, 5 green after: - records `trigger='slash'` on keyboard open - records `trigger='rightClick'` on clientX/clientY open - clears trigger on close - Escape on right-click open: closes without inserting `/` - ArrowLeft on right-click open: closes without inserting `/` - All 64 context-menu plugin tests pass. - All 44 ContextMenu.vue component tests pass. Verification - Full `super-editor` suite: 12 716 / 12 716 pass. - Browser repro on the dev app: - Right-click → Escape: menu closes, doc unchanged (no stray `/`). - Right-click → ArrowLeft: menu closes, doc unchanged. - Slash → Escape: menu closes, `/` inserted at anchor (regression preserved). - Slash → ArrowLeft: menu closes, `/` inserted at anchor (was previously swallowed because the PM plugin branch never reached when hidden input holds focus).
|
@luccas-harbour pushed `92e7c0268` addressing the Codex P1 plus the additional bugs you flagged. Browser-verified end-to-end on the dev app. Root causeThe plugin state didn't differentiate slash-keystroke opens from right-click opens. Both dismissal paths (the PM `handleKeyDown` and the Vue `handleGlobalKeyDown`) unconditionally reinserted `/` at the anchor — fine for slash-triggered opens (the original keystroke was preventDefault'd, so we owe the user a literal slash), but wrong for right-click opens where no keystroke was ever suppressed. Fix
TDD evidence5 new failing tests before the fix, 5 green after:
Regression guard
Browser repro on the dev app
Notes on your two additional observationsYou also mentioned (1) the slash insert on Escape causing a "new line" and (2) needing to press Escape twice. I couldn't reproduce either in agent-browser repro — single Escape always dismisses, and the inserted `/` lands on the same paragraph (`paraCount: 1`). Could you re-test on this revision and let me know if either still repros? If they do, a video / exact steps would help me trace — there may be a focus-state edge case my synthetic tests aren't hitting. |
luccas-harbour
left a comment
luccas-harbour
left a comment
There was a problem hiding this comment.
LGTM. I couldn't reproduce the issues I found during manual testing anymore 👍
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.98 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.142 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.144 |
|
🎉 This PR is included in superdoc-cli v0.8.0-next.113 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0-next.96 |
|
🎉 This PR is included in superdoc v1.30.0-next.94 The release is available on GitHub release |
3 participants
Demo
CleanShot.2026-05-11.at.15.00.36.mp4
CleanShot.2026-05-11.at.14.59.55.mp4
Summary
Three independent bugs combined to break the dismiss-and-retype flow on the
/command menu:handleKeyDownnor the Vue component's documentkeydownlistener caught them, so pressing Backspace after opening the menu left it open.slashCooldownlocked out subsequent/presses immediately after dismissal. Type/, dismiss the menu, type/again — and a literal/got inserted instead of the menu reopening./waspreventDefault'd on open, so the user's keystroke disappeared.Per the ticket spec (matching Google Docs):
/reopens the menu immediately, no cooldown.Changes
extensions/context-menu/context-menu.js(PM plugin): removedslashCooldown+ its 5 s timeout.handleKeyDownnow branches onBackspace/Delete(close, no insert),Escape/ArrowLeft(close, insert/at anchor), and the existing/-to-open path.components/context-menu/ContextMenu.vue(Vue): focus shifts to the hidden search input when the menu opens, so the PM plugin can't see keys typed afterward. Added the same Backspace/Delete and Escape branches tohandleGlobalKeyDownso the dismissal works whichever element holds focus.extensions/context-menu/context-menu.test.js: removed the three cooldown unit tests (they locked in the buggy behavior); added six new unit tests for Backspace / Delete / Escape / immediate reopen.Test plan
pnpm testforsuper-editor(12 711 / 12 724 pass — same asmainaside from the new tests)./→ press Backspace → press/again→→ ``/→ press Escape//→ press Delete→/Insert text/Insert table/Pasteand first item highlightedWhy remove the cooldown entirely
The original 5 s cooldown was intended to prevent rapid menu reopens, but its only behavioral effect was the bug in the ticket — typing
/again after dismissal inserted a literal/for 5 seconds. The dismissal handlers above are now explicit about state, so the cooldown's purpose is fully served bypluginState.openitself: when the menu is already open, the second/is ignored at the dispatch site without needing a separate timer.