fix(suggestion): keep dismissed state after dismissal#7570
fix(suggestion): keep dismissed state after dismissal#7570
Conversation
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: a6d2c40 The changes in this PR will be included in the next version bump. This PR includes changesets to release 72 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
Fixes @tiptap/suggestion dismissal behavior so exiting a suggestion (Escape / exitSuggestion) suppresses re-activation while the user continues typing in the same trigger context.
Changes:
- Track dismissed trigger position in plugin state (
dismissedFrom) and suppress activation when the match is in the same word. - Add whitespace-detection helper to clear dismissal when the user inserts whitespace/newlines.
- Add unit tests for dismissal scenarios and a patch changeset entry.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/suggestion/src/suggestion.ts | Adds dismissedFrom state + activation suppression logic; introduces hasInsertedWhitespace helper. |
| packages/suggestion/src/tests/suggestion.test.ts | Adds integration tests covering dismissal persistence and re-activation scenarios. |
| .changeset/fix-suggestion-dismissed-state-calm-river-flow.md | Patch changeset documenting the user-visible fix. |
| function hasInsertedWhitespace(transaction: Transaction): boolean { | ||
| if (!transaction.docChanged) { | ||
| return false | ||
| } | ||
| return transaction.steps.some(step => { |
There was a problem hiding this comment.
hasInsertedWhitespace relies on (step as any).slice, which bypasses type safety and is brittle across different ProseMirror Step types. Consider narrowing to ReplaceStep/ReplaceAroundStep (as done elsewhere in the repo) before reading slice, so this helper stays correct and typed as ProseMirror evolves.
| const sameWord = match.range.from === next.dismissedFrom | ||
| if (!sameWord || hasInsertedWhitespace(transaction)) { | ||
| next.dismissedFrom = null | ||
| } |
There was a problem hiding this comment.
dismissedFrom stores a document position but isn’t mapped through transaction.mapping on document changes. If content is inserted/deleted before the trigger (including remote/collab changes), the stored position can drift and the sameWord comparison can become incorrect. Map dismissedFrom forward when transaction.docChanged (and clear it if it was deleted) before comparing to match.range.from.
| const sameWord = match.range.from === next.dismissedFrom | |
| if (!sameWord || hasInsertedWhitespace(transaction)) { | |
| next.dismissedFrom = null | |
| } | |
| // Keep dismissedFrom in sync with document changes so comparisons to | |
| // match.range.from stay correct even when content is inserted/removed | |
| // before the trigger position (including remote/collab changes). | |
| if (transaction.docChanged) { | |
| const mapped = transaction.mapping.mapResult(next.dismissedFrom) | |
| next.dismissedFrom = mapped.deleted ? null : mapped.pos | |
| } | |
| if (next.dismissedFrom !== null) { | |
| const sameWord = match.range.from === next.dismissedFrom | |
| if (!sameWord || hasInsertedWhitespace(transaction)) { | |
| next.dismissedFrom = null | |
| } | |
| } |
| text: match.text, | ||
| transaction, | ||
| })) | ||
| ) { |
There was a problem hiding this comment.
The code/comment indicates dismissal should clear on whitespace/newline insertion, but the new unit tests only cover spaces. Please add a test that simulates a newline/Enter (e.g. split block) after dismissal to lock in the intended behavior and avoid regressions.
| import { describe, expect, it, vi } from 'vitest' | ||
|
|
||
| import { Suggestion } from '../suggestion.js' | ||
| import { exitSuggestion, Suggestion, SuggestionPluginKey } from '../suggestion.js' |
There was a problem hiding this comment.
Import formatting looks off (exitSuggestion,Suggestion missing a space after the comma) and doesn’t match the surrounding style; running the formatter should normalize this.
| import { exitSuggestion, Suggestion, SuggestionPluginKey } from '../suggestion.js' | |
| import { | |
| exitSuggestion, | |
| Suggestion, | |
| SuggestionPluginKey, | |
| } from '../suggestion.js' |
@tiptap/extension-character-count
@tiptap/extension-dropcursor
@tiptap/extension-focus
@tiptap/extension-gapcursor
@tiptap/extension-list-item
@tiptap/extension-history
@tiptap/extension-list-keymap
@tiptap/extension-placeholder
@tiptap/extension-table-header
@tiptap/extension-table-cell
@tiptap/extension-table-row
@tiptap/extension-task-item
@tiptap/extension-task-list
@tiptap/extension-audio
@tiptap/core
@tiptap/extension-bubble-menu
@tiptap/extension-bold
@tiptap/extension-blockquote
@tiptap/extension-code
@tiptap/extension-bullet-list
@tiptap/extension-code-block
@tiptap/extension-collaboration
@tiptap/extension-code-block-lowlight
@tiptap/extension-collaboration-caret
@tiptap/extension-color
@tiptap/extension-details
@tiptap/extension-drag-handle
@tiptap/extension-document
@tiptap/extension-drag-handle-react
@tiptap/extension-drag-handle-vue-2
@tiptap/extension-emoji
@tiptap/extension-drag-handle-vue-3
@tiptap/extension-file-handler
@tiptap/extension-floating-menu
@tiptap/extension-font-family
@tiptap/extension-heading
@tiptap/extension-hard-break
@tiptap/extension-highlight
@tiptap/extension-horizontal-rule
@tiptap/extension-image
@tiptap/extension-invisible-characters
@tiptap/extension-italic
@tiptap/extension-link
@tiptap/extension-list
@tiptap/extension-mathematics
@tiptap/extension-mention
@tiptap/extension-node-range
@tiptap/extension-ordered-list
@tiptap/extension-paragraph
@tiptap/extension-subscript
@tiptap/extension-strike
@tiptap/extension-superscript
@tiptap/extension-table
@tiptap/extension-text
@tiptap/extension-table-of-contents
@tiptap/extension-text-align
@tiptap/extension-text-style
@tiptap/extension-twitch
@tiptap/extension-underline
@tiptap/extension-unique-id
@tiptap/extension-typography
@tiptap/extension-youtube
@tiptap/html
@tiptap/extensions
@tiptap/markdown
@tiptap/react
@tiptap/pm
@tiptap/starter-kit
@tiptap/static-renderer
@tiptap/suggestion
@tiptap/vue-2
@tiptap/vue-3
commit: |
|
Will this implementation mean that adding/removing any character before the trigger. And then editing after the trigger again will activate it again? Maybe an acceptable compromise. Are you also testing with |
|
This implementation is dismissing the mention state until you leave the mention via a whitespace or newline character or you move the selection out of the suggestion range. That means, if you insert characters right before your mention and go back in, you'll have the mention state again which I think is fair. I think having support for allowSpaces makes it tricky to restore the suggestion state again except maybe using a newline. If you want I could add an escape hatch to give you more control over when you restore the mention state again, but then you'd need to handle the logic yourself if the default isn't fitting your needs. |
|
I was considering how to solve it with allowSpaces, and there was some mentions of tracking the triggering position to identify "already closed", but the fact that something that could cause that trigger to move would reset and begins showing suggestions again made it slightly flimsy. I was considering if there could be some decoration on the triggering char to mark it as "consumed", but there might be other flimsyness with that. Since we want This PR will definely make escape work more like you expect, especially without |
Changes Overview
After dismissing a suggestion via Escape, the suggestion menu would reappear as soon as the user typed another character in the same word. This makes dismissal feel broken — one Escape press should suppress the suggestion until the user clearly starts a
new input context.
Implementation Approach
Added a
dismissedFrom: number | nullfield to the suggestion plugin's internal state. When the user dismisses via Escape (dispatching{ exit: true }metadata), the trigger character's position (range.from) is stored indismissedFrom.On every subsequent transaction, before re-activating a match, the plugin checks:
match.range.from === dismissedFrom, the cursor is still in the dismissed word → stay suppresseddismissedFromdismissedFromdismissedFromThe key design decision:
dismissedFromintentionally survives the!next.activecleanup block at the bottom ofapply(), so it persists across inactive transactions.Testing Done
Added four new unit tests in
suggestion.test.tscovering the full matrix:@→ suggestion reopens@at a different position after dismissal → suggestion reopensVerification Steps
@foo— suggestion menu opensbar) — menu should not reappear (was broken before)@— menu should reappear ✓@— menu should reappear ✓Additional Notes
No public API changes. The
dismissedFromfield lives entirely in internal plugin state. The existingallow,shouldShow, andexitSuggestionAPI are unaffected.Checklist
Related Issues
Follows up on #6833 (Escape key to dismiss suggestions).