feat(slash-menu): notion-like slash menu UX with inline filtering#2027
feat(slash-menu): notion-like slash menu UX with inline filtering#2027
Conversation
Rewrite the slash menu to follow Notion's UX pattern: typing / inserts the character into the document and opens the menu. Subsequent keystrokes filter items in real-time. ArrowUp/ArrowDown navigate with wrapping, Enter selects, and Escape deletes the /query text and closes the menu. - PM plugin emits editor events (slashMenu:navigate) for arrow/enter keys instead of relying on DOM event bubbling (fragile in presentation mode) - Vue component listens via editor.on() instead of document-level keydown - Wrapping navigation: ArrowUp from first wraps to last and vice versa - Improved selected item highlight (blue tint vs near-invisible gray) - Auto-close when cursor leaves /query range or moves to a different block - Right-click context menu preserved with separate trigger flow - Fixed test mock resolution (slash-menu.js directly, not index.js barrel) - Added mock state reset between tests to prevent cross-test leaking
There was a problem hiding this comment.
Pull request overview
This PR updates the slash menu to a Notion-like inline UX: typing / inserts the character into the document, opens the menu beneath the cursor, and uses subsequent document text to filter menu items in real time. It also replaces DOM keydown handling with editor-emitted navigation events for better reliability in presentation mode.
Changes:
- Update the ProseMirror slash-menu plugin to insert
/in-doc, auto-close based on cursor movement, and emitslashMenu:navigateevents for ArrowUp/Down/Enter. - Refactor
SlashMenu.vueto derive filtering from document text (textBetween) and handle navigation via editor events (with wrap-around selection). - Adjust and stabilize tests/mocks to match new event flow and document-based filtering.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/super-editor/src/extensions/slash-menu/slash-menu.js | Inserts / into the document when opening, adds slash-range auto-close, introduces trigger state, and emits slashMenu:navigate for keyboard navigation. |
| packages/super-editor/src/components/slash-menu/SlashMenu.vue | Removes hidden-input approach; syncs query from doc transactions, filters sections inline, and handles navigation via editor events. |
| packages/super-editor/src/extensions/slash-menu/slash-menu.test.js | Updates plugin tests for in-doc / insertion, Escape cleanup, trigger handling, and immediate reopen behavior. |
| packages/super-editor/src/components/slash-menu/tests/SlashMenu.test.js | Updates Vue tests to mock the correct module id, reset mock state, and simulate doc-based filtering + navigate events. |
| packages/super-editor/src/components/slash-menu/tests/testHelpers.js | Extends mocks (adds tr.delete) and updates listener assertions for slashMenu:navigate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 8393971be4
ℹ️ 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".
tupizz
requested review from
afn,
andrii-harbour,
artem-harbour,
caio-pizzol and
harbournick
Merge origin/main into slash-menu-notion-ux branch. Resolves conflicts caused by SD-1824 renaming slash-menu to context-menu on main while this branch modified the original slash-menu files. Resolution: keep PR's Notion-like UX behavior with main's naming conventions (contextMenu events, context-menu CSS classes, ContextMenuPluginKey).
…ation mode VerticalNavigation plugin was registered before ContextMenu in the plugin array, so it intercepted ArrowUp/ArrowDown first and moved the cursor to the previous line. This caused the ContextMenu plugin's apply() to detect the cursor left the /query range and auto-close the menu. Fix: check if context menu is open in VerticalNavigation's handleKeyDown and yield to let the ContextMenu plugin handle arrow navigation.
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
@tupizz good stuff — the inline slash menu feels much better than the old hidden-input approach.
two things to fix: a missing ?. on item.label that can crash filtering, and a bug where ArrowRight lets the cursor drift past the query text so executeCommand can accidentally delete real content. details inline.
one question: should the menu closing on zero results also delete the /query text? right now Escape deletes it but auto-close doesn't.
a few small cleanup ideas inline too (optional). test coverage for the new auto-close logic in apply() would be valuable.
left inline comments.
| return sections.value | ||
| .map((section) => ({ | ||
| ...section, | ||
| items: section.items.filter((item) => item.label.toLowerCase().includes(q)), |
There was a problem hiding this comment.
the old code had item.label?. here. without it, a custom item with no label crashes the menu.
| items: section.items.filter((item) => item.label.toLowerCase().includes(q)), | |
| items: section.items.filter((item) => (item.label ?? '').toLowerCase().includes(q)), |
| } | ||
| // All other keys pass through to PM (chars go into the document). | ||
| // apply() auto-closes if cursor leaves the /query range. | ||
| return false; |
There was a problem hiding this comment.
ArrowRight passes through to ProseMirror when the slash menu is open, so the cursor can move past the typed query into existing text. then executeCommand deletes from anchorPos to cursorPos — which now includes text the user didn't type. easiest fix: cap the delete range to only the / + query length.
| // Auto-close when search yields no results (only after sections have loaded) | ||
| watch(filteredItems, (items) => { | ||
| if (isOpen.value && searchQuery.value && items.length === 0 && sections.value.length > 0) { | ||
| closeMenu(); |
There was a problem hiding this comment.
Escape deletes the /query text, but this auto-close path doesn't. intentional? if yes, a short comment explaining why would help.
| } | ||
| }); | ||
| // Keep selectedId in sync with filtered results | ||
| watch(filteredItems, (items) => { |
There was a problem hiding this comment.
these two watchers watch the same thing and their conditions don't overlap. merge into one?
|
|
||
| // Close if no cursor or cursor at/before the / character | ||
| if (!$cursor || $cursor.pos <= mappedAnchor) { | ||
| editor.emit('contextMenu:close'); |
There was a problem hiding this comment.
this emit + return pattern shows up 3 times in 30 lines. a small closeState() helper would clean it up.
| try { | ||
| const $anchor = tr.doc.resolve(mappedAnchor); | ||
| // Find the nearest block ancestor for each position (handles run wrappers) | ||
| let anchorBlock = null; |
There was a problem hiding this comment.
same loop twice, just different variable. pull out a findBlockAncestor($pos) helper?
| sections.value = getItems({ ...context, trigger: 'slash' }); | ||
| currentContext.value = context; | ||
| const pluginState = ContextMenuPluginKey.getState(props.editor.state); | ||
| const trigger = pluginState?.trigger === 'click' ? 'click' : 'slash'; |
There was a problem hiding this comment.
pluginState, trigger, and selectedId are computed the same way in both branches. hoist above the if?
Resolves SD-1945
Summary
Rewrites the slash menu to follow Notion's UX pattern: typing
/inserts the character into the document and opens the menu below. Subsequent keystrokes filter items in real-time from the document text. This replaces the previous hidden-input approach with a more intuitive inline editing experience.Merged with main's context-menu rename (SD-1824) — all naming now uses
contextMenuconventions (contextMenu:open,contextMenu:navigate,ContextMenuPluginKey,.context-menuCSS classes).Behavior
Slash menu opens with
/visible in document/in a paragraph → the character appears in the document and the menu opens below the cursorReal-time filtering from document text
/to filter items (e.g.,/tabshows only "Insert table")Keyboard navigation with wrapping
/querytext from documentEscape cleans up
/querytext from the document and closes the menu/was typedRight-click context menu preserved
clicktrigger flow — no filtering, no document text insertionChanges
PM Plugin (
context-menu.js)contextMenu:navigateevents viaeditor.emit()instead of relying on DOM event bubbling, which was fragile in presentation mode (PresentationInputBridge creates synthetic KeyboardEvents that don't bubble naturally to document-level listeners)/is inserted into the document in the same transaction as the menu open — the text is visible and used for filtering/queryrange or moves to a different blockVue Component (
ContextMenu.vue)editor.on('contextMenu:navigate')instead ofdocument.addEventListener('keydown')— reliable in both flow and presentation modes/anchor and cursor position viastate.doc.textBetween()rgba(55, 53, 47, 0.08)(8% gray, nearly invisible) torgba(35, 131, 226, 0.14)(blue tint, clearly visible)VerticalNavigation fix (
vertical-navigation.js)return false) instead of intercepting arrow keys for cursor movement. This prevents ArrowUp from moving the cursor to the previous line and closing the menu.Naming alignment with main (SD-1824)
slashMenu:*→contextMenu:open,contextMenu:close,contextMenu:navigateSlashMenuPluginKey→ContextMenuPluginKey(deprecated alias preserved).slash-menu-*→.context-menu-*slashMenu→contextMenuTest Fixes
vi.mock('@extensions/context-menu')resolved toindex.jsbarrel, but the component importscontext-menu.jsdirectly — different module identifiers meant the mock didn't apply. Fixed tovi.mock('@extensions/context-menu/context-menu')ContextMenuPluginKey.getState.mockReset()in top-levelbeforeEachto prevent mock state leaking between teststr.delete: Added missingdeletemethod to mock transaction objectcontextMenu:navigateinstead of documentkeydownTest plan
/opens menu with character visible in document/filters items in real-time/querytext/querytext and closes menu/queryrange