fix: cannot copy image when it is the first/only block in editor (#2016)#2672
fix: cannot copy image when it is the first/only block in editor (#2016)#2672yamiletpineda wants to merge 3 commits intoTypeCellOS:mainfrom
Conversation
|
@Rn123456789 is attempting to deploy a commit to the TypeCell Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe changes implement proper selection and clipboard handling for non-editable content (images) when using select-all. A Changes
Sequence DiagramsequenceDiagram
participant User
participant KeyboardHandler as Mod-a Handler
participant SelectionSystem as Selection System
participant ClipboardAPI as Clipboard API
participant HTMLSerializer as Clipboard Serializer
User->>KeyboardHandler: Press Ctrl+A
KeyboardHandler->>SelectionSystem: Create AllSelection(pos 0)
SelectionSystem->>SelectionSystem: Dispatch transaction
User->>ClipboardAPI: Press Ctrl+C
ClipboardAPI->>HTMLSerializer: Get selection content
HTMLSerializer->>HTMLSerializer: Detect blockGroup wrapper
HTMLSerializer->>HTMLSerializer: Unwrap fragment
HTMLSerializer->>HTMLSerializer: Construct Slice from unwrapped content
HTMLSerializer->>ClipboardAPI: Return serialized HTML
ClipboardAPI->>User: Copy to clipboard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/src/end-to-end/copypaste/copypaste.test.ts`:
- Around line 191-213: Add a new end-to-end test that mirrors the existing
"Image as first block should be able to be copied using Ctrl+A" but inserts a
text block after the image to cover the "image first, text follows" regression;
use the same helpers (focusOnEditor, executeSlashCommand to insert the image via
IMAGE_EMBED_URL, waitForSelector `img[src="${IMAGE_EMBED_URL}"]`), then insert
or type a text block after the image (e.g., create a paragraph block via
keyboard or executeSlashCommand), call copyPasteAll(page) and assert with
compareDocToSnapshot using a new snapshot name like "imageThenText.json" to lock
in mixed-content behavior.
🪄 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: 49a2dc8f-0476-4b92-abf9-57c6b96354a1
📒 Files selected for processing (3)
packages/core/src/api/clipboard/toClipboard/copyExtension.tspackages/core/src/extensions/tiptap-extensions/KeyboardShortcuts/KeyboardShortcutsExtension.tstests/src/end-to-end/copypaste/copypaste.test.ts
| test("Image as first block should be able to be copied using Ctrl+A", async ({ | ||
| page, | ||
| browserName, | ||
| }) => { | ||
| test.skip( | ||
| browserName === "firefox" || browserName === "webkit", | ||
| "Firefox doesn't yet support the async clipboard API. Webkit copy/paste stopped working after updating to Playwright 1.33.", | ||
| ); | ||
|
|
||
| await focusOnEditor(page); | ||
|
|
||
| const IMAGE_EMBED_URL = "https://placehold.co/800x540.png"; | ||
| await executeSlashCommand(page, "image"); | ||
|
|
||
| await page.click(`[data-test="embed-tab"]`); | ||
| await page.click(`[data-test="embed-input"]`); | ||
| await page.keyboard.type(IMAGE_EMBED_URL); | ||
| await page.click(`[data-test="embed-input-button"]`); | ||
| await page.waitForSelector(`img[src="${IMAGE_EMBED_URL}"]`); | ||
|
|
||
| await copyPasteAll(page); | ||
|
|
||
| await compareDocToSnapshot(page, "imageAsFirstBlock.json"); |
There was a problem hiding this comment.
Add coverage for the “image first, text follows” case.
This test covers the only-image scenario, but issue #2016 also reports that when text follows the leading image, Ctrl+A/C copies only the text. Please add a mixed-content case so the fix is locked down for both reported regressions.
Suggested additional coverage
+ test("Image as first block followed by text should be copied using Ctrl+A", async ({
+ page,
+ browserName,
+ }) => {
+ test.skip(
+ browserName === "firefox" || browserName === "webkit",
+ "Firefox doesn't yet support the async clipboard API. Webkit copy/paste stopped working after updating to Playwright 1.33.",
+ );
+
+ await focusOnEditor(page);
+
+ const IMAGE_EMBED_URL = "https://placehold.co/800x540.png";
+ await executeSlashCommand(page, "image");
+
+ await page.click(`[data-test="embed-tab"]`);
+ await page.click(`[data-test="embed-input"]`);
+ await page.keyboard.type(IMAGE_EMBED_URL);
+ await page.click(`[data-test="embed-input-button"]`);
+ await page.waitForSelector(`img[src="${IMAGE_EMBED_URL}"]`);
+
+ await page.keyboard.press("ArrowDown");
+ await page.keyboard.press("Enter");
+ await page.keyboard.type("paragraph after image");
+
+ await copyPasteAll(page);
+
+ await compareDocToSnapshot(page, "imageAsFirstBlockWithText.json");
+ });📝 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.
| test("Image as first block should be able to be copied using Ctrl+A", async ({ | |
| page, | |
| browserName, | |
| }) => { | |
| test.skip( | |
| browserName === "firefox" || browserName === "webkit", | |
| "Firefox doesn't yet support the async clipboard API. Webkit copy/paste stopped working after updating to Playwright 1.33.", | |
| ); | |
| await focusOnEditor(page); | |
| const IMAGE_EMBED_URL = "https://placehold.co/800x540.png"; | |
| await executeSlashCommand(page, "image"); | |
| await page.click(`[data-test="embed-tab"]`); | |
| await page.click(`[data-test="embed-input"]`); | |
| await page.keyboard.type(IMAGE_EMBED_URL); | |
| await page.click(`[data-test="embed-input-button"]`); | |
| await page.waitForSelector(`img[src="${IMAGE_EMBED_URL}"]`); | |
| await copyPasteAll(page); | |
| await compareDocToSnapshot(page, "imageAsFirstBlock.json"); | |
| test("Image as first block should be able to be copied using Ctrl+A", async ({ | |
| page, | |
| browserName, | |
| }) => { | |
| test.skip( | |
| browserName === "firefox" || browserName === "webkit", | |
| "Firefox doesn't yet support the async clipboard API. Webkit copy/paste stopped working after updating to Playwright 1.33.", | |
| ); | |
| await focusOnEditor(page); | |
| const IMAGE_EMBED_URL = "https://placehold.co/800x540.png"; | |
| await executeSlashCommand(page, "image"); | |
| await page.click(`[data-test="embed-tab"]`); | |
| await page.click(`[data-test="embed-input"]`); | |
| await page.keyboard.type(IMAGE_EMBED_URL); | |
| await page.click(`[data-test="embed-input-button"]`); | |
| await page.waitForSelector(`img[src="${IMAGE_EMBED_URL}"]`); | |
| await copyPasteAll(page); | |
| await compareDocToSnapshot(page, "imageAsFirstBlock.json"); | |
| }); | |
| test("Image as first block followed by text should be copied using Ctrl+A", async ({ | |
| page, | |
| browserName, | |
| }) => { | |
| test.skip( | |
| browserName === "firefox" || browserName === "webkit", | |
| "Firefox doesn't yet support the async clipboard API. Webkit copy/paste stopped working after updating to Playwright 1.33.", | |
| ); | |
| await focusOnEditor(page); | |
| const IMAGE_EMBED_URL = "https://placehold.co/800x540.png"; | |
| await executeSlashCommand(page, "image"); | |
| await page.click(`[data-test="embed-tab"]`); | |
| await page.click(`[data-test="embed-input"]`); | |
| await page.keyboard.type(IMAGE_EMBED_URL); | |
| await page.click(`[data-test="embed-input-button"]`); | |
| await page.waitForSelector(`img[src="${IMAGE_EMBED_URL}"]`); | |
| await page.keyboard.press("ArrowDown"); | |
| await page.keyboard.press("Enter"); | |
| await page.keyboard.type("paragraph after image"); | |
| await copyPasteAll(page); | |
| await compareDocToSnapshot(page, "imageAsFirstBlockWithText.json"); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/src/end-to-end/copypaste/copypaste.test.ts` around lines 191 - 213, Add
a new end-to-end test that mirrors the existing "Image as first block should be
able to be copied using Ctrl+A" but inserts a text block after the image to
cover the "image first, text follows" regression; use the same helpers
(focusOnEditor, executeSlashCommand to insert the image via IMAGE_EMBED_URL,
waitForSelector `img[src="${IMAGE_EMBED_URL}"]`), then insert or type a text
block after the image (e.g., create a paragraph block via keyboard or
executeSlashCommand), call copyPasteAll(page) and assert with
compareDocToSnapshot using a new snapshot name like "imageThenText.json" to lock
in mixed-content behavior.
nperez0111
left a comment
There was a problem hiding this comment.
@yamiletpineda it seems that you've submitted two PRs that have overlapping changes & seem to identify some root issue with the copy behavior not actually containing all of the content that it should. Can you please close each of these & merge into 1 PR please.
I'm not sure about your use of the AllSelection here, and why it'd be any different from the existing keybind for it.
On your other PR you touch the column behavior which I'm also not certain that I understand why it was changed. Can you please explain them
Summary
Fixes #2016. Image cannot be copied using
Ctrl+AandCtrl+Ckeyboard shortcuts when it is the first or only block in the editor.Rationale
A user should be able to copy an image using the appropriate keyboard commands. As this is a common user action, the unexpected behavior is disruptive to user workflows.
Changes
mod-akeyboard shortcut handler inKeyboardShortcutsExtension.tsthat forces an AllSelection covering the entire document from position 0. Without this, theCtrl+Akeyboard command created a TextSelection starting at position 6, skipping the first block when it was a non-editable node (e.g. image).blockGroupfrom the selected fragment before building clipboard data incopyExtension.ts, preventing nested blocks when pasting.Impact
Ctrl+Anow selects all blocks including non-editable ones at the start of the document.Testing
copypaste.test.ts.blockGroupnode.Screenshots/Video
copy-image-keyboard-shortcut.mov
Checklist
Additional Notes
Ctrl+A. Follow-up CSS work needed.pnpm playwright test copypastefor the first time.Summary by CodeRabbit
New Features
Bug Fixes
Tests