fix: prevent paste from indenting blocks under the current block#342
Merged
Conversation
Reviewer's GuideAdjusts paste handling in the Slate/Yjs editor so that pasted blocks become siblings at the correct indent level, correctly decodes Slate fragments from clipboard HTML, and fixes fragment conversion so inner text-wrapper nodes are preserved rather than turned into nested blocks, with new e2e coverage for paste indentation behavior. Sequence diagram for updated paste handling with sibling block insertionsequenceDiagram
actor User
participant ReactEditor as ReactEditor_withInsertData
participant Clipboard
participant YjsEditor
participant YjsDoc
User->>ReactEditor: paste
ReactEditor->>Clipboard: data.getData(application/x-slate-fragment)
alt no_native_fragment
ReactEditor->>Clipboard: data.getData(text/html)
ReactEditor->>ReactEditor: extractSlateFragmentFromHTML(html)
end
alt rawFragment_found
ReactEditor->>ReactEditor: decodeSlateFragment(rawFragment)
alt parsed_ok
ReactEditor->>ReactEditor: convertSlateFragmentTo(parsed)
ReactEditor->>YjsEditor: insertFragmentAsSiblings(fragment)
alt inserted_as_siblings
YjsEditor->>YjsDoc: slateContentInsertToYData(parentId,index,fragment,doc)
YjsEditor->>YjsEditor: Transforms.select(Editor.end(...))
else fallback_to_slate
ReactEditor->>ReactEditor: insertFragment(fragment)
end
else malformed_fragment
ReactEditor->>ReactEditor: fallback_to_other_handlers
end
else no_fragment
ReactEditor->>ReactEditor: existing_paste_handlers
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
extractSlateFragmentFromHTMLhelper uses a very narrow regex (data-slate-fragment="(.+?)"), which won’t handle single quotes, escaped quotes, or attributes split across lines; consider either reusingslate-dom’s helper directly or switching to a more robust approach (e.g. DOMParser + attribute read, or a regex that mirrorsslate-dom’s implementation more closely). - In
insertFragmentAsSiblings, theallBlocksguard assumes every fragment node is a block whose first child is aYjsEditorKey.textwrapper; as more block/inline shapes are introduced this may cause unexpected fallbacks toinsertFragment—it might be safer to either log when this guard fails or broaden the shape handling to explicitly support common non-text-wrapper cases (e.g. void blocks). - In
convertSlateFragmentTo,Object.values(BlockType).includes(type)is evaluated for every node; consider precomputing aSetof valid block types or a predicate function to avoid repeated array scans in large fragments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `extractSlateFragmentFromHTML` helper uses a very narrow regex (`data-slate-fragment="(.+?)"`), which won’t handle single quotes, escaped quotes, or attributes split across lines; consider either reusing `slate-dom`’s helper directly or switching to a more robust approach (e.g. DOMParser + attribute read, or a regex that mirrors `slate-dom`’s implementation more closely).
- In `insertFragmentAsSiblings`, the `allBlocks` guard assumes every fragment node is a block whose first child is a `YjsEditorKey.text` wrapper; as more block/inline shapes are introduced this may cause unexpected fallbacks to `insertFragment`—it might be safer to either log when this guard fails or broaden the shape handling to explicitly support common non-text-wrapper cases (e.g. void blocks).
- In `convertSlateFragmentTo`, `Object.values(BlockType).includes(type)` is evaluated for every node; consider precomputing a `Set` of valid block types or a predicate function to avoid repeated array scans in large fragments.
## Individual Comments
### Comment 1
<location path="src/components/editor/utils/fragment.ts" line_range="550-542" />
<code_context>
}
- return null;
+ const mappedChildren = node.children
+ .map(traverse)
+ .filter(Boolean) as SlateNode[];
+
+ const hasTextWrapper =
+ mappedChildren.length > 0 &&
+ SlateElement.isElement(mappedChildren[0] as SlateNode) &&
+ (mappedChildren[0] as SlateElement).type === YjsEditorKey.text;
+
+ const blockChildren = hasTextWrapper
+ ? mappedChildren
+ : [
+ {
+ textId: blockId,
</code_context>
<issue_to_address>
**issue (bug_risk):** Blocks may end up with mixed element/text children, which can violate the expected block schema.
Because `traverse` now returns raw `SlateText` for text nodes and `SlateElement` for elements, `mappedChildren` can contain both. In the `hasTextWrapper === false` branch, you prepend a synthetic text wrapper but still spread `...mappedChildren`, which can leave raw `SlateText` nodes as direct block children alongside the wrapper. Many Slate/Yjs schemas assume blocks only have element children, so this mixed structure may break downstream logic. Consider normalizing `mappedChildren` here (e.g. wrapping stray text nodes) or enforcing that only elements are present before building `blockChildren`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| }, | ||
| ...children, | ||
| ]; | ||
| const blockId = generateBlockId(); |
There was a problem hiding this comment.
issue (bug_risk): Blocks may end up with mixed element/text children, which can violate the expected block schema.
Because traverse now returns raw SlateText for text nodes and SlateElement for elements, mappedChildren can contain both. In the hasTextWrapper === false branch, you prepend a synthetic text wrapper but still spread ...mappedChildren, which can leave raw SlateText nodes as direct block children alongside the wrapper. Many Slate/Yjs schemas assume blocks only have element children, so this mixed structure may break downstream logic. Consider normalizing mappedChildren here (e.g. wrapping stray text nodes) or enforcing that only elements are present before building blockChildren.
`convertSlateFragmentTo` treated Slate's inner text-wrapper (type:'text') as a regular block, wrapping every pasted block in an extra Paragraph and causing each paste to be indented one level. Slate's `insertFragment` then nested those blocks under the cursor's parent, which also broke Backspace after deleting the parent. Fix paste in three places: - `convertSlateFragmentTo`: preserve text-wrapper nodes as-is, only treat real BlockType elements as blocks. - `withInsertData`: recover the Slate fragment from the `data-slate-fragment` HTML attribute when the `application/x-slate-fragment` clipboard MIME entry is missing (matches slate-dom's regex). Wrap decode in try/catch so malformed clipboard data falls through to the text/HTML handlers. - New `insertFragmentAsSiblings` writes pasted blocks directly to the YJS doc as siblings of the current block, mirroring `Transforms.insertFragment` semantics: deletes expanded selection first, replaces the current block if empty, and places the cursor at the end of the last inserted block. Adds a Playwright spec covering the indent regression, the backspace-after-delete follow-up, and paste-over-selection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
appflowy
force-pushed
the
fix/paste-indent-and-selection
branch
from
a7da7ae to
4cde60d
Compare
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
Pasting content copied from within the editor produced blocks that were nested one level under the cursor's parent — e.g. typing
1,2,3,4then select-all → copy → Enter → paste rendered the pasted lines indented under4. Deleting that parent then broke Backspace because the empty parent still held nested children.Three root causes were fixed:
convertSlateFragmentTotreated Slate's inner text-wrapper element (type: 'text') as a regular block. Since'text'isn't in theBlockTypeenum it was rewritten into a nestedParagraph, double-wrapping every pasted block.application/x-slate-fragmentMIME entry; Slate's ownslate-domfalls back to a regex ondata-slate-fragmentin the HTML. We weren't doing that, so paste went through the HTML-parser path with stale shape.insertFragmentnests the fragment when the cursor sits inside a text-wrapper (deep path). A newinsertFragmentAsSiblingswrites pasted blocks directly to the YJS doc as siblings of the current block.The sibling-insert path also mirrors
Transforms.insertFragmentsemantics: deletes the expanded selection first, replaces the current block if empty, and places the cursor at the end of the last inserted block.Test plan
playwright/e2e/editor/blocks/paste_indent.spec.ts— 3 cases (indent regression, Backspace-after-delete, paste-over-selection)playwright/e2e/editor/blocks/tests pass (12/12) — no regression in code-block paste, scroll, or unsupported-block tests🤖 Generated with Claude Code
Summary by Sourcery
Fix pasting Slate fragments so blocks are inserted as siblings at the correct indent level instead of becoming nested under the current block.
Bug Fixes:
Enhancements:
insertFragmentsemantics for a consistent editing experience.Tests: