add pause flag#26
Conversation
📝 WalkthroughWalkthroughThis PR adds a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/shared/hooks.ts (1)
239-244: Consider setting initial paused state immediately after scene creation to eliminate a brief visual timing gap.If
pausedis initiallytrue, the scene briefly renders unpaused (when created at line 162) before the effect (lines 239-244) runs and pauses it. Set the initial state right after assignment to avoid this:if (scene) { sceneRef.current = scene; if (paused !== undefined) { scene.paused = paused; } hasAttemptedRef.current = false; setInitError(null); isInitializingRef.current = false; onLoad?.(); }The existing effect will continue to handle subsequent
pausedprop changes.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.mdsrc/next/index.tsxsrc/react/index.tsxsrc/shared/constants.tssrc/shared/hooks.tssrc/shared/types.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/next/index.tsx (1)
src/shared/constants.ts (1)
DEFAULT_VALUES(8-21)
src/react/index.tsx (1)
src/shared/constants.ts (1)
DEFAULT_VALUES(8-21)
🔇 Additional comments (6)
README.md (1)
213-213: LGTM! Documentation is clear and accurate.The prop is properly documented with correct type and default value, consistent with the implementation.
src/shared/types.ts (1)
20-20: LGTM! Type addition is correct.The optional
pausedproperty is properly added toUnicornSceneProps. Note thatUnicornStudioSceneinterface (line 33) already includespaused?: boolean, which aligns well with this change.src/react/index.tsx (1)
22-22: LGTM! Implementation mirrors the Next.js version.The
pausedprop is correctly integrated following the same pattern as insrc/next/index.tsx. The same verification regarding Unicorn Studio library support applies here.Also applies to: 51-51
src/shared/constants.ts (1)
17-17: LGTM! Appropriate default value.Setting
paused: falseas the default is correct—scenes should animate by default, with users opting in to pause behavior when needed.src/next/index.tsx (1)
25-25: Verify that Unicorn Studio's Scene class supports apausedproperty.The prop is correctly wired through the component to the
useUnicornScenehook, where it's synced to the scene object at lines 240–244 ofhooks.ts. However, the Unicorn Studio v2.0.1 documentation does not explicitly document apausedproperty on the Scene class. TheUnicornSceneConfiginterface also does not include this property, meaning it can only be set post-initialization via direct assignment (sceneRef.current.paused = paused). Please confirm with Unicorn Studio that this property exists and is supported in v2.0.1 to ensure it will work as intended.src/shared/hooks.ts (1)
23-23: LGTM! Clean interface extension.The
pausedproperty is correctly added as optional to the interface and properly destructured in the hook parameters.Also applies to: 40-40
The
Sceneclass has a bool pause attribute which can be toggled. This introduces that flag to the component so the user can pause and unpause the scene's rendering.Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.