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.
| // Check editor listeners | ||
| expect(editor.on).toHaveBeenCalledWith('update', expect.any(Function)); | ||
| expect(editor.on).toHaveBeenCalledWith('slashMenu:open', expect.any(Function)); | ||
| expect(editor.on).toHaveBeenCalledWith('slashMenu:close', expect.any(Function)); | ||
| expect(editor.on).toHaveBeenCalledWith('slashMenu:navigate', expect.any(Function)); | ||
|
|
There was a problem hiding this comment.
SlashMenu.vue now registers an editor.on('transaction', …) listener (and removes it on unmount), but these helper assertions don’t verify setup/cleanup for that listener. Adding assertions for transaction here would better guard against event-listener leaks/regressions.
| return sections.value | ||
| .map((section) => ({ | ||
| ...section, | ||
| items: section.items.filter((item) => item.label.toLowerCase().includes(q)), |
There was a problem hiding this comment.
filteredSections calls item.label.toLowerCase() without guarding, whereas previous code tolerated missing/undefined labels. If a menu item (especially from slashMenuConfig custom items) has no label, filtering will throw and break the menu; consider coercing/guarding ((item.label ?? '')) before lowercasing.
| items: section.items.filter((item) => item.label.toLowerCase().includes(q)), | |
| items: section.items.filter((item) => (item.label ?? '').toLowerCase().includes(q)), |
| // 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.
The watcher that auto-closes the menu when searchQuery yields zero results will prevent the "No results" UI from being usable (and may cause flicker). If the intended UX is to show No results, remove this auto-close; if auto-close is intended, the template footer/message should be adjusted accordingly.
| // 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(); | |
| } | |
| }); |
| // Navigation keys: consumed by PM so cursor doesn't move. | ||
| // Emit editor event for SlashMenu.vue to handle navigation. | ||
| if (event.key === 'ArrowDown' || event.key === 'ArrowUp' || event.key === 'Enter') { | ||
| event.preventDefault(); | ||
| editor.emit('slashMenu:navigate', { key: event.key }); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Now that navigation is handled by emitting slashMenu:navigate, the module-level docs/events list at the top of this file should be updated to include this new emitted event (and its payload), so consumers know it exists.
| const ensureStateShape = (value = {}) => ({ | ||
| open: false, | ||
| selected: null, | ||
| anchorPos: null, | ||
| menuPosition: null, | ||
| trigger: null, // 'slash' (types into doc, filters) or 'click' (context menu) | ||
| disabled: isMenuDisabled(), | ||
| ...value, | ||
| }); |
There was a problem hiding this comment.
A new trigger field was added to the plugin state shape, but the SlashMenuState / SlashMenuMeta JSDoc typedefs above don’t document it. Please update the typedefs to include trigger (and its allowed values) so downstream code and IDE tooling stay accurate.
| if (pluginState.trigger === 'slash') { | ||
| // Escape: delete /query text and close | ||
| if (event.key === 'Escape') { | ||
| event.preventDefault(); | ||
| const { anchorPos } = pluginState; | ||
| const cursorPos = view.state.selection.from; | ||
| const tr = view.state.tr; | ||
| if (anchorPos !== null && cursorPos > anchorPos) { | ||
| tr.delete(anchorPos, cursorPos); | ||
| } | ||
| tr.setMeta(SlashMenuPluginKey, { type: 'close' }); | ||
| view.dispatch(tr); | ||
| view.focus(); | ||
| return true; | ||
| } | ||
| // All other keys pass through to PM (chars go into the document). | ||
| // apply() auto-closes if cursor leaves the /query range. | ||
| return false; | ||
| } | ||
|
|
||
| // 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); | ||
| // Context-menu trigger: close on any non-navigation key | ||
| if (event.key === 'Escape' || event.key === 'ArrowLeft') { | ||
| view.dispatch(view.state.tr.setMeta(SlashMenuPluginKey, { type: 'close' })); | ||
| view.focus(); | ||
| return true; | ||
| } | ||
| return true; | ||
| view.dispatch(view.state.tr.setMeta(SlashMenuPluginKey, { type: 'close' })); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
The behavior for ArrowLeft/close has changed (slash-triggered menus intentionally pass ArrowLeft through, while click-triggered menus close on ArrowLeft/Escape). The inline comments and/or documentation around this branch should be updated so readers don’t assume ArrowLeft always closes the menu.
![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".
| if (cursorPos > pluginState.anchorPos) { | ||
| tr.delete(pluginState.anchorPos, cursorPos); |
There was a problem hiding this comment.
This deletion uses anchorPos..cursorPos as the query span, but after opening / the plugin allows non-handled keys (like ArrowRight) to move the cursor into pre-existing paragraph text while the menu stays open; executing a command (or Escape in the plugin) then removes original document characters, not just /query. A reproducible case is opening / at the start of a non-empty paragraph, pressing ArrowRight, then selecting an item: the leading character(s) from existing text are deleted.
Useful? React with 👍 / 👎.
tupizz
requested review from
afn,
andrii-harbour,
artem-harbour,
caio-pizzol and
harbournick
1 participant
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.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 (
slash-menu.js)slashMenu: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 (
SlashMenu.vue)editor.on('slashMenu: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)Test Fixes
vi.mock('@extensions/slash-menu')resolved toindex.jsbarrel, but the component importsslash-menu.jsdirectly — different module identifiers meant the mock didn't apply. Fixed tovi.mock('@extensions/slash-menu/slash-menu')SlashMenuPluginKey.getState.mockReset()in top-levelbeforeEachto prevent mock state leaking between teststr.delete: Added missingdeletemethod to mock transaction objectslashMenu:navigateinstead of documentkeydownTest plan
/opens menu with character visible in document/filters items in real-time/querytext/querytext and closes menu/queryrange