-
Notifications
You must be signed in to change notification settings - Fork 0
feat: full layout copy and paste #930
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
commit: |
WalkthroughAdds copy/paste layout UI and localized strings across many visual-editor locale files. LayoutHeader now implements "Copy Layout" (serializes current app state and writes JSON to the clipboard) and "Paste Layout" (reads clipboard, parses JSON, validates required fields, runs migrations via the migration registry with streamDocument, then dispatches setData/append history). InternalLayoutEditor replaced hard-coded paste error alerts with localized messages. Error handling covers parse, validation, and migration failures. Sequence Diagram(s)sequenceDiagram
participant User
participant LayoutHeader
participant Clipboard
participant MigrationRegistry as Migration
participant StreamDocument as Doc
participant History
User->>LayoutHeader: Click "Copy Layout"
LayoutHeader->>LayoutHeader: JSON.stringify(appState.data)
LayoutHeader->>Clipboard: writeText(JSON)
Clipboard-->>LayoutHeader: success / failure
alt success
LayoutHeader-->>User: (no alert) or implicit success
else failure
LayoutHeader-->>User: alert(failedToCopyLayout)
end
User->>LayoutHeader: Click "Paste Layout"
LayoutHeader->>Clipboard: readText()
Clipboard-->>LayoutHeader: text
LayoutHeader->>LayoutHeader: try JSON.parse(text)
alt parse error
LayoutHeader-->>User: alert(failedToPasteLayoutInvalidData)
else parsed
LayoutHeader->>LayoutHeader: validate required fields (root, content)
alt invalid component/layout data
LayoutHeader-->>User: alert(failedToPasteComponentInvalidData)
else valid
LayoutHeader->>Migration: migrate(parsedData)
Migration->>Doc: migrateWithStreamDocument(parsedData, config)
Doc-->>Migration: migratedData
Migration-->>LayoutHeader: migratedData
LayoutHeader->>History: dispatch setData / append history entry
History-->>LayoutHeader: updated state
LayoutHeader-->>User: new layout applied / success
end
end
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (2)
117-129: Handle clipboard write failures explicitly in Copy Layout
navigator.clipboard.writeTextcan fail (e.g., HTTP origin, denied permission), and the handler neither awaits nor reports errors. Making the handlerasyncand adding minimal error handling would avoid silent failures and confusing UX.Example:
- <Button - variant="outline" - onClick={() => { - const { appState } = getPuck(); - - navigator.clipboard.writeText( - JSON.stringify(appState.data, null, 2) - ); - }} + <Button + variant="outline" + onClick={async () => { + const { appState } = getPuck(); + if (!navigator.clipboard) { + alert("Copy failed: Clipboard API is not available in this browser."); + return; + } + + try { + await navigator.clipboard.writeText( + JSON.stringify(appState.data, null, 2) + ); + } catch { + alert("Copy failed: Unable to write to clipboard."); + } + }}
22-22: Clipboard paste flow works; consider tightening validation, history safety, and i18nThe overall paste flow (read clipboard → parse → validate shape → migrate with
migrationRegistry+useDocument→ push history + advance index) is sound, but a few small tweaks would make it more robust and user-friendly:
Distinguish permission/technical failures from bad data
navigator.clipboard.readText()andJSON.parseerrors are currently surfaced as “Invalid component data.” This is misleading when the failure is due to permissions or protocol. Consider checkingnavigator.clipboardfirst and using a more generic message (or separate messages) for read/parse failures vs. structural validation.Guard against empty histories before indexing
histories[histories.length - 1]assumes at least one entry. If that invariant ever breaks, paste will throw instead of falling back gracefully. A defensive guard keeps this safe:const newHistory = [...histories,{state: {ui: histories[histories.length - 1].state.ui,data: migratedPastedData,},},];
const lastUi =histories[histories.length - 1]?.state.ui ?? {};const newHistory = [...histories,{state: {ui: lastUi,data: migratedPastedData,},},];
- Localize error messages
The twoalert("Failed to paste: Invalid component data.");calls are hard-coded English. Since the rest of the header already usespt(...), consider introducing a translation key (e.g.,pasteLayoutErrorInvalidData) and usingpthere to keep UX consistent across locales.These are all incremental; the existing behavior is functionally correct for well-formed clipboard data.
Also applies to: 44-44, 130-177
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
packages/visual-editor/locales/cs/visual-editor.json(2 hunks)packages/visual-editor/locales/da/visual-editor.json(2 hunks)packages/visual-editor/locales/de/visual-editor.json(2 hunks)packages/visual-editor/locales/en-GB/visual-editor.json(2 hunks)packages/visual-editor/locales/en/visual-editor.json(2 hunks)packages/visual-editor/locales/es/visual-editor.json(2 hunks)packages/visual-editor/locales/et/visual-editor.json(2 hunks)packages/visual-editor/locales/fi/visual-editor.json(2 hunks)packages/visual-editor/locales/fr/visual-editor.json(2 hunks)packages/visual-editor/locales/hr/visual-editor.json(2 hunks)packages/visual-editor/locales/hu/visual-editor.json(2 hunks)packages/visual-editor/locales/it/visual-editor.json(2 hunks)packages/visual-editor/locales/ja/visual-editor.json(2 hunks)packages/visual-editor/locales/lt/visual-editor.json(2 hunks)packages/visual-editor/locales/lv/visual-editor.json(2 hunks)packages/visual-editor/locales/nb/visual-editor.json(2 hunks)packages/visual-editor/locales/nl/visual-editor.json(2 hunks)packages/visual-editor/locales/pl/visual-editor.json(2 hunks)packages/visual-editor/locales/pt/visual-editor.json(2 hunks)packages/visual-editor/locales/ro/visual-editor.json(2 hunks)packages/visual-editor/locales/sk/visual-editor.json(2 hunks)packages/visual-editor/locales/sv/visual-editor.json(2 hunks)packages/visual-editor/locales/tr/visual-editor.json(2 hunks)packages/visual-editor/locales/zh-TW/visual-editor.json(2 hunks)packages/visual-editor/locales/zh/visual-editor.json(2 hunks)packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx(3 hunks)
🔇 Additional comments (25)
packages/visual-editor/locales/et/visual-editor.json (1)
135-135: Localization additions approved for Estonian.The two new keys are correctly translated and properly positioned in alphabetical order. JSON syntax is valid.
Also applies to: 555-555
packages/visual-editor/locales/nl/visual-editor.json (1)
135-135: Localization additions approved for Dutch.Translations are accurate and follow Dutch conventions (hyphenated "lay-out"). Placement and formatting are consistent with the Estonian locale additions.
Also applies to: 555-555
packages/visual-editor/locales/ro/visual-editor.json (1)
135-135: Localization additions approved for Romanian.Translations correctly use formal verb conjugation ("Copiați," "Lipiți"). Placement and structure align with other locale additions.
Also applies to: 555-555
packages/visual-editor/locales/en-GB/visual-editor.json (1)
134-134: Localization additions approved for British English.Translations use standard English with consistent title-case capitalization matching existing patterns. Line number variance from other locales is expected.
Also applies to: 554-554
packages/visual-editor/locales/lv/visual-editor.json (1)
135-135: Localization additions approved for Latvian.Latvian translations are linguistically correct with proper diacritical marks preserved. Placement and structure are consistent.
Also applies to: 555-555
packages/visual-editor/locales/cs/visual-editor.json (1)
138-138: Localization additions approved for Czech.Czech translations use infinitive forms appropriately and preserve diacritical marks correctly. Line number offset reflects content ordering in this locale variant.
Also applies to: 562-562
packages/visual-editor/locales/pt/visual-editor.json (1)
135-135: Localization additions approved for Portuguese.Portuguese translations use appropriate infinitive forms. "Layout" is retained as an English loanword, which is standard in Portuguese UI terminology. Placement and structure are consistent.
Also applies to: 555-555
packages/visual-editor/locales/fi/visual-editor.json (1)
135-135: Localization additions approved for Finnish.Finnish translations use imperative verb forms, which is appropriate for button labels. Placement and structure align with other locale additions.
Also applies to: 555-555
packages/visual-editor/locales/da/visual-editor.json (1)
138-138: Locale keys properly added and positioned ✓The
copyLayoutandpasteLayoutkeys are correctly placed in alphabetical order with appropriate Danish translations. No issues detected.Also applies to: 562-562
packages/visual-editor/locales/ja/visual-editor.json (1)
135-135: Locale keys properly added and positioned ✓The
copyLayoutandpasteLayoutkeys are correctly placed with appropriate Japanese translations using proper linguistic forms. No issues detected.Also applies to: 555-555
packages/visual-editor/locales/tr/visual-editor.json (1)
135-135: Locale keys properly added and positioned ✓The
copyLayoutandpasteLayoutkeys are correctly placed with appropriate Turkish translations using imperative verb forms. No issues detected.Also applies to: 555-555
packages/visual-editor/locales/zh-TW/visual-editor.json (1)
135-135: Locale keys properly added and positioned ✓The
copyLayoutandpasteLayoutkeys are correctly placed with appropriate Traditional Chinese translations. No issues detected.Also applies to: 555-555
packages/visual-editor/locales/it/visual-editor.json (1)
135-135: Locale keys properly added and positioned ✓The
copyLayoutandpasteLayoutkeys are correctly placed with appropriate Italian translations using imperative verb forms. No issues detected.Also applies to: 555-555
packages/visual-editor/locales/zh/visual-editor.json (1)
135-135: Locale keys properly added and positioned ✓The
copyLayoutandpasteLayoutkeys are correctly placed with appropriate Simplified Chinese translations. No issues detected.Also applies to: 555-555
packages/visual-editor/locales/lt/visual-editor.json (1)
135-135: Locale keys properly added and positioned ✓The
copyLayoutandpasteLayoutkeys are correctly placed with appropriate Lithuanian translations using infinitive verb forms. No issues detected.Also applies to: 555-555
packages/visual-editor/locales/fr/visual-editor.json (1)
134-134: Locale keys properly added and positioned ✓The
copyLayoutandpasteLayoutkeys are correctly placed with appropriate French translations using the standard French phrase "mise en page" for layout. No issues detected.Also applies to: 554-554
packages/visual-editor/locales/de/visual-editor.json (1)
137-137: Localization strings properly added for German locale. The two new translation keys (copyLayoutandpasteLayout) are correctly positioned in alphabetical order within the root-level translation map with appropriate German translations ("Layout kopieren" and "Layout einfügen"). ✓Also applies to: 560-560
packages/visual-editor/locales/en/visual-editor.json (1)
134-134: Localization strings properly added for English locale. The two new keys (copyLayoutandpasteLayout) follow the same alphabetical ordering as the German locale, with clear, user-friendly translations ("Copy Layout" and "Paste Layout"). ✓Also applies to: 554-554
packages/visual-editor/locales/es/visual-editor.json (1)
134-134: Localization strings properly added for Spanish locale. The translations ("Copiar diseño" and "Pegar diseño") are grammatically correct and follow the established alphabetical ordering pattern. ✓Also applies to: 554-554
packages/visual-editor/locales/pl/visual-editor.json (1)
135-135: Localization strings properly added for Polish locale. The translations ("Skopiuj układ" and "Wklej układ") are linguistically appropriate and positioned consistently with other locales in the PR. ✓Also applies to: 556-556
packages/visual-editor/locales/sk/visual-editor.json (1)
135-135: Localization strings properly added for Slovak locale. The translations ("Kopírovať rozloženie" and "Prilepiť rozloženie") follow the established pattern and are grammatically sound. ✓Also applies to: 556-556
packages/visual-editor/locales/hr/visual-editor.json (1)
135-135: Localization strings properly added for Croatian locale. The translations ("Kopiraj izgled" and "Zalijepi izgled") are linguistically appropriate and positioned correctly within the translation map. ✓Also applies to: 555-555
packages/visual-editor/locales/hu/visual-editor.json (1)
135-135: Localization strings properly added for Hungarian locale. The translations ("Elrendezés másolása" and "Elrendezés beillesztése") are grammatically correct and follow the consistent alphabetical ordering pattern across all locales. ✓Also applies to: 555-555
packages/visual-editor/locales/nb/visual-editor.json (1)
135-135: Localization strings properly added for Norwegian Bokmål locale. The translations ("Kopier layout" and "Lim inn layout") are linguistically sound and positioned consistently with the broader set of locale updates in this PR. ✓Also applies to: 555-555
packages/visual-editor/locales/sv/visual-editor.json (1)
135-135: Copy/Paste layout translations look consistentThe new
copyLayout/pasteLayoutkeys are named consistently with usage inLayoutHeaderand the Swedish strings align with existing clipboard-related labels.Also applies to: 556-556
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (1)
136-171: Consider more specific error handling for debugging.The catch block handles all errors uniformly—JSON parse errors, migration failures, and clipboard access issues all show the same generic message. While this simplifies UX, it can make debugging difficult for users and developers.
Consider differentiating error types:
onClick={async () => { try { const { dispatch, config } = getPuck(); const rawClipboardText = await navigator.clipboard.readText(); + let pastedData; + try { - const pastedData = JSON.parse(rawClipboardText); + pastedData = JSON.parse(rawClipboardText); + } catch { + alert( + pt( + "failedToPasteLayoutInvalidData", + "Failed to paste: Invalid layout data." + ) + ); + return; + } if ( !pastedData || typeof pastedData !== "object" || pastedData.root === undefined || pastedData.content === undefined ) { alert( pt( "failedToPasteLayoutInvalidData", "Failed to paste: Invalid layout data." ) ); return; } const migratedPastedData = migrate( pastedData, migrationRegistry, config, streamDocument ); dispatch({ type: "setData", data: migratedPastedData, }); } catch { alert(pt("failedToPasteLayout", "Failed to paste layout.")); - return; } }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (3)
packages/visual-editor/src/components/atoms/button.tsx (1)
Button(89-115)packages/visual-editor/src/utils/migrate.ts (1)
migrate(44-105)packages/visual-editor/src/components/migrations/migrationRegistry.ts (1)
migrationRegistry(52-97)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: call_unit_test / unit_tests (20.x)
- GitHub Check: call_unit_test / unit_tests (18.x)
- GitHub Check: semgrep/ci
- GitHub Check: create-dev-release
🔇 Additional comments (5)
packages/visual-editor/src/internal/puck/components/LayoutHeader.tsx (5)
22-22: LGTM!The import is necessary for obtaining the
streamDocumentinstance used during migration of pasted layout data.
44-44: LGTM!Correctly initializes
streamDocumentfor use in the paste operation's migration flow.
117-133: LGTM!The copy layout implementation correctly serializes the current state to the clipboard with proper error handling and localization.
176-180: LGTM!The separator appropriately groups the copy/paste buttons and follows the existing separator pattern used elsewhere in the header.
164-167: Thedispatch({ type: "setData" })action automatically updates history via the Puck library's built-in history management.The code is correct as-is. The
setDatadispatch triggers Puck's internal history tracking—the library automatically captures state changes, and the reactivehistoriesandindexselectors in LayoutHeader (lines 49-51) update accordingly, which triggers theonHistoryChangecallback via the useEffect hook (lines 56-58). This is the standard pattern for history management in Puck and matches the usage in InternalLayoutEditor.tsx.The catch block (lines 168-171) does suppress specific error details (JSON.parse errors, clipboard failures, migration errors), but the implementation is functionally correct.
Screen.Recording.2025-12-03.at.10.46.23.AM.mov
https://dev.yext.com/s/1000152098/yextsites/66304/pagesets
The figma shows the button being greyed out if the clipboard data is invalid, however this isn't possible due to browser restrictions on passively reading the clipboard