Unify all marquees on one rAF engine: MarqueeWall + QuickActionWall + fae/mingo/it/sec theme SSOT#1495
Conversation
… theme SSOT [preview:none] Co-Authored-By: Claude Fable 5 <[email protected]>
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe PR introduces shared marquee animation and synchronization primitives, adds themed quick-action walls with loading and brick-wall layouts, migrates ChangesMarquee and quick-action surfaces
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant QuickActionMarquee
participant QuickActionWall
participant MarqueeWall
participant useMarqueeEngine
QuickActionMarquee->>QuickActionWall: map items to themed chips
QuickActionWall->>MarqueeWall: render chips, rows, fades, and speed
MarqueeWall->>useMarqueeEngine: provide pause and wrap configuration
useMarqueeEngine->>MarqueeWall: apply looping track transforms
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openframe-frontend-core/src/components/features/cards-strip.tsx (1)
335-343: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winStop the animation engine while the strip is offscreen.
nearViewportRefonly feedsisPaused, souseMarqueeEnginecontinues scheduling rAF callbacks indefinitely while offscreen. Store intersection state in React state and passactive: marqueeActive && nearViewportasMarqueeWalldoes.Also applies to: 463-470
🤖 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 `@openframe-frontend-core/src/components/features/cards-strip.tsx` around lines 335 - 343, Update the IntersectionObserver logic in the cards strip component to store the current intersection state in React state instead of only updating nearViewportRef. Pass an active value combining marqueeActive with the near-viewport state into useMarqueeEngine, matching MarqueeWall’s behavior, so animation scheduling stops while the strip is offscreen; update the corresponding duplicated observer logic as well.
🧹 Nitpick comments (2)
openframe-frontend-core/src/stories/QuickActionWall.stories.tsx (1)
47-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace hardcoded spacing and radius utilities with ODS tokens.
The new story surfaces use
p-8,p-4, androunded-md. Use the corresponding ODS spacing and radius utilities so the examples accurately demonstrate design-system usage.As per coding guidelines, “Do not use hardcoded styles; translate Figma/Tailwind output and all hand-written styling to ODS tokens.”
Also applies to: 61-61, 90-90
🤖 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 `@openframe-frontend-core/src/stories/QuickActionWall.stories.tsx` at line 47, Replace the hardcoded p-8, p-4, and rounded-md classes in the QuickActionWall story examples with their corresponding ODS spacing and radius utilities, including the occurrences referenced near lines 47, 61, and 90. Preserve the existing layout and styling intent while ensuring all spacing and radius values use ODS tokens.Source: Coding guidelines
openframe-frontend-core/src/components/ui/marquee-wall.tsx (1)
185-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace hardcoded marquee dimensions with ODS tokens.
The component defaults and stories encode spacing as raw pixel values rather than token-backed CSS lengths.
openframe-frontend-core/src/components/ui/marquee-wall.tsx#L185-L190: replace fade-size literals with semantic ODS variables.openframe-frontend-core/src/components/ui/marquee-wall.tsx#L276-L276: replace the numeric copy-gap default with an ODS spacing variable.openframe-frontend-core/src/stories/MarqueeWall.stories.tsx#L52-L52: use a token-backedcopyGap.openframe-frontend-core/src/stories/MarqueeWall.stories.tsx#L67-L67: use a token-backedcopyGap.openframe-frontend-core/src/stories/MarqueeWall.stories.tsx#L91-L91: use a token-backedcopyGap.openframe-frontend-core/src/stories/MarqueeWall.stories.tsx#L111-L112: use token-backed fade and copy-gap values.As per coding guidelines, raw pixel sizes are not allowed in
openframe-frontend-core/src/**/*.{ts,tsx,css}.🤖 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 `@openframe-frontend-core/src/components/ui/marquee-wall.tsx` around lines 185 - 190, Replace all raw pixel marquee dimensions with semantic ODS CSS variables: update DEFAULT_FADE_SIZE in marquee-wall.tsx, the copyGap default near line 276, and every listed copyGap/fade value in MarqueeWall.stories.tsx. Apply the appropriate token-backed lengths at marquee-wall.tsx lines 185-190 and 276, and stories lines 52, 67, 91, and 111-112; make no direct changes elsewhere.Source: Coding guidelines
🤖 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 `@openframe-frontend-core/src/components/chat/quick-action-marquee.tsx`:
- Line 85: Update the copyGap prop in the quick-action marquee to use the ODS
chip-gap token, passing "var(--spacing-system-xsf)" instead of the hardcoded
value 8 so the clone seam tracks token changes.
In `@openframe-frontend-core/src/components/chat/quick-action-wall.tsx`:
- Around line 185-197: Update the row chip spacing in the MarqueeWall mapping
within the quick-action wall component to use the same copyGap value or CSS
variable already applied to the row stack and clone seam, replacing the fixed
--spacing-system-xsf gap. Preserve the existing fallback behavior so all three
gaps share one pitch.
In `@openframe-frontend-core/src/components/features/cards-strip.tsx`:
- Around line 308-313: Update the overflow state calculation in the cards-strip
sizing logic so any singleCopy width greater than scroller.clientWidth remains
navigable and enables chevrons. Separate this from the marquee/cloning
eligibility check, which may continue requiring overflow beyond TRACK_GAP_PX,
and ensure the static-row path does not suppress navigation for small real
overflow.
In `@openframe-frontend-core/src/components/ui/marquee-wall.tsx`:
- Around line 39-48: Update the MarqueeSyncMember synchronization and
driver-promotion logic around useMarqueeSync so every follower keeps its local
posRef aligned with the shared/current transform. Before starting the engine for
a promoted driver, initialize its position from that synchronized value rather
than its stale local position, preserving continuity when the prior driver
unmounts.
---
Outside diff comments:
In `@openframe-frontend-core/src/components/features/cards-strip.tsx`:
- Around line 335-343: Update the IntersectionObserver logic in the cards strip
component to store the current intersection state in React state instead of only
updating nearViewportRef. Pass an active value combining marqueeActive with the
near-viewport state into useMarqueeEngine, matching MarqueeWall’s behavior, so
animation scheduling stops while the strip is offscreen; update the
corresponding duplicated observer logic as well.
---
Nitpick comments:
In `@openframe-frontend-core/src/components/ui/marquee-wall.tsx`:
- Around line 185-190: Replace all raw pixel marquee dimensions with semantic
ODS CSS variables: update DEFAULT_FADE_SIZE in marquee-wall.tsx, the copyGap
default near line 276, and every listed copyGap/fade value in
MarqueeWall.stories.tsx. Apply the appropriate token-backed lengths at
marquee-wall.tsx lines 185-190 and 276, and stories lines 52, 67, 91, and
111-112; make no direct changes elsewhere.
In `@openframe-frontend-core/src/stories/QuickActionWall.stories.tsx`:
- Line 47: Replace the hardcoded p-8, p-4, and rounded-md classes in the
QuickActionWall story examples with their corresponding ODS spacing and radius
utilities, including the occurrences referenced near lines 47, 61, and 90.
Preserve the existing layout and styling intent while ensuring all spacing and
radius values use ODS tokens.
🪄 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 Plus
Run ID: 26241a9d-60c0-4912-885d-59c4c4425105
📒 Files selected for processing (15)
openframe-frontend-core/src/components/chat/chat-quick-action-row.tsxopenframe-frontend-core/src/components/chat/index.tsopenframe-frontend-core/src/components/chat/quick-action-chip.tsxopenframe-frontend-core/src/components/chat/quick-action-marquee.tsxopenframe-frontend-core/src/components/chat/quick-action-wall.tsxopenframe-frontend-core/src/components/features/cards-strip.tsxopenframe-frontend-core/src/components/ui/index.tsopenframe-frontend-core/src/components/ui/marquee-wall.tsxopenframe-frontend-core/src/hooks/ui/index.tsopenframe-frontend-core/src/hooks/ui/use-marquee-engine.tsopenframe-frontend-core/src/hooks/ui/use-suppress-clone-focus.tsopenframe-frontend-core/src/stories/MarqueeWall.stories.tsxopenframe-frontend-core/src/stories/QuickActionMarquee.stories.tsxopenframe-frontend-core/src/stories/QuickActionWall.stories.tsxopenframe-frontend-core/tailwind.config.ts
💤 Files with no reviewable changes (1)
- openframe-frontend-core/tailwind.config.ts
…Marquee [preview:none] Co-Authored-By: Claude Fable 5 <[email protected]>
…all pitch, viewport-gated engine, sync position handoff - QuickActionChipButton/QuickActionChip/QuickActionWall accept caller-injected theme accents (themeAccent / themeAccents) so server-configured fae/mingo colors override the fallback-only QUICK_ACTION_THEMES map (Michael's review comment) - QuickActionWall: ONE pitch (copyGap, default var(--spacing-system-xsf)) drives chip gap, clone seam, and brick row-stack gap via new MarqueeWall contentStyle (CodeRabbit major) - MarqueeSyncController carries the live position; a promoted driver seeds its engine from it instead of teleporting to 0 (CodeRabbit major) - CardsStrip: viewport now gates the engine's active (rAF fully stops off-screen) instead of being a pause reason; split overflows (chevrons, >1px) from marqueeEligible (>gap) so razor-thin overflow keeps navigation (CodeRabbit major+minor) - QuickActionMarquee copyGap + story spacing moved to ODS tokens (CodeRabbit minor) Co-Authored-By: Claude Fable 5 <[email protected]>
1 participant
What changed
One marquee animation for the whole design system, plus the unified quick-action wall:
useMarqueeEngine(hooks/ui) — THE rAF marquee core (float position, velocity envelope, wrap range withgetWrapMin, glide), extracted verbatim fromCardsStrip. CardsStrip now consumes it; its inline engine and local clone-focus hook are deleted.MarqueeWall(ui) — transform-driven endless wall (no inner scroller): axis x/y, reverse, edge fades via the singleMarqueeWallFadescomponent (renders only while content overflows), measured seam wrap (supports CSS-var gaps),useMarqueeSyncpixel-locked pairs with driver re-election,trackIdoffset registry +getMarqueeAppliedOffset/getMarqueeCloneTwinfor FLIP-morph coordination (used by the hub deck).QuickActionWall(chat) — the chip wall preset: per-chip fae/mingo/it/sec themes, brick-rows mode (stacked independent row marquees), repeat-padding (minChips), shared skeleton spread (WALL_SKELETON_LABEL_CH),interleaveSSOT.QuickActionMarqueeis now a thin single-row preset of it (own track-doubling +qa-marqueekeyframes deleted from the tailwind config; API moved from duration to px/s).QUICK_ACTION_THEMES— fae/mingo/it/sec accents + lozenges in one map, derived from the sameAGENT_ACCENTSsource asgetAgentAccent;QuickActionChipFromDatais the single chip-data mapper (row + wall + marquee).MarqueeWallandQuickActionWall.Why
Three competing marquee implementations (CardsStrip rAF,
qa-marqueeCSS keyframes, static walls with hand-rolled fades) and four chip-wall variants collapsed into one path each — "unify means delete variations".Reviewer notes
claude/marquee-animation-unify-6058c3— merge this lib PR first, then release + bump the hub pin.🤖 Generated with Claude Code
Summary by CodeRabbit