fix(slides): propagate design system CSS vars to all preview surfaces#1145
fix(slides): propagate design system CSS vars to all preview surfaces#1145Abdul-M01 wants to merge 7 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
steve8708
left a comment
There was a problem hiding this comment.
great catch - thanks @Abdul-M01 !
theres some bot feedback here too - take a look and fix whatever you agree with, ignore anything you don't
also one CI check to fix too
|
@Abdul-M01 this PR is great - just one piece of feedback above to get it in |
Add design_system_id to deck_share_links so shared presentations render the deck's design system instead of always defaulting.
6e8c0de to
0568897
Compare
Visual recap — skippedThe visual recap job did not run for this pull request. This is informational only and does not block the PR. Recap skipped for |
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🔴
Review Details
This incremental update addresses the previous front-end wiring gaps: designSystem is now threaded through presentation/shared/route/history surfaces, SharedDeckResponse carries designSystemId, and SlideRenderer applies var(--ds-bg, #000000) with a default design system fallback. The overall approach remains sound and moves the feature closer to true O(1) theming.
That said, two follow-on correctness issues remain. First, the shared-link fix snapshots only the designSystemId, but shared viewers still resolve tokens client-side through get-design-system; for private design systems, anonymous viewers cannot access that resource and will silently fall back to the default theme. Second, history previews now always render with the current deck's design system even though version snapshots already carry their own designSystemId, so rebranded decks can show misleading restore previews.
Risk assessment: Standard. No security or schema-destructive concerns, but there is one user-visible regression in shared presentations and one medium-severity correctness issue in history previews. Browser testing was attempted after the dev server recovered, but the browser executor had no working browser tools, so visual verification could not be completed.
🧪 Browser testing: Attempted, but browser executor was unavailable; UI flows could not be verified
| title: title || storedDeck.title || "Untitled", | ||
| slides: JSON.stringify(slides), | ||
| aspectRatio: storedDeck.aspectRatio ?? null, | ||
| designSystemId, |
There was a problem hiding this comment.
🔴 Shared links persist only a private design-system ID
This stores just designSystemId, but shared viewers resolve it later through get-design-system. For the common case where the linked design system is still private, anonymous shared viewers fall back to the default theme instead of the deck's branding.
…solveAccess gates resource-level access
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🔴
Review Details
This incremental update fixes the earlier HistoryPanel mismatch by resolving selectedVersion?.designSystemId and passing the resulting theme into SlideRenderer, which is the right direction for snapshot previews. The rest of the design-system threading still looks structurally sound across editor, presentation, share, and slide-preview surfaces.
However, the latest follow-up introduces two new correctness issues. First, marking get-design-system as requiresAuth: false does not actually make it callable by anonymous shared viewers, because the Slides auth plugin still applies the global auth guard to /_agent-native/actions/*; in practice, unauthenticated requests still return 401 before the action handler runs. Second, the share-link snapshot now stores only designSystemId, so already-issued share links no longer preserve the exact styling that was published — edits or deletion of the linked design system can silently rebrand or break an existing shared presentation.
Risk assessment: Standard. The HistoryPanel fix is good and the prior preview-thread issue has been resolved, but anonymous/shared presentation theming is still not correct, and share snapshots are no longer self-contained. Browser verification was attempted again; the dev server was healthy, but browser executors still lacked usable browser automation tools, so most visual checks could not be completed.
🧪 Browser testing: Attempted, but browser executor tooling was unavailable; only non-browser HTTP/code-path checks could be completed
| }), | ||
| readOnly: true, | ||
| http: { method: "GET" }, | ||
| requiresAuth: false, |
There was a problem hiding this comment.
🔴 requiresAuth false still leaves this action behind the global auth guard
This action is marked requiresAuth: false, but the Slides auth plugin still treats /_agent-native/actions/* as private unless the path is explicitly public. Anonymous requests to this route still 401 before the action handler runs, so shared viewers without a session cannot load design-system data.
| title: title || storedDeck.title || "Untitled", | ||
| slides: JSON.stringify(slides), | ||
| aspectRatio: storedDeck.aspectRatio ?? null, | ||
| designSystemId, |
There was a problem hiding this comment.
🟡 Shared links no longer preserve the published design-system styling
The share snapshot now stores only designSystemId, so the shared presentation resolves the live design-system record at view time. If that design system is edited, deleted, or otherwise changes later, already-issued share links can silently rebrand or fall back instead of preserving the styling they were shared with.
|
we just merged a big localization PR that created some conflicts, if you can resolve @Abdul-M01 and check if any of the other automated feedback looks worth fixing would love to get this in |
Summary
Applying a design system to an existing deck was O(n) in tool calls.
The agent had to read, rewrite, and update each slide individually. On
a modest deck this produced 25+ tool calls. This PR reduces retroactive application to O(1).
Problem
Skill templates contained hardcoded color and font values baked into
slide HTML.
SlideRendereralready injected CSS custom properties atrender time, but the hardcoded values bypassed the cascade, forcing
the agent to rewrite each slide's content when a design system was
applied or changed.
When a design system was applied retroactively to an existing deck,
sidebar thumbnails and browse-card previews continued to render with
default tokens —
designSystemwas never forwarded toEditorSidebaror
DeckCard.Solution
Replace all hardcoded values in skill templates with
var(--ds-*)references so new slides are token-aware from creation. Retroactive
application becomes a single call —
SlideRendererhandles the restvia CSS cascade.
Thread
designSystemthrough to all three render surfaces so themain editor, sidebar thumbnails, and browse cards stay consistent.
Fallback mirrors
useDeckDesignSystemDEFAULT_DESIGN_SYSTEMbehaviour at each call site.
Key Changes
create-deck/SKILL.md,slide-editing/SKILL.md— replace hardcodedhex/font values with
var(--ds-*)tokensdesign-systems/SKILL.md— add retroactive application guidanceSlideRenderer.tsx— add--ds-heading-weightand--ds-body-weightto CSS var injection block
EditorSidebar.tsx,DeckEditor.tsx— threaddesignSystempropthrough to
<SlideRenderer>Index.tsx,DeckCard.tsx— builddesignSystemByIdmap and threaddesignSystemthrough to<SlideRenderer>Known limitation
The current implementation stores share links as self-contained copies — duplicating title,
slides, and aspect ratio rather than referencing the live deck. This change follows that pattern
by persisting
design_system_idinto the share link at creation time rather than joining back todecksat read time. Schema changes were required becauseSharedPresentationloads entirelyfrom
deck_share_links, so without the column, there was no design system ID to pass intouseDeckDesignSystem.Share links created before this change have
design_system_id = NULLand continue to renderwith default branding — no regression. New shares of existing decks capture the current
designSystemIdat creation time. A backfill migration for already-published share links isscoped as a follow-up.