fix: Slash menu item selection behaviour (BLO-1222)#2838
Open
matthewlipski wants to merge 1 commit into
Open
Conversation
… content are found after the inserted block
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR fixes cursor navigation behavior at document end by modifying ChangesEnd-of-document Cursor Navigation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
@blocknote/ariakit
@blocknote/code-block
@blocknote/core
@blocknote/mantine
@blocknote/react
@blocknote/server-util
@blocknote/shadcn
@blocknote/xl-ai
@blocknote/xl-docx-exporter
@blocknote/xl-email-exporter
@blocknote/xl-multi-column
@blocknote/xl-odt-exporter
@blocknote/xl-pdf-exporter
commit: |
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/core/src/extensions/SuggestionMenu/getDefaultSlashMenuItems.ts`:
- Around line 35-39: The code currently hard-codes inserting { type: "paragraph"
} in getDefaultSlashMenuItems (used by insertOrUpdateBlockForSlashMenu), which
will break for custom schemas without a paragraph type; update the fallback to
select a valid inline-content block type from editor.schema.blockSchema (e.g.,
find the first block whose content model allows inline content) and use that
type in the call to editor.insertBlocks, and if none exists, throw a clear
schema error mentioning insertOrUpdateBlockForSlashMenu and
editor.schema.blockSchema so failure is explicit.
In `@tests/src/utils/copypaste.ts`:
- Around line 18-21: Guard the paragraph fallback by checking that the NodeList
referenced by paragraphs has at least one element before calling
userEvent.click; if paragraphs.length === 0 (and .bn-trailing-block is absent)
throw a clear error or fail the test instead of calling
userEvent.click(undefined). Update the code around the paragraphs variable and
the userEvent.click(paragraphs[paragraphs.length - 1]) call to perform this
explicit empty-check and error message.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e545d6a-b606-498d-9a64-d547b7b47d30
⛔ Files ignored due to path filters (17)
tests/src/end-to-end/ariakit/__screenshots__/ariakit.test.tsx/ariakit-image-toolbar-chromium-linux.pngis excluded by!**/*.pngtests/src/end-to-end/ariakit/__screenshots__/ariakit.test.tsx/ariakit-image-toolbar-firefox-linux.pngis excluded by!**/*.pngtests/src/end-to-end/ariakit/__screenshots__/ariakit.test.tsx/ariakit-image-toolbar-webkit-linux.pngis excluded by!**/*.pngtests/src/end-to-end/images/__screenshots__/images.test.tsx/create-image-chromium-linux.pngis excluded by!**/*.pngtests/src/end-to-end/images/__screenshots__/images.test.tsx/create-image-firefox-linux.pngis excluded by!**/*.pngtests/src/end-to-end/images/__screenshots__/images.test.tsx/create-image-webkit-linux.pngis excluded by!**/*.pngtests/src/end-to-end/images/__snapshots__/createImage.jsonis excluded by!**/__snapshots__/**tests/src/end-to-end/images/__snapshots__/embedImage.jsonis excluded by!**/__snapshots__/**tests/src/end-to-end/images/__snapshots__/resizeImage.jsonis excluded by!**/__snapshots__/**tests/src/end-to-end/keyboardhandlers/__snapshots__/deleteImage.jsonis excluded by!**/__snapshots__/**tests/src/end-to-end/keyboardhandlers/__snapshots__/deleteImageChild.jsonis excluded by!**/__snapshots__/**tests/src/end-to-end/shadcn/__screenshots__/shadcn.test.tsx/shadcn-image-toolbar-chromium-linux.pngis excluded by!**/*.pngtests/src/end-to-end/shadcn/__screenshots__/shadcn.test.tsx/shadcn-image-toolbar-firefox-linux.pngis excluded by!**/*.pngtests/src/end-to-end/shadcn/__screenshots__/shadcn.test.tsx/shadcn-image-toolbar-webkit-linux.pngis excluded by!**/*.pngtests/src/end-to-end/theming/__screenshots__/theming.test.tsx/dark-image-toolbar-chromium-linux.pngis excluded by!**/*.pngtests/src/end-to-end/theming/__screenshots__/theming.test.tsx/dark-image-toolbar-firefox-linux.pngis excluded by!**/*.pngtests/src/end-to-end/theming/__screenshots__/theming.test.tsx/dark-image-toolbar-webkit-linux.pngis excluded by!**/*.png
📒 Files selected for processing (4)
packages/core/src/extensions/SuggestionMenu/getDefaultSlashMenuItems.tstests/src/end-to-end/dragdrop/dragdrop.test.tsxtests/src/end-to-end/keyboardhandlers/keyboardhandlers.test.tsxtests/src/utils/copypaste.ts
Comment on lines
+35
to
+39
| const lastBlock = editor.document[editor.document.length - 1]; | ||
| const newBlock = editor.insertBlocks( | ||
| [{ type: "paragraph" }], | ||
| lastBlock, | ||
| "after", |
Contributor
There was a problem hiding this comment.
Avoid hard-coding "paragraph" in a schema-generic cursor fallback.
Line 37 always inserts { type: "paragraph" }, but this helper is reached through exported insertOrUpdateBlockForSlashMenu. In custom schemas without paragraph, this path will fail at runtime when insertion reaches document end. Pick an available inline-content type from editor.schema.blockSchema (or fail with a clear schema error) instead of assuming paragraph.
Proposed direction
- const newBlock = editor.insertBlocks(
- [{ type: "paragraph" }],
+ const fallbackInlineType = Object.entries(editor.schema.blockSchema).find(
+ ([, spec]) => spec.content === "inline",
+ )?.[0];
+ if (!fallbackInlineType) {
+ throw new Error(
+ "No inline-content block type available to place text cursor.",
+ );
+ }
+ const newBlock = editor.insertBlocks(
+ [{ type: fallbackInlineType as keyof BSchema }],
lastBlock,
"after",
)[0];🤖 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 `@packages/core/src/extensions/SuggestionMenu/getDefaultSlashMenuItems.ts`
around lines 35 - 39, The code currently hard-codes inserting { type:
"paragraph" } in getDefaultSlashMenuItems (used by
insertOrUpdateBlockForSlashMenu), which will break for custom schemas without a
paragraph type; update the fallback to select a valid inline-content block type
from editor.schema.blockSchema (e.g., find the first block whose content model
allows inline content) and use that type in the call to editor.insertBlocks, and
if none exists, throw a clear schema error mentioning
insertOrUpdateBlockForSlashMenu and editor.schema.blockSchema so failure is
explicit.
Comment on lines
+18
to
+21
| const paragraphs = document.querySelectorAll<HTMLElement>( | ||
| '[data-content-type="paragraph"]', | ||
| ); | ||
| await userEvent.click(paragraphs[paragraphs.length - 1]); |
Contributor
There was a problem hiding this comment.
Guard the paragraph fallback before clicking.
If .bn-trailing-block is absent and no paragraph exists, Line 21 clicks undefined and fails non-deterministically. Add an explicit empty-check with a clear error.
Suggested fix
} else {
const paragraphs = document.querySelectorAll<HTMLElement>(
'[data-content-type="paragraph"]',
);
+ if (paragraphs.length === 0) {
+ throw new Error(
+ "copyPaste: no trailing block or paragraph available to focus before paste.",
+ );
+ }
await userEvent.click(paragraphs[paragraphs.length - 1]);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const paragraphs = document.querySelectorAll<HTMLElement>( | |
| '[data-content-type="paragraph"]', | |
| ); | |
| await userEvent.click(paragraphs[paragraphs.length - 1]); | |
| const paragraphs = document.querySelectorAll<HTMLElement>( | |
| '[data-content-type="paragraph"]', | |
| ); | |
| if (paragraphs.length === 0) { | |
| throw new Error( | |
| "copyPaste: no trailing block or paragraph available to focus before paste.", | |
| ); | |
| } | |
| await userEvent.click(paragraphs[paragraphs.length - 1]); |
🤖 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 `@tests/src/utils/copypaste.ts` around lines 18 - 21, Guard the paragraph
fallback by checking that the NodeList referenced by paragraphs has at least one
element before calling userEvent.click; if paragraphs.length === 0 (and
.bn-trailing-block is absent) throw a clear error or fail the test instead of
calling userEvent.click(undefined). Update the code around the paragraphs
variable and the userEvent.click(paragraphs[paragraphs.length - 1]) call to
perform this explicit empty-check and error message.
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.
Summary
setSelectionToNextContentEditableBlockfinds the next block with rich text content, from the block currently containing the text cursor, and moves the cursor to it. The issue is that a block with rich text content doesn't always exist later in the document, leading to situations where e.g. a divider is inserted via the slash menu and selected.This PR makes it so that when this happens, an empty paragraph is added to the end of the document and the text cursor is move to it.
Closes #2817
Rationale
While minor, we generally want to avoid moving the selection to a block without rich text content for better UX.
Changes
setSelectionToNextContentEditableBlock.Impact
setSelectionToNextContentEditableBlockmay also be used by consumers (throughinsertOrUpdateBlockForSlashMenu), so they will be forced to adopt this new behaviour. Should be fine as it is overall an improvement.Testing
Updated e2e tests/snapshots.
Screenshots/Video
N/A
Checklist
Additional Notes
N/A
Summary by CodeRabbit
Bug Fixes
Tests