feat: add SlideTransition API for per-page animation control#149
Conversation
Lets slide authors declare per-page enter/exit animations as WAAPI keyframes, with an optional module-level default. The framework owns the transition lifecycle (two-layer stack, direction-aware --osd-dir, interrupt cancellation, prefers-reduced-motion fallback) so authors can focus on the visual. Internal SlideTransitionLayer drives both the Player and the editor viewer.
Six pages exercising the new API: module-level fade default, slide-from- edge with --osd-dir mirroring, scale-pop with blur, direction-aware clip-path wipe, and parallel blur cross-fade. Arrow through to see each transition; backward navigation mirrors via the CSS direction variable.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduces a per-page slide transition API using the Web Animations API. Defines ChangesSlide Transition Feature
Sequence DiagramsequenceDiagram
participant React
participant SlideTransitionLayer
participant DOM as DOM Slots
participant WAA as Web Animations API
React->>SlideTransitionLayer: index changes
SlideTransitionLayer->>SlideTransitionLayer: resolve transition from page
SlideTransitionLayer->>SlideTransitionLayer: compute direction (forward/backward)
SlideTransitionLayer->>SlideTransitionLayer: set outgoing & current indices
SlideTransitionLayer->>DOM: render outgoing and current pages
SlideTransitionLayer->>SlideTransitionLayer: effect runs when outgoing set
SlideTransitionLayer->>WAA: runPhase(outgoing, exit)
SlideTransitionLayer->>WAA: runPhase(current, enter)
WAA->>WAA: animate with finished promises
WAA-->>SlideTransitionLayer: both animations complete
SlideTransitionLayer->>SlideTransitionLayer: clear outgoing state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/app/routes/slide.tsx (1)
314-322:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
transitionprop for consistency with playMode Player.This Player instance (when
!showSlideUi) lacks thetransitionprop, while the playMode Player at line 330 passes it. Both should behave consistently regarding slide transitions.Proposed fix
if (!showSlideUi) { return ( <Player pages={pages} design={slide.design} + transition={slide.transition} index={index} onIndexChange={goTo} onExit={() => {}} allowExit={false} /> ); }🤖 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/core/src/app/routes/slide.tsx` around lines 314 - 322, The Player instance rendered when !showSlideUi is missing the transition prop, causing inconsistent behavior with the playMode Player; update the Player call that passes pages, design (slide.design), index, onIndexChange (goTo), onExit, and allowExit to also pass the same transition prop used by the playMode Player so both Players use the same transition configuration.
🤖 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 @.changeset/slide-transition-api.md:
- Line 5: Rewrite the changeset one-line to use present-tense, user-facing
wording describing the change from the user's perspective: replace "Add
SlideTransition API for declaring per-page page-transition animations." with a
present-tense sentence like "Provides a SlideTransition API that lets pages
declare per-page transition animations." Ensure the line stays short, direct,
and mentions the SlideTransition API and the user-visible effect (per-page
transition animations).
In `@packages/core/src/app/components/slide-transition-layer.tsx`:
- Around line 45-63: The transition early-return when disabled is true skips
cancelling running animations and snapping to the final state; update the
handler (the block that references index, currentRef.current, animsRef.current,
resolveTransition, setCurrent, setOutgoing) so that cancellation of any
in-flight animations always runs before returning for a disabled transition, and
then immediately setCurrent(next) and setOutgoing(null) (snap to final state) if
disabled or no transition was resolved; ensure animsRef.current is cleared after
cancellation and keep the existing try/catch around a.cancel() to avoid
throwing.
- Around line 50-68: The two fixed sibling page containers in the
SlideTransitionLayer render function are unkeyed, causing React to reuse DOM
nodes and break in-flight animations on quick navigation; fix it by giving each
container a stable key derived from the page identity (e.g., use current and
outgoing values or a page id from pages[next/prev]) or refactor the sibling
structure to render the page nodes as keyed children that can move between
positions; update the render code in SlideTransitionLayer (where the two fixed
divs are created) to assign keys tied to the page identity so that the DOM nodes
are preserved for ongoing animations, and verify animsRef, setCurrent and
setOutgoing logic continue to reference the same keyed elements.
---
Outside diff comments:
In `@packages/core/src/app/routes/slide.tsx`:
- Around line 314-322: The Player instance rendered when !showSlideUi is missing
the transition prop, causing inconsistent behavior with the playMode Player;
update the Player call that passes pages, design (slide.design), index,
onIndexChange (goTo), onExit, and allowExit to also pass the same transition
prop used by the playMode Player so both Players use the same transition
configuration.
🪄 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: e8b4e17c-d935-4387-9f9c-863b0fe54b02
📒 Files selected for processing (9)
.changeset/slide-transition-api.mdapps/demo/slides/slide-transitions/index.tsxpackages/core/src/app/components/player.tsxpackages/core/src/app/components/slide-transition-layer.tsxpackages/core/src/app/lib/sdk.tspackages/core/src/app/lib/transition.tspackages/core/src/app/lib/use-prefers-reduced-motion.tspackages/core/src/app/routes/slide.tsxpackages/core/src/index.ts
| '@open-slide/core': minor | ||
| --- | ||
|
|
||
| Add SlideTransition API for declaring per-page page-transition animations. |
There was a problem hiding this comment.
Use present-tense, user-facing wording in the changeset line.
Please rewrite this to describe user-visible impact in present tense (instead of imperative “Add …”).
Suggested update
-Add SlideTransition API for declaring per-page page-transition animations.
+Slide authors can configure per-page transition animations with SlideTransition.As per coding guidelines, “Changeset descriptions must be short and direct: one line, present-tense, describing what changed from a user's perspective.”
📝 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.
| Add SlideTransition API for declaring per-page page-transition animations. | |
| Slide authors can configure per-page transition animations with SlideTransition. |
🤖 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 @.changeset/slide-transition-api.md at line 5, Rewrite the changeset one-line
to use present-tense, user-facing wording describing the change from the user's
perspective: replace "Add SlideTransition API for declaring per-page
page-transition animations." with a present-tense sentence like "Provides a
SlideTransition API that lets pages declare per-page transition animations."
Ensure the line stays short, direct, and mentions the SlideTransition API and
the user-visible effect (per-page transition animations).
| if (index === currentRef.current) return; | ||
|
|
||
| const prev = currentRef.current; | ||
| const next = index; | ||
|
|
||
| // Interrupt: cancel in-flight animations. The previously-incoming page | ||
| // (currentRef) becomes the new outgoing; React reuses its DOM slot. | ||
| for (const a of animsRef.current) { | ||
| try { | ||
| a.cancel(); | ||
| } catch {} | ||
| } | ||
| animsRef.current = []; | ||
|
|
||
| const transition = resolveTransition(pages, next, moduleTransition); | ||
| if (disabled || !transition) { | ||
| setCurrent(next); | ||
| setOutgoing(null); | ||
| return; |
There was a problem hiding this comment.
disabled does not stop an already-running transition.
When disabled flips to true mid-transition, the early return prevents cancellation/snap, so motion continues until completion.
Proposed fix
useEffect(() => {
- if (index === currentRef.current) return;
+ if (disabled) {
+ for (const a of animsRef.current) {
+ try {
+ a.cancel();
+ } catch {}
+ }
+ animsRef.current = [];
+ setOutgoing(null);
+ return;
+ }
+ if (index === currentRef.current) return;Also applies to: 71-115
🤖 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/core/src/app/components/slide-transition-layer.tsx` around lines 45
- 63, The transition early-return when disabled is true skips cancelling running
animations and snapping to the final state; update the handler (the block that
references index, currentRef.current, animsRef.current, resolveTransition,
setCurrent, setOutgoing) so that cancellation of any in-flight animations always
runs before returning for a disabled transition, and then immediately
setCurrent(next) and setOutgoing(null) (snap to final state) if disabled or no
transition was resolved; ensure animsRef.current is cleared after cancellation
and keep the existing try/catch around a.cancel() to avoid throwing.
| // Interrupt: cancel in-flight animations. The previously-incoming page | ||
| // (currentRef) becomes the new outgoing; React reuses its DOM slot. | ||
| for (const a of animsRef.current) { | ||
| try { | ||
| a.cancel(); | ||
| } catch {} | ||
| } | ||
| animsRef.current = []; | ||
|
|
||
| const transition = resolveTransition(pages, next, moduleTransition); | ||
| if (disabled || !transition) { | ||
| setCurrent(next); | ||
| setOutgoing(null); | ||
| return; | ||
| } | ||
|
|
||
| setDirection(next > prev ? 'forward' : 'backward'); | ||
| setOutgoing(prev); | ||
| setCurrent(next); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In React 18 reconciliation, if two sibling <div> elements are rendered without keys, are they preserved by position (first stays first, second stays second) rather than moving an existing DOM node from position 2 into position 1?
💡 Result:
In React, when two sibling elements are rendered without keys, React reconciles them based on their position in the tree [1][2]. React assumes that elements at the same position in the parent are the same across renders if their types match [1][2]. If you have two sibling div elements and their order changes, or if one is removed, React does not "move" the existing DOM nodes to new positions [1][3]. Instead, it compares the elements at each position index [1][2]. If the element type at a specific position remains the same, React updates the existing DOM node to reflect the new props [4][3]. If the element type changes, React unmounts the old component (destroying its state) and mounts a new one [1][2]. Consequently, if you swap the order of two unkeyed sibling divs, React will not move the DOM nodes [5][3]. Instead, it will see that the element at position 1 has changed (it now has the content/props of the former second element) and the element at position 2 has changed (it now has the content/props of the former first element) [1][5]. React will update the existing DOM nodes in place to match the new content [5][3]. This behavior is why unkeyed lists or siblings can lead to state-related bugs: because React associates state with the position in the tree, not the specific element instance [5][2]. If you need to preserve state or ensure DOM nodes are moved rather than updated in place, you must provide stable, unique keys [6][3][2].
Citations:
- 1: https://cekrem.github.io/posts/react-reconciliation-deep-dive/
- 2: https://react.dev/learn/preserving-and-resetting-state
- 3: https://www.jsguide.dev/topic/react-lists-and-keys
- 4: https://legacy.reactjs.org/docs/reconciliation.html
- 5: https://www.developerway.com/posts/reconciliation-in-react
- 6: https://docs.bswen.com/blog/2026-03-03-react-reconciliation-keys/
🏁 Script executed:
cat -n packages/core/src/app/components/slide-transition-layer.tsx | head -100Repository: 1weiho/open-slide
Length of output: 3837
🏁 Script executed:
cat -n packages/core/src/app/components/slide-transition-layer.tsx | tail -n +100Repository: 1weiho/open-slide
Length of output: 1710
🏁 Script executed:
# Verify the lines 133-143 reference mentioned in the review
sed -n '133,143p' packages/core/src/app/components/slide-transition-layer.tsxRepository: 1weiho/open-slide
Length of output: 501
Unkeyed sibling containers cause re-render-in-place behavior that disrupts animation continuity on navigation interrupt.
The component renders two fixed sibling divs without keys (lines 133–146). When navigation is interrupted—such as starting a transition from page 0 to 1, then immediately navigating to 2—React updates these divs by position rather than preserving their DOM identity. The previously-incoming page (being animated in to the second div) gets re-rendered into the first div's outgoing position, disrupting its animation state and causing visual snapping under rapid navigation.
To preserve animation continuity, each page container needs a stable key or the sibling structure needs to be refactored to allow DOM node movement rather than in-place updates.
Also applies to lines 133–146.
🤖 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/core/src/app/components/slide-transition-layer.tsx` around lines 50
- 68, The two fixed sibling page containers in the SlideTransitionLayer render
function are unkeyed, causing React to reuse DOM nodes and break in-flight
animations on quick navigation; fix it by giving each container a stable key
derived from the page identity (e.g., use current and outgoing values or a page
id from pages[next/prev]) or refactor the sibling structure to render the page
nodes as keyed children that can move between positions; update the render code
in SlideTransitionLayer (where the two fixed divs are created) to assign keys
tied to the page identity so that the DOM nodes are preserved for ongoing
animations, and verify animsRef, setCurrent and setOutgoing logic continue to
reference the same keyed elements.
API: add optional TransitionPhase.delay so authors can express the out-then-in stagger that defines tasteful slide motion (exit starts immediately, enter delays ~80ms). Switch animation fill to 'both' so delayed enter phases hold their first keyframe during the delay instead of flashing the live element state. Set the transition-layer wrapper background to var(--osd-bg) so a section-break "hold" (both layers at opacity 0) doesn't flash the underlying canvas white. Demo: drop the six-different-transitions deck. Replace with one house transition (200ms fade + 6px rise, exit/enter overlapped) used on every page, a Cover variant with a touch more rise and a single- direction blur, and one Section-break variant on the chapter slide. Editorial dark palette, serif display, no per-page swatches — the deck now demonstrates restraint instead of variety.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/core/src/app/components/slide-transition-layer.tsx (1)
45-65:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle
disabledbefore the index short-circuit to stop in-flight motion.Line 46 returns early when
indexis unchanged, so togglingdisabledduring an active transition does not cancel running animations.Proposed fix
useEffect(() => { + if (disabled) { + for (const a of animsRef.current) { + try { + a.cancel(); + } catch {} + } + animsRef.current = []; + setOutgoing(null); + if (index !== currentRef.current) setCurrent(index); + return; + } + if (index === currentRef.current) return; const prev = currentRef.current; const next = index; @@ - if (disabled || !transition) { + if (!transition) { setCurrent(next); setOutgoing(null); return; }🤖 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/core/src/app/components/slide-transition-layer.tsx` around lines 45 - 65, Move the disabled check to the top of the useEffect so toggling disabled will interrupt any running animations even when index === currentRef.current; specifically in the useEffect that references index and currentRef.current, check if (disabled) first, iterate animsRef.current and call cancel() inside try/catch, clear animsRef.current, call setCurrent(index) and setOutgoing(null) (or otherwise ensure the state is reset), then return; keep the existing logic using resolveTransition, setCurrent and setOutgoing for the non-disabled path.
🤖 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 `@apps/demo/slides/slide-transitions/index.tsx`:
- Around line 115-126: The h2 currently renders the heading string in normal
flow so any '\n' characters are collapsed; update the render in
slide-transitions/index.tsx so newlines are preserved by either adding CSS
whiteSpace: 'pre-wrap' to the h2 style or by splitting the heading on '\n' and
interleaving <br/>s (e.g., heading.split('\n').map(...)) so each line break in
the heading prop is rendered; change the h2 element (where heading is used) to
use one of these approaches.
---
Duplicate comments:
In `@packages/core/src/app/components/slide-transition-layer.tsx`:
- Around line 45-65: Move the disabled check to the top of the useEffect so
toggling disabled will interrupt any running animations even when index ===
currentRef.current; specifically in the useEffect that references index and
currentRef.current, check if (disabled) first, iterate animsRef.current and call
cancel() inside try/catch, clear animsRef.current, call setCurrent(index) and
setOutgoing(null) (or otherwise ensure the state is reset), then return; keep
the existing logic using resolveTransition, setCurrent and setOutgoing for the
non-disabled path.
🪄 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: 158664c8-e657-4400-bc0a-4db6e83e8977
📒 Files selected for processing (3)
apps/demo/slides/slide-transitions/index.tsxpackages/core/src/app/components/slide-transition-layer.tsxpackages/core/src/app/lib/transition.ts
| <h2 | ||
| style={{ | ||
| fontFamily: 'var(--osd-font-display)', | ||
| fontSize: 92, | ||
| fontWeight: 400, | ||
| lineHeight: 1.02, | ||
| letterSpacing: '-0.025em', | ||
| margin: 0, | ||
| }} | ||
| > | ||
| {heading} | ||
| </h2> |
There was a problem hiding this comment.
\n in heading is collapsed, so forced line breaks won’t render.
At Line 125, heading is rendered in normal text flow; the newline characters provided at Lines 164, 173, and 222 collapse to spaces.
Proposed fix
<h2
style={{
fontFamily: 'var(--osd-font-display)',
fontSize: 92,
fontWeight: 400,
lineHeight: 1.02,
letterSpacing: '-0.025em',
+ whiteSpace: 'pre-line',
margin: 0,
}}
>Also applies to: 164-164, 173-173, 222-222
🤖 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 `@apps/demo/slides/slide-transitions/index.tsx` around lines 115 - 126, The h2
currently renders the heading string in normal flow so any '\n' characters are
collapsed; update the render in slide-transitions/index.tsx so newlines are
preserved by either adding CSS whiteSpace: 'pre-wrap' to the h2 style or by
splitting the heading on '\n' and interleaving <br/>s (e.g.,
heading.split('\n').map(...)) so each line break in the heading prop is
rendered; change the h2 element (where heading is used) to use one of these
approaches.
The previous restraint pass collapsed the deck onto a single house transition. This version restores per-page variety while keeping every move inside the same family: ~200ms total duration, ease-out enter delayed 80ms after an ease-in exit, opacity always animated, translate ≤12px, scale ≤3%. Six members of the family, one per page: settle — Y + soft blur falloff (cover) dissolve — pure opacity, larger overlap (Apple safe default) rise — 6px Y, the house quiet (also the module default) breath — exit, 120ms hold, enter (section divider) bloom — scale 0.97 → 1, no translate fall — mirrored rise, enters from above (closing) Each page footer names its transition.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/demo/slides/slide-transitions/index.tsx (1)
296-408: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winTrim transition banner/comments to guideline-compliant WHY-only notes.
These added comments are mostly section banners and WHAT-descriptions of code behavior. Please remove or rewrite them to only capture non-obvious constraints/workarounds.
As per coding guidelines, "
**/*.{ts,tsx,js,jsx}: Default to writing no comments. Only add one when the WHY is non-obvious ... Don't explain WHAT the code does ... don't use section-divider banners."🤖 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 `@apps/demo/slides/slide-transitions/index.tsx` around lines 296 - 408, The file contains verbose section banners and WHAT-focused comments above transition objects (e.g., Cover.transition, Family.transition, the exported transition, Pause.transition, SmallMagnitudes.transition, Closing.transition); remove or rewrite those comments so they only state non-obvious rationale or workarounds (WHY), not descriptions of what the code does or decorative section dividers—keep a single short WHY note where needed and delete the rest to comply with the guideline that comments should explain intent or surprising constraints only.
♻️ Duplicate comments (1)
apps/demo/slides/slide-transitions/index.tsx (1)
117-128:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve authored line breaks in
heading.
headingincludes\n, but the current<h2>styling collapses newlines into spaces. AddwhiteSpace: 'pre-line'(or render with explicit<br />) so those breaks display.Also applies to: 169-169, 179-179, 243-243
🤖 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 `@apps/demo/slides/slide-transitions/index.tsx` around lines 117 - 128, The H2 rendering of the variable heading is collapsing authored newlines; update the style object on the <h2> (the element that renders {heading}) to include whiteSpace: 'pre-line' so existing \n characters produce line breaks, or alternatively split heading on '\n' and render with explicit <br/>s; apply the same change to the other heading occurrences that render {heading} in this file (the additional <h2> usages noted in the review).
🤖 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.
Outside diff comments:
In `@apps/demo/slides/slide-transitions/index.tsx`:
- Around line 296-408: The file contains verbose section banners and
WHAT-focused comments above transition objects (e.g., Cover.transition,
Family.transition, the exported transition, Pause.transition,
SmallMagnitudes.transition, Closing.transition); remove or rewrite those
comments so they only state non-obvious rationale or workarounds (WHY), not
descriptions of what the code does or decorative section dividers—keep a single
short WHY note where needed and delete the rest to comply with the guideline
that comments should explain intent or surprising constraints only.
---
Duplicate comments:
In `@apps/demo/slides/slide-transitions/index.tsx`:
- Around line 117-128: The H2 rendering of the variable heading is collapsing
authored newlines; update the style object on the <h2> (the element that renders
{heading}) to include whiteSpace: 'pre-line' so existing \n characters produce
line breaks, or alternatively split heading on '\n' and render with explicit
<br/>s; apply the same change to the other heading occurrences that render
{heading} in this file (the additional <h2> usages noted in the review).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d669482-cac8-4c99-bd3b-df64f2333d27
📒 Files selected for processing (1)
apps/demo/slides/slide-transitions/index.tsx
Add a Page Transitions section to the slide-authoring skill in both the core source and the CLI template snapshot. Covers the API contract (per-page vs module-level, incoming-page-wins, --osd-dir direction hook), the design principles (one DNA per deck, 140-280ms duration band, 12px / 3% magnitude ceiling, ease-in exit / ease-out enter, out-then-in stagger), and a six-member family of tasteful defaults — rise, dissolve, settle, bloom, fall, breath — as paste- ready few-shot examples. Adds a self-review checklist item and a dedicated anti-pattern list (translateX 100%, scale-pop, clip-path wipe, parallel blur, linear easing, durations past 350ms, six-different-transitions decks).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/cli/template/.agents/skills/slide-authoring/SKILL.md`:
- Around line 376-382: The dissolve SlideTransition example (symbol: dissolve)
currently uses exit.duration 200 and enter.delay 40 which violates the
documented exit range (140–180ms) and produces a 160ms overlap vs the
recommended ~80ms; update the example to comply or document the deviation:
either set exit.duration to 180 and enter.delay to 80 (keeping enter.duration
240) to match the principles, or add a short inline comment next to the dissolve
definition explaining that the longer exit (200ms) and larger overlap (160ms)
are intentional to produce a quieter, smoother dissolve; reference EASE_IN and
EASE_OUT in the comment if noting the intentional stylistic choice.
🪄 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: 22debdd3-ef42-46ab-a9a6-b9f45bc0a556
📒 Files selected for processing (3)
.changeset/slide-transition-skill-docs.mdpackages/cli/template/.agents/skills/slide-authoring/SKILL.mdpackages/core/skills/slide-authoring/SKILL.md
✅ Files skipped from review due to trivial changes (2)
- .changeset/slide-transition-skill-docs.md
- packages/core/skills/slide-authoring/SKILL.md
| const dissolve: SlideTransition = { | ||
| duration: 240, | ||
| exit: { duration: 200, easing: EASE_IN, | ||
| keyframes: [{ opacity: 1 }, { opacity: 0 }] }, | ||
| enter: { duration: 240, delay: 40, easing: EASE_OUT, | ||
| keyframes: [{ opacity: 0 }, { opacity: 1 }] }, | ||
| }; |
There was a problem hiding this comment.
DISSOLVE example deviates from stated principles without explanation.
The exit duration is 200ms, which exceeds the 140–180ms exit range stated in the design principles (line 347). Additionally, the effective overlap is 160ms (200ms exit - 40ms enter delay), which is double the recommended ~80ms stagger mentioned in line 347.
While this may be intentional for the "quietest possible" transition, the deviation is not explicitly called out. Consider either:
- Adjusting the timings to fit within the guidelines (e.g., exit 180ms, enter 240ms, delay 80ms), or
- Adding a brief comment in the example explaining why DISSOLVE intentionally uses a longer exit and overlap for smoothness.
📝 Proposed alignment with principles
// DISSOLVE — pure opacity. The quietest possible.
const dissolve: SlideTransition = {
duration: 240,
- exit: { duration: 200, easing: EASE_IN,
+ exit: { duration: 180, easing: EASE_IN,
keyframes: [{ opacity: 1 }, { opacity: 0 }] },
- enter: { duration: 240, delay: 40, easing: EASE_OUT,
+ enter: { duration: 240, delay: 80, easing: EASE_OUT,
keyframes: [{ opacity: 0 }, { opacity: 1 }] },
};Alternatively, if the longer timing is intentional:
-// DISSOLVE — pure opacity. The quietest possible.
+// DISSOLVE — pure opacity. The quietest possible. Longer overlap (160ms) for extra smoothness.
const dissolve: SlideTransition = {📝 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.
| const dissolve: SlideTransition = { | |
| duration: 240, | |
| exit: { duration: 200, easing: EASE_IN, | |
| keyframes: [{ opacity: 1 }, { opacity: 0 }] }, | |
| enter: { duration: 240, delay: 40, easing: EASE_OUT, | |
| keyframes: [{ opacity: 0 }, { opacity: 1 }] }, | |
| }; | |
| const dissolve: SlideTransition = { | |
| duration: 240, | |
| exit: { duration: 180, easing: EASE_IN, | |
| keyframes: [{ opacity: 1 }, { opacity: 0 }] }, | |
| enter: { duration: 240, delay: 80, easing: EASE_OUT, | |
| keyframes: [{ opacity: 0 }, { opacity: 1 }] }, | |
| }; |
🤖 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/cli/template/.agents/skills/slide-authoring/SKILL.md` around lines
376 - 382, The dissolve SlideTransition example (symbol: dissolve) currently
uses exit.duration 200 and enter.delay 40 which violates the documented exit
range (140–180ms) and produces a 160ms overlap vs the recommended ~80ms; update
the example to comply or document the deviation: either set exit.duration to 180
and enter.delay to 80 (keeping enter.duration 240) to match the principles, or
add a short inline comment next to the dissolve definition explaining that the
longer exit (200ms) and larger overlap (160ms) are intentional to produce a
quieter, smoother dissolve; reference EASE_IN and EASE_OUT in the comment if
noting the intentional stylistic choice.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.changeset/slide-transition-api.md (1)
6-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse present-tense, user-facing wording per guideline.
The changeset description still uses imperative mood "Add" instead of present tense. As per coding guidelines, changeset descriptions must use present tense and describe what changed from the user's perspective.
📝 Suggested rewrite
-Add SlideTransition API for declaring per-page page-transition animations, plus a transitions section in the bundled slide-authoring skill with tasteful few-shot examples. +Introduces SlideTransition API for per-page transition animations and adds transition guidance to slide-authoring skill.Or more concisely:
-Add SlideTransition API for declaring per-page page-transition animations, plus a transitions section in the bundled slide-authoring skill with tasteful few-shot examples. +Adds SlideTransition API for per-page transition animations and authoring guidance.As per coding guidelines: "Changeset descriptions must be short and direct: one line, present-tense, describing what changed from a user's perspective."
🤖 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 @.changeset/slide-transition-api.md at line 6, Rewrite the changeset one-line to present-tense, user-facing wording: replace the imperative "Add SlideTransition API..." with a present-tense description such as "SlideTransition API enables per-page page-transition animations and the bundled slide-authoring skill includes a transitions section with tasteful few-shot examples," updating the .changeset/slide-transition-api.md summary to reference SlideTransition API and the transitions section in the bundled slide-authoring skill using present-tense phrasing.
🤖 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.
Duplicate comments:
In @.changeset/slide-transition-api.md:
- Line 6: Rewrite the changeset one-line to present-tense, user-facing wording:
replace the imperative "Add SlideTransition API..." with a present-tense
description such as "SlideTransition API enables per-page page-transition
animations and the bundled slide-authoring skill includes a transitions section
with tasteful few-shot examples," updating the
.changeset/slide-transition-api.md summary to reference SlideTransition API and
the transitions section in the bundled slide-authoring skill using present-tense
phrasing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 136f93e5-8bff-431b-9155-f3abaa0767ac
📒 Files selected for processing (1)
.changeset/slide-transition-api.md
1weiho
changed the title
SlideTransition API for per-page animation control
Iris (clip-path), 3D flip (perspective + rotateY), chromatic glitch (stacked drop-shadow + steps easing), warp (blur + scale), shear sweep (translate + skewX), and portal (rotate + scale collapse). Sits opposite the restrained "On Tasteful Transitions" deck. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 participants
Introduces a new
SlideTransitionAPI that allows slide authors to declare custom enter/exit animations on a per-page basis. The framework manages the animation lifecycle while authors control the visual behavior.Changes
New transition system (
packages/core/src/app/lib/transition.ts): DefinesSlideTransitiontype withduration,easing,enter, andexitphases. IncludesresolveTransition()helper that checks for page-level overrides before falling back to module defaults.SlideTransitionLayer component (
packages/core/src/app/components/slide-transition-layer.tsx): Orchestrates dual-layer animation rendering. Manages outgoing/incoming page lifecycle, cancels in-flight animations on rapid navigation, and applies direction-aware CSS variables (--osd-dir) for directional transitions.Reduced motion support (
packages/core/src/app/lib/use-prefers-reduced-motion.ts): New hook respectsprefers-reduced-motionmedia query, disabling animations for users who prefer them.SDK updates (
packages/core/src/app/lib/sdk.ts): ExtendedPagetype to include optionaltransitionproperty.Player integration (
packages/core/src/app/components/player.tsx,packages/core/src/app/routes/slide.tsx): WiredSlideTransitionLayerinto both presentation and edit modes, passing module-level transition defaults and respecting reduced-motion preferences.Demo slide deck (
apps/demo/slides/slide-transitions/index.tsx): Comprehensive example showcasing five distinct transition patterns (slide-from-side, scale-pop, clip-wipe, blur-cross) with direction-aware CSS variables and per-page animation overrides.Exports (
packages/core/src/index.ts): ExposedSlideTransitiontype for public API.Implementation notes
fill: 'forwards'to maintain final state.--osd-dirCSS variable (1 for forward, -1 for backward) for responsive transition behavior.https://claude.ai/code/session_01Ef5QVLmVuyb8XrYn8EPxfn
Summary by CodeRabbit
Release Notes
New Features
SlideTransitionAPI enabling customizable page-transition animations with per-page overrides and module-level defaults.Documentation