Add slides layout change UI with placeholder identity#197
Merged
Conversation
The slides package already exposes applyLayout in the store API but no UI surfaces it, and the current implementation is additive-only (placeholder accumulation across switches). The design adds placeholder identity tracking on Element so layout switches preserve typed content by slot type and demote orphans instead of dropping them. The plan splits the work into eleven TDD-driven tasks shipped as a single PR. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Lays groundwork for layout-change UX where slot identity must survive layout switches. Without a ref on the element, we cannot tell user-added content from layout-generated placeholders, and orphan handling on layout downgrade has no signal to act on. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Replaces `as never` Block casts with a typed `textBlock(text)` helper so future changes to Block can no longer silently bypass type-checking in this test file. Adds three edges that pin the contract: empty blocks array (vacuous-true), block with empty inlines (vacuous-true), and multi-block where one is non-empty. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Required for type-first slot matching when applying a new layout. Without this, switching from title-body to title-only could route the body's text into the new title slot purely by index, which would surprise the author. Eleven slot mappings are kept in one place to keep the spec/code in sync. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Single source of truth for layout-change semantics so MemSlidesStore and YorkieSlidesStore can never drift. The three-pass algorithm (partition / type-first match / demote orphans) preserves typed content across slot-count changes and never destroys non-empty elements; users rely on undo, not confirm dialogs. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
addSlide now stamps placeholderRef on each generated element so a later applyLayout can match by slot identity. applyLayout itself delegates to the shared pure helper, replacing the conservative additive-only body with type-first matching and orphan demote. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
addSlide and applyLayoutToSlide both compute the (type, index) identity for every layout placeholder. Keeping two copies of the formula creates a drift hazard — divergence would silently misroute typed content on layout switches. The shared helper also collapses the per-call O(n^2) slice+filter into a one-pass Map counter at the cost of one extra import. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Both stores now share the slot-matching pure function so collab clients see identical local-first results. The Yorkie variant runs inside this.doc.update; applyLayoutToSlide switched to in-place elements.splice so the same body works on both plain arrays and Yorkie array proxies. Yorkie element types gain an optional placeholderRef to match the model. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
A two-client scenario in the existing slides integration suite asserts that simultaneous applyLayout calls on the same slide converge to one layoutId via last-writer-wins. Without this gate, a future change to mutation patterns inside applyLayoutToSlide could silently break collab parity for layout switches. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
When `slide.elements` is a Yorkie array proxy, the previous
implementation crashed in two compounding ways:
1. `{...reuse, frame: ..., placeholderRef: ...}` spread the proxy and
captured non-serializable proxy methods (toJSON), which Yorkie's
subsequent splice rejected with "Unsupported type of value:
function".
2. Even with the spread removed, re-`splice`-ing the same proxy back
into the array fails because Yorkie's CRDTArray proxy is not
recognized as an Array (`Array.isArray` returns false on proxies),
so `buildCRDTElement` falls through to `ObjectProxy.buildObjectMembers`,
which `Object.entries(...)`s the proxy and surfaces the CRDTArray's
internal `elements` field — a function — and throws.
The new shape decides each existing element's fate (reuse / demote /
delete / leave-alone) without removing it, mutates surviving entries
in place via per-field assignment (which Yorkie ObjectProxy turns into
proper CRDT operations), removes dead orphans via single-element
splices from highest index down, and pushes only fresh-from-spec
elements (plain JSON, safe to splice in). MemSlidesStore behavior is
unchanged; previously-broken Yorkie applyLayout now works.
T6's "edit + applyLayout" integration case re-enabled to pin the
regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Reuses renderThumbnail against a synthetic slide so picker cells reflect the active theme without a dedicated icon set per theme. The module-level cache is keyed by (theme, master, layout, size); theme switches naturally route to different keys, so old entries become GC-eligible without an explicit invalidation API. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
v1.0 layouts ship with empty staticElements arrays so the omission was invisible, but v1.5 will populate them with decorative dividers, page numbers, and footer text per the spec. Including them now means picker cells reflect the full layout the user is choosing, not just the placeholder skeleton. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Promotes the JSON deep-clone helper from store/memory.ts into model/clone.ts so addSlide, applyLayoutToSlide, and any future caller share a single implementation. Verified renderThumbnail and its callees never mutate slide/element state, so the synthetic-slide clones in layout-preview were dead defensive copies; removing them shaves work per cache miss and aligns the code with reality. The comment justifying the clones was also factually wrong. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The slides package stays framework-free, so the picker mounts a plain div with a 4-column canvas grid. Both upcoming call sites (split-button on the thumbnail panel, slide context menu) reuse this single API rather than each rolling their own popover. Esc and outside-click resolve to onClose without firing onPick. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Plain click still adds a blank slide so existing muscle memory keeps working; the new dropdown zone opens the layout picker so authors discover the eleven built-in layouts without leaving the rail. Picker onPick routes through the same store.batch the plain click uses, so undo collapses to one step. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Right-clicking the slide background now offers "Change layout…"
seeded with the current slide's layoutId so authors see what
they have selected. Pick fires applyLayout inside store.batch so
undo collapses to one step. The lastContext{X,Y} viewport coords
are captured at contextmenu time so the picker anchors at the
click position rather than at logical (slide) coords.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Records the surprises that came out of the eleven-task implementation — chief among them the Yorkie array-proxy spread bug that broke a naive "build new array, reassign" applyLayout. Encoding the lesson makes the next agent jump straight to the splice-based pattern instead of repeating the discovery from scratch. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (21)
📝 WalkthroughWalkthroughThis PR implements end-to-end slide layout switching with placeholder identity preservation. The feature enables authors to change slide layouts while preserving element content and identity through typed slot matching. Changes span model types, a shared pure layout application algorithm, store integration, canvas preview rendering with caching, vanilla-DOM picker UI, and editor wiring with comprehensive tests and documentation. ChangesSlides Layout Switching Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 |
Contributor
Verification: verify:selfResult: ✅ PASS in 159.2s
Verification: verify:integrationResult: ✅ PASS |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/frontend/src/app/slides/yorkie-slides-store.ts (1)
502-532:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
rebuildSlidedropsplaceholderRef, silently breaking placeholder identity on slide reorder.Both branches reconstruct each element with only
id,type,frame, anddata—placeholderRefis not propagated. SincerebuildSlideis the only path throughmoveSlide(Line 469) andmoveSlides(Line 483), any user drag-reorder erases placeholder identity.moveSlidesis especially destructive because it rebuilds every slide in the deck, not just the moved ones (Lines 482‑486), so a single bulk reorder strips the ref from every element.Once
placeholderRefis gone, the nextapplyLayouton that slide treats former placeholders as user-added — content no longer follows its slot, and orphan demotion no longer applies. Integration tests underapplyLayoutcover the live-proxy path and don't exercise reorder-then-apply, so this slips through CI.The same omission exists in
replaceRoot's text branch (Lines 338‑345). It's currently masked because the snapshot comes fromread(), which (perlessons.md§2) already strips the field — but fixingread()alone won’t fixreplaceRoot; both reconstructions need the propagation.🛠️ Proposed fix — propagate placeholderRef through both reconstructions
private rebuildSlide(src: YorkieSlide): YorkieSlide { const background = yorkieToPlain<YorkieSlide['background']>((src as { background: unknown }).background); const layoutId = (src as { layoutId: string }).layoutId; const id = (src as { id: string }).id; const elements = ((src as { elements: unknown[] }).elements ?? []).map((e) => { - const el = e as { id: string; type: string; frame: unknown; data: unknown }; + const el = e as { + id: string; + type: string; + frame: unknown; + data: unknown; + placeholderRef?: unknown; + }; + const placeholderRef = el.placeholderRef + ? yorkieToPlain<YorkieElement['placeholderRef']>(el.placeholderRef) + : undefined; if (el.type === 'text') { const blocks = yorkieToPlain<Block[]>((el.data as { blocks?: unknown }).blocks) ?? []; return { id: el.id, type: 'text', frame: yorkieToPlain<Frame>(el.frame), + ...(placeholderRef ? { placeholderRef } : {}), data: { blocks: clone(blocks) }, } as YorkieElement; } return { id: el.id, type: el.type as 'image' | 'shape', frame: yorkieToPlain<Frame>(el.frame), + ...(placeholderRef ? { placeholderRef } : {}), data: yorkieToPlain<object>(el.data), } as YorkieElement; });A matching change is needed in
replaceRoot's text branch around Line 338, and a follow-up to propagateplaceholderRefinread()(per the lessons doc) — onceread()carries the field,clone(e)inreplaceRoot's non-text branch will pick it up automatically.Worth adding a regression test along the lines of: addSlide('title-body') → moveSlide(...) → assert each element still has
placeholderRef.🤖 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/frontend/src/app/slides/yorkie-slides-store.ts` around lines 502 - 532, rebuildSlide currently reconstructs elements without copying placeholderRef, which causes placeholder identity to be lost during moveSlide/moveSlides; update rebuildSlide's element mapping to propagate (e.g. include (el as any).placeholderRef when building both the text branch and the non-text branch so YorkieElement retains placeholderRef, and make the analogous change in replaceRoot's text branch (replaceRoot) so that reconstructed text elements also keep placeholderRef; also follow up by ensuring read() preserves placeholderRef so clone(e) in replaceRoot's non-text branch carries it through.
🧹 Nitpick comments (1)
packages/slides/src/model/layout.ts (1)
295-300: ⚡ Quick winFresh placeholder leaks the spec's
placeholderfield into the Element.
clone(slot.spec)copies the entirePlaceholderSpec, which includes the extraplaceholder: { type }field that doesn't belong onElement. Theas Elementcast hides this from TypeScript, but the property is silently retained on every freshly materialized placeholder — bloating snapshots, leaking layout-spec metadata into element data, and potentially confusing any code that pattern-matches on'placeholder' in element.♻️ Strip `placeholder` before composing the element
for (let si = 0; si < slots.length; si++) { if (usedSlots.has(si)) continue; const slot = slots[si]; - const fresh = { - ...clone(slot.spec), - id: generateId(), - placeholderRef: slot.ref, - } as Element; + const { placeholder: _ph, ...rest } = clone(slot.spec); + const fresh = { + ...rest, + id: generateId(), + placeholderRef: slot.ref, + } as Element; slide.elements.push(fresh); }The same fix applies to the
addSlidemappers inpackages/slides/src/store/memory.tsandpackages/frontend/src/app/slides/yorkie-slides-store.tsif they spread the spec the same way.🤖 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/slides/src/model/layout.ts` around lines 295 - 300, The fresh Element creation is leaking the PlaceholderSpec.placeholder field by doing clone(slot.spec) and casting to Element; modify the composition so you omit the placeholder property before casting—e.g., create a shallow copy of slot.spec without the placeholder key and then spread that into the new object with id: generateId() and placeholderRef: slot.ref before pushing to slide.elements; apply the same omission fix in the analogous mappers in addSlide within packages/slides/src/store/memory.ts and packages/frontend/src/app/slides/yorkie-slides-store.ts to prevent layout-spec metadata from being retained on Element instances.
🤖 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/frontend/tests/app/slides/yorkie-slides-concurrent.integration.ts`:
- Around line 175-198: The test currently mutates the title via
ctx.storeA.withTextElement(...) but never asserts the text value survived; add
explicit assertions after the sync and after the existing presence checks: use
ctx.storeA.read() and ctx.storeB.read(), locate the title element by id
(titleId) in a.slides[0].elements and b.slides[0].elements, and assert that the
element's inline text (e.g., element.inlines[0].text) equals 'Hello' on both A
and B to ensure the guarded write didn't silently no-op.
In `@packages/slides/src/model/layout.ts`:
- Around line 248-265: Reused placeholder-bearing elements (found in reuses via
slide.elements and slots) are left in their original positions, so final element
order can diverge from newLayout.placeholders; after the current matching loop
in applyLayoutToSlide, reorder only the surviving reused elements to follow the
order of newLayout.placeholders: build a map from placeholder ref (type+index)
to its reused element (from reuses), remove those reused elements from
slide.elements while preserving the relative order of non-placeholder/user-added
elements, then insert the reused elements back in the sequence in the exact
order of newLayout.placeholders so z-order/tab-order matches the new layout;
keep deletions/demotions logic unchanged.
- Around line 269-278: The demotion loop currently sets d.element.placeholderRef
= undefined which creates a "property set" CRDT operation; replace that
assignment with a delete operation so the field is removed from the Yorkie
object (i.e., use delete on the placeholderRef property for items in the
demotions array). Update the demotions loop that touches
d.element.placeholderRef (the demotions variable and element.placeholderRef
symbol) to call delete d.element.placeholderRef instead of assigning undefined
to ensure proper CRDT delete semantics and consistency with other code like
yorkie-slides-store.ts.
In `@packages/slides/src/view/canvas/layout-preview.ts`:
- Around line 53-69: The code currently caches the canvas even when
canvas.getContext('2d') returns null, causing a permanently blank canvas to be
reused; update the logic in layout-preview.ts so cache.set(key, canvas) only
runs when a valid 2D context was obtained (i.e., move or duplicate the cache.set
call inside the if (ctx) block after renderThumbnail or return early when ctx is
null), referencing the existing canvas.getContext('2d') check, the ctx variable,
renderThumbnail(ctx, ...), syntheticSlide(layout) and the cache.set(key, canvas)
call to locate and change the behavior.
In `@packages/slides/src/view/editor/layout-picker.ts`:
- Around line 31-32: The popover is positioned directly from opts.anchor.x/y
which can place it offscreen; after appending the popover element, measure its
size (popover.getBoundingClientRect()) and clamp left and top to the viewport
using window.innerWidth/innerHeight minus the popover width/height (and a small
margin, e.g. 8px), ensuring values are >= 0; update popover.style.left/top with
the clamped values (use the existing popover and opts.anchor symbols) so the
picker never extends past the right or bottom edges.
- Around line 49-99: Add basic keyboard/focus support so keyboard and assistive
tech can use the picker: make each cell focusable and operable (set
cell.tabIndex = 0 and role="button"), attach a keydown handler on the cell to
call opts.onPick(layout.id) and close() when Enter or Space is pressed
(preventDefault), and preserve the existing click handler; when creating the
popover set popover.tabIndex = -1 and move focus into it (popover.focus()) after
it is added so focus enters the dialog; keep existing close(), onKey(), and
onOutside() behavior and ensure dataset.selected/visual outline remains in sync
for the currently selected layout.
---
Outside diff comments:
In `@packages/frontend/src/app/slides/yorkie-slides-store.ts`:
- Around line 502-532: rebuildSlide currently reconstructs elements without
copying placeholderRef, which causes placeholder identity to be lost during
moveSlide/moveSlides; update rebuildSlide's element mapping to propagate (e.g.
include (el as any).placeholderRef when building both the text branch and the
non-text branch so YorkieElement retains placeholderRef, and make the analogous
change in replaceRoot's text branch (replaceRoot) so that reconstructed text
elements also keep placeholderRef; also follow up by ensuring read() preserves
placeholderRef so clone(e) in replaceRoot's non-text branch carries it through.
---
Nitpick comments:
In `@packages/slides/src/model/layout.ts`:
- Around line 295-300: The fresh Element creation is leaking the
PlaceholderSpec.placeholder field by doing clone(slot.spec) and casting to
Element; modify the composition so you omit the placeholder property before
casting—e.g., create a shallow copy of slot.spec without the placeholder key and
then spread that into the new object with id: generateId() and placeholderRef:
slot.ref before pushing to slide.elements; apply the same omission fix in the
analogous mappers in addSlide within packages/slides/src/store/memory.ts and
packages/frontend/src/app/slides/yorkie-slides-store.ts to prevent layout-spec
metadata from being retained on Element instances.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b63b2a6e-48b9-482d-9550-4d0ffa3d032a
📒 Files selected for processing (26)
docs/design/README.mddocs/design/slides/slides-layout-change.mddocs/tasks/README.mddocs/tasks/active/20260508-slides-layout-change-lessons.mddocs/tasks/active/20260508-slides-layout-change-todo.mdpackages/frontend/src/app/slides/yorkie-slides-store.tspackages/frontend/src/types/slides-document.tspackages/frontend/tests/app/slides/yorkie-slides-concurrent.integration.tspackages/slides/src/index.tspackages/slides/src/model/clone.tspackages/slides/src/model/element.test.tspackages/slides/src/model/element.tspackages/slides/src/model/layout-apply.test.tspackages/slides/src/model/layout.test.tspackages/slides/src/model/layout.tspackages/slides/src/model/presentation.tspackages/slides/src/store/memory.test.tspackages/slides/src/store/memory.tspackages/slides/src/view/canvas/layout-preview.test.tspackages/slides/src/view/canvas/layout-preview.tspackages/slides/src/view/editor/editor.test.tspackages/slides/src/view/editor/editor.tspackages/slides/src/view/editor/layout-picker.test.tspackages/slides/src/view/editor/layout-picker.tspackages/slides/src/view/editor/thumbnail-panel.test.tspackages/slides/src/view/editor/thumbnail-panel.ts
Assigning undefined to a Yorkie ObjectProxy field generates a "property set" CRDT op that keeps the field with an undefined value; delete generates the correct "remove field" op so peers and the read view never see a leftover key. Functionally both worked for our truthy-check partition, but the delete form is faithful to the lessons file's algorithm description and matches yorkie-slides-store.ts. Also documents the array-order preservation contract that the T6.1 rewrite established but the docstring did not yet state. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- Layout picker now exposes role=listbox + role=option, focusable cells, Enter/Space handler, and an on-mount focus move so keyboard and assistive-tech users can drive the feature. - Picker also clamps to the viewport so the 4-column grid never clips off the right or bottom edge when invoked near a corner. - renderLayoutPreview no longer caches a blank canvas when the 2D context is unavailable, leaving callers free to retry. - The "edit + applyLayout" Yorkie integration test now asserts the title text actually survived rather than letting the guarded write silently no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Surfaced during PR #197 smoke: a fresh slide from a non-blank layout shows empty placeholder boxes that aren't visually distinguishable from each other. Authors can't tell where the title goes vs. the body. Plans the same Google Slides / Keynote fix — muted "Click to add title" hints inside empty placeholders that vanish on first character typed. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
A new slide from a non-blank layout otherwise reads as visually empty — authors cannot tell where the title goes vs. the body. Google Slides / PowerPoint / Keynote all solve this with muted "Click to add title" hints inside empty placeholders. The hint table is a single seam ready for future i18n, and only fires for ref-bearing elements so user-added text boxes are unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
read() rebuilt each element from the Yorkie proxy but dropped the placeholderRef field, so the rendering path saw user-added shapes where layout placeholders should have been. T12 ghost text relies on this signal — without the propagation the hint never fired in the live editor even though MemSlidesStore tests had been green all along. Lessons file flagged this as latent in T6.1; T12 is what made it bite. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Marks lesson #2 as no longer latent; T12 ghost text was the trigger that turned the dropped-field bug into a live render miss, and the fix is one yorkieToPlain unwrap per element branch. The new lesson cites the audit rule for future agents: new element fields need both write (addSlide) AND read (read()) propagation, MemSlidesStore parity tests miss it. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The first cut of T12 hard-coded 32px sans-serif left-aligned for every slot, so the title hint and the body hint and the caption hint all looked identical — defeating the whole point of the hint, which is to telegraph what the slot will look like once typed into. Hint now reads its font role + size, color role, and alignment from the active master's PlaceholderStyle, matching the typography the user will actually see on first character. Adds caption (14 px body / textSecondary) and big-number (96 px heading / centered) entries to DEFAULT_MASTER so all five placeholder slots have a defined style. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Ghost text already adopts the slot's master PlaceholderStyle (font role/size, color role, alignment), but typed text was falling back to the docs DEFAULT_INLINE_STYLE (11px Arial) so the user saw a tiny black string where the ghost had promised a 44px heading. Seeding the placeholder element's first inline with fontSize, fontFamily, and a role-bound color at addSlide time means typed characters inherit the right typography from the very first keystroke. applyLayoutToSlide takes the same seeding path for slots it has to materialize fresh. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Marks every step in the todo file as completed and moves the todo + lessons pair into docs/tasks/archive/2026/05/ so docs/tasks/active/ shows only in-flight work. The lessons file's #2 entry was already updated mid-PR when the read() placeholderRef stripping turned out to be live, not latent — that record carries forward into the archive. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
11 tasks
hackerwins
added a commit
that referenced
this pull request
May 11, 2026
Adds @wafflebase/slides as a third surface alongside Sheets and Docs, plus 53-shape library (Phase 1+2), adjustment handles for 9 pilot shapes (P3-A.1), live snap guides, align/distribute toolbar, themed authoring, and layout-change UI. Also ships Sheets cell comments (Phase B), docs peer-avatar caret jump, yorkie-js-sdk 0.7.8 upgrade, and CLI/REST API improvements (docs export imageFetcher, expired session refresh, REST docs split). Minor bump because slides is a new top-level package. Highlights: #184 #185 #186 #187 #188 #189 #190 #191 #192 #197 #198 #201 #202 #203 #204 #205 #206 #207 #209 #210
7 tasks
8 tasks
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
Lets slides authors pick a layout when inserting a new slide and change the layout of an existing slide, without losing typed content.
Element.placeholderRef = { type, index }so slot identity survives layout switches. Type-first matching (with index fallback for same-type slots like the two-column layout's twobodyslots).applyLayoutToSlidepure function backs bothMemSlidesStoreandYorkieSlidesStore. Orphan placeholders demote to plain elements rather than getting destroyed; users rely on undo, not confirm dialogs.showLayoutPickerpopover. Opened by a new▾split on the thumbnail panel+button (existing left zone keeps blank-insert) and by a "Change layout…" item on the slide canvas context menu.renderThumbnailagainst a synthetic slide, with a module-level cache keyed by(theme, master, layout, size).Spec:
docs/design/slides/slides-layout-change.mdPlan + lessons:
docs/tasks/active/20260508-slides-layout-change-todo.md,docs/tasks/active/20260508-slides-layout-change-lessons.mdTest plan
pnpm verify:fast(47 files / 764 tests green)applyLayoutToSlidescenarios (blank → fresh, preserve typed into same slot, ambiguous body 0/1 by index, fewer slots empty → delete, fewer slots non-empty → demote, user elements untouched), 5showLayoutPickercases, 4renderLayoutPreviewcache cases, picker wiring on the thumbnail panel split and on the slide context menu.yorkie-slides-concurrent.integration.tscover concurrentapplyLayoutconvergence and "edit + applyLayout" without proxy crash.Smoke checklist
Run
pnpm dev, then walk through:+ Add slide(left zone) appends ablankslide.▾next to+ Add slide→ picker shows 11 layout previews → picktitle-body→ slide appears with title + body placeholders.title-bodyoutlined → picktitle-only→ "Hello" stays in title; the body text remains as a free-floating text box at its old position (placeholder demoted, not deleted).title-two-columns. TypeLin left body,Rin right. Change totitle-only→ exactly one demoted body element remains besides the title slot. Switch back totitle-two-columns→ original body slot pair re-appears (matched by(type, index)).Notes for reviewers
docs/tasks/active/20260508-slides-layout-change-lessons.md§ 1 and § 4 before suggesting code changes toapplyLayoutToSlideor to any new@wafflebase/slidesexport.YorkieSlidesStore.read()stripsplaceholderRefin its conversion path. Doesn't bite the layout-change feature (writes go through the live proxy), but worth a follow-up before any consumer readsplaceholderRefoffread(). See lessons § 2.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation