FIX: re-render/jitter(brand-compliance-component) #3078
Conversation
|
@admirsaheta is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughReplaces a custom RAF/ref parallax with Framer Motion hooks ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (3)
28-36: Comment describes different value than code uses.The comment mentions "multiplier 2000" but the actual implementation uses
500 * speed. Consider updating the comment to reflect the actual value for accuracy.🔎 Proposed fix
// Map scroll progress (0 to 1) to transform values // When scroll is 0 (element entering from bottom), we want positive offset (pushed down) // When scroll is 1 (element exiting at top), we want negative offset (pulled up) - // The multiplier 2000 is an approximation of the viewport height + element height range to match previous 'distanceFromCenter' logic roughly + // The multiplier 500 controls the parallax intensity range const yRange = useTransform( scrollYProgress, [0, 1], [500 * speed, -500 * speed] );
59-61: Hardcoded user-facing text should use next-intl.As per coding guidelines for this path, user-facing text should use next-intl messages/hooks instead of hardcoded strings. This applies to:
- "Brand Colors" (lines 60, 189)
- "Brand compliance" (line 322)
- The description text (line 327)
Consider extracting these strings to your i18n message files and using the
useTranslationshook:const t = useTranslations('landing.brandCompliance'); // Then use t('brandColors'), t('title'), t('description')Also applies to: 188-190, 321-328
65-78: Add a comment explaining the duplicate color shades.The
colorClassesarray contains intentional duplicates: the middle shade (e.g.,bg-slate-500) and darkest shade (e.g.,bg-slate-900) appear twice in each color group. This pattern is consistent across all color groups (Slate, Gray, Zinc, Orange, Amber, Lime, Green, Cyan, Blue, Indigo, Violet, Purple, Pink, Rose). Add a brief comment documenting why these duplicates exist—whether for visual spacing, equal distribution, or another design purpose.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (2)
apps/web/client/src/app/_components/landing-page/color-swatch-group.tsx (1)
ColorSwatchGroup(8-28)packages/ui/src/components/icons/index.tsx (1)
Icons(138-3667)
🔇 Additional comments (3)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (3)
1-8: LGTM!The
"use client"directive is correctly applied since this component uses React hooks (useRef) and the motion/react library for scroll-driven animations. Import paths are appropriate.
38-51: Nice improvement to the parallax implementation.The use of
useSpringwithuseScrollanduseTransformis an effective pattern that decouples the animation from React's render cycle. Thewill-change-transformhint appropriately signals GPU acceleration. This should eliminate the jitter issues described in the PR.
57-58: Good use of opposite parallax speeds for visual depth.Using
speed={0.08}andspeed={-0.08}creates an effective parallax depth effect where the two panels move in opposite directions during scroll. This is a nice touch for visual engagement.Also applies to: 186-187
…s/brand-compliance.tsx Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (2)
315-330: Consider using next-intl for user-facing text.The title "Brand compliance" (line 322) and description text (line 327) are hardcoded strings. Per the coding guidelines for this codebase, user-facing text should use next-intl messages/hooks for internationalization support.
Based on coding guidelines for
apps/web/client/src/**/*.tsxfiles.
1-8: Usemotion/react-clientinstead ofmotion/reactfor better App Router optimization.The motion package (v12.23.19) is correctly installed. However, since this component is in the App Router (apps/web/client/src/app/), importing from
motion/react-clientinstead ofmotion/reactreduces client-side JavaScript as documented in the motion package for Next.js.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Prefer TailwindCSS-first styling and reuse existing UI components from onlook/ui and local patterns
Applied to files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (2)
apps/web/client/src/app/_components/landing-page/color-swatch-group.tsx (1)
ColorSwatchGroup(8-28)packages/ui/src/components/icons/index.tsx (1)
Icons(138-3667)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
Show resolved
Hide resolved
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (1)
35-35: Comment mentions multiplier 2000 but code uses 500.The comment references "multiplier 2000" while the actual code uses
500 * speed. Update the comment to match the implementation.
🧹 Nitpick comments (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (1)
64-64: Hardcoded user-facing text should use next-intl.Per coding guidelines, avoid hardcoded user-facing text in this path. The strings "Brand Colors", "Brand compliance", and the description should be internationalized using
next-intlmessages/hooks.Example locations:
- Line 64, 186:
"Brand Colors"- Line 312:
"Brand compliance"- Line 317:
"Make your fonts, colors, and styles all speak the same language."Also applies to: 186-186, 311-312, 316-317
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Prefer TailwindCSS-first styling and reuse existing UI components from onlook/ui and local patterns
Applied to files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (1)
apps/web/client/src/app/_components/landing-page/color-swatch-group.tsx (1)
ColorSwatchGroup(8-28)
🔇 Additional comments (4)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (4)
1-8: LGTM!The
"use client"directive is correctly applied since the component uses React hooks (useRef) and browser APIs (window.matchMedia). Imports follow the project's path alias conventions.
42-54: Good use of spring physics and GPU hints.The
useSpringconfiguration provides smooth dampened motion, andwill-change-transformcorrectly hints hardware acceleration. Clean implementation.
67-179: Color swatch arrays are now correctly structured.Each color group now uses 11 unique Tailwind shades (50–950) without duplicates. This correctly integrates with
ColorSwatchGroup's slicing logic (first 6 for row 1, remaining 5 + Plus icon for row 2).Also applies to: 189-300
60-61: Dual-panel parallax with opposing speeds works well.Using
speed={0.08}andspeed={-0.08}creates a balanced parallax effect where panels move in opposite directions during scroll. Theoverflow-hiddencontainer correctly constrains the motion.Also applies to: 183-184
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (1)
32-32: Update the comment to match the actual multiplier.The comment mentions "2000" but the code on line 36 uses
500 * speed. This inconsistency was flagged in a previous review.
🧹 Nitpick comments (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (1)
61-61: Consider using next-intl for user-facing text.The component contains hardcoded English strings ("Brand Colors", "Brand compliance", and the description). Per the coding guidelines, user-facing text in
apps/web/client/src/**/*.tsxshould use next-intl messages/hooks for proper internationalization support.📋 Example refactor using next-intl
Import the translation hook:
import { useTranslations } from 'next-intl';Inside the component:
const t = useTranslations('landing.brandCompliance');Then replace the strings:
// Line 61 and 183 {t('brandColors')} // Line 309 {t('title')} // Line 314 {t('description')}Add corresponding keys to your messages file:
{ "landing": { "brandCompliance": { "brandColors": "Brand Colors", "title": "Brand compliance", "description": "Make your fonts, colors, and styles all speak the same language." } } }Based on coding guidelines for this path pattern.
Also applies to: 183-183, 309-309, 314-314
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.tsx : Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Applied to files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (1)
packages/ui/src/components/icons/index.tsx (1)
Icons(138-3667)
🔇 Additional comments (3)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (3)
1-9: LGTM! Clean imports and correct client directive.The "use client" directive is appropriately placed for a component using React hooks and motion effects. Path aliases and imports are well-structured.
16-52: Excellent parallax implementation with proper accessibility support.The refactor successfully:
- Uses
useReducedMotion()hook for SSR-safe motion preference detection- Applies
will-change-transformfor hardware acceleration- Implements smooth spring physics with
useSpring- Correctly disables animation when reduced motion is preferred
The scroll offset configuration
["start end", "end start"]provides the full parallax range as the element traverses the viewport.
54-319: LGTM! Well-structured dual-parallax layout.The component effectively implements:
- Two opposing parallax containers (speeds ±0.08) for dynamic visual effect
- Correct color scale arrays with 11 unique shades (50-950) matching the
ColorSwatchGroupcomponent's layout expectations- Clean separation between animated content and static footer section
The refactor successfully eliminates manual scroll listeners and layout thrashing while maintaining smooth animation performance.
There was a problem hiding this comment.
thanks you for fixing bugs just look at what coderabbit and the other bost syas and then i think this would be passing pr
do you agree @coderabbitai
|
As for the next internationalization part. I don't see the landing page being localized. Only the app. So avoided adding the keys for it :) |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (1)
16-52: LGTM: Solid Motion-based parallax implementation.The ParallaxContainer correctly uses Motion hooks (useScroll, useTransform, useSpring) with proper reduced-motion support via useReducedMotion. The spring physics parameters and will-change-transform hint optimize for smooth performance.
💡 Optional: Make the comment more explicit
The comment on lines 29-32 could explicitly mention the "500" multiplier for clarity:
- // The multiplier is an approximation of the viewport height + element height range to match previous 'distanceFromCenter' logic roughly + // The multiplier 500 is an approximation of the viewport height + element height range to match previous 'distanceFromCenter' logic roughly
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧠 Learnings (1)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.tsx : Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Applied to files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🔇 Additional comments (5)
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (5)
1-8: LGTM: Correct "use client" directive and imports.The "use client" directive is properly placed since this component uses React hooks and browser APIs. The imports from motion/react and path aliases are correct.
10-14: LGTM: Well-defined interface.The ParallaxContainerProps interface is clearly typed with appropriate optional parameters for speed and className customization.
64-176: LGTM: Color arrays are now correct.All ColorSwatchGroup components in the first panel now use the complete 11-shade Tailwind color scale (50-950) without duplicates. This properly addresses the previous review feedback about duplicate color values.
186-297: LGTM: Color arrays are consistent and complete.All ColorSwatchGroup components in the second panel correctly use the complete 11-shade Tailwind color scale (50-950). The color organization is logical and consistent with the first panel.
55-301: LGTM: Well-structured dual-panel parallax layout.The component effectively uses two ParallaxContainer instances with opposing speeds (0.08 and -0.08) to create an engaging parallax effect. The layout is well-organized with proper positioning and responsive design considerations. The refactor from manual scroll handling to Motion hooks is a solid performance improvement.
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
Outdated
Show resolved
Hide resolved
|
@admirsaheta I think you should commit the code that coderabbit ai is saying that yo should fix or you could use the prompt it is giving fro a higher chances of getting merged |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/client/messages/en.d.json.tsapps/web/client/messages/en.jsonapps/web/client/messages/es.jsonapps/web/client/messages/ja.jsonapps/web/client/messages/ko.jsonapps/web/client/messages/zh.jsonapps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/web/client/messages/ja.json
🧰 Additional context used
📓 Path-based instructions (8)
apps/web/client/messages/**
📄 CodeRabbit inference engine (AGENTS.md)
Add or modify internationalized strings in apps/web/client/messages/* instead of hardcoding text
Files:
apps/web/client/messages/en.d.json.tsapps/web/client/messages/es.jsonapps/web/client/messages/zh.jsonapps/web/client/messages/en.jsonapps/web/client/messages/ko.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/messages/en.d.json.tsapps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/messages/en.d.json.tsapps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/messages/**/*.{json,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Add or modify translation keys in apps/web/client/messages/* and keep keys stable (prefer additions over breaking renames)
Files:
apps/web/client/messages/en.d.json.tsapps/web/client/messages/es.jsonapps/web/client/messages/zh.jsonapps/web/client/messages/en.jsonapps/web/client/messages/ko.json
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🧠 Learnings (5)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/messages/**/*.{json,ts} : Add or modify translation keys in apps/web/client/messages/* and keep keys stable (prefer additions over breaking renames)
Applied to files:
apps/web/client/messages/en.d.json.tsapps/web/client/messages/es.jsonapps/web/client/messages/zh.jsonapps/web/client/messages/en.jsonapps/web/client/messages/ko.jsonapps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/messages/** : Add or modify internationalized strings in apps/web/client/messages/* instead of hardcoding text
Applied to files:
apps/web/client/messages/en.d.json.tsapps/web/client/messages/es.jsonapps/web/client/messages/zh.jsonapps/web/client/messages/en.jsonapps/web/client/messages/ko.jsonapps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.{ts,tsx} : Avoid hardcoded user-facing text; use next-intl messages/hooks instead
Applied to files:
apps/web/client/messages/en.d.json.tsapps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/app/**/*.tsx : Avoid hardcoded user-facing text; use next-intl messages/hooks
Applied to files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
Repo: onlook-dev/onlook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/**/*.tsx : Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Applied to files:
apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx
🔇 Additional comments (8)
apps/web/client/messages/en.json (1)
367-371: LGTM!The new
BrandComplianceBlockkeys are properly structured and will serve as the source for other locale translations.apps/web/client/messages/en.d.json.ts (1)
370-374: LGTM!The TypeScript declaration correctly reflects the new
BrandComplianceBlockstructure.apps/web/client/src/app/_components/landing-page/feature-blocks/brand-compliance.tsx (6)
1-10: LGTM!Proper client directive and imports. Uses
useReducedMotionfrommotion/react(correctly handles SSR), path aliases for imports, and next-intl for translations.
18-54: Well-implemented parallax with accessibility support.The
ParallaxContainercorrectly:
- Uses
useReducedMotion()to respect user motion preferences (addresses prior SSR concern)- Applies spring physics for smooth, dampened motion
- Uses
will-change-transformfor GPU acceleration hints
56-65: LGTM!Properly uses
useTranslations()withtransKeysfor internationalized text, addressing the prior hardcoded text concern. Based on coding guidelines forapps/web/client/src/**/*.{ts,tsx}.
68-180: Color palettes are now unique and complete.Each
ColorSwatchGrouphas 11 distinct shades (50 through 950) without duplicates, addressing the prior review concern about duplicate color values.
184-304: Second parallax panel with opposite direction.The negative speed value (
-0.08) creates a visually appealing counter-motion effect. Color palettes (Cyan through Rose) are properly structured with unique 50-950 shades.
306-321: LGTM!Title and description properly use translation keys, completing the i18n refactor for this component.
| "BrandComplianceBlock": { | ||
| "brandColors": "Brand Colors", | ||
| "title": "Brand compliance", | ||
| "description": "Make your fonts, colors, and styles all speak the same language." | ||
| } |
There was a problem hiding this comment.
Missing Spanish translations for BrandComplianceBlock.
The values are in English but this is the Spanish locale file. These should be translated to Spanish.
Example translations:
"BrandComplianceBlock": {
"brandColors": "Colores de marca",
"title": "Cumplimiento de marca",
"description": "Haz que tus fuentes, colores y estilos hablen el mismo idioma."
}🤖 Prompt for AI Agents
In apps/web/client/messages/es.json around lines 367 to 371 the
BrandComplianceBlock values are still in English; replace them with the provided
Spanish translations (brandColors -> "Colores de marca", title -> "Cumplimiento
de marca", description -> "Haz que tus fuentes, colores y estilos hablen el
mismo idioma.") ensuring you update the JSON values only (preserve keys, commas
and indentation) and keep the file valid JSON.
| "BrandComplianceBlock": { | ||
| "brandColors": "Brand Colors", | ||
| "title": "Brand compliance", | ||
| "description": "Make your fonts, colors, and styles all speak the same language." | ||
| } |
There was a problem hiding this comment.
Missing Korean translations for BrandComplianceBlock.
The values are in English but this is the Korean locale file. These should be translated to Korean for proper localization.
Example translations:
"BrandComplianceBlock": {
"brandColors": "브랜드 색상",
"title": "브랜드 일관성",
"description": "글꼴, 색상 및 스타일이 모두 동일한 언어를 사용하도록 하세요."
}🤖 Prompt for AI Agents
In apps/web/client/messages/ko.json around lines 367 to 371, the
BrandComplianceBlock entries are still in English; replace them with the Korean
translations so the locale file is consistent (brandColors -> "브랜드 색상", title ->
"브랜드 일관성", description -> "글꼴, 색상 및 스타일이 모두 동일한 언어를 사용하도록 하세요."). Ensure keys
remain unchanged and only the values are updated, preserving JSON formatting and
escaping as needed.
| "BrandComplianceBlock": { | ||
| "brandColors": "Brand Colors", | ||
| "title": "Brand compliance", | ||
| "description": "Make your fonts, colors, and styles all speak the same language." | ||
| } |
There was a problem hiding this comment.
Missing Chinese translations for BrandComplianceBlock.
The values are in English but this is the Chinese locale file. These should be translated to Chinese.
Example translations:
"BrandComplianceBlock": {
"brandColors": "品牌颜色",
"title": "品牌一致性",
"description": "让您的字体、颜色和样式都使用相同的语言。"
}🤖 Prompt for AI Agents
In apps/web/client/messages/zh.json around lines 367 to 371, the
BrandComplianceBlock values are left in English; replace them with the Chinese
translations provided in the review (brandColors -> "品牌颜色", title -> "品牌一致性",
description -> "让您的字体、颜色和样式都使用相同的语言。") so the Chinese locale file contains
localized strings; update the three values exactly and ensure the JSON remains
valid (proper commas/quotes).
2 participants
Description
Optimized the parallax scroll effect in the BrandComplianceBlock component to improve performance and fluidity, particularly on mobile devices.
Related Issues
N/A
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Changes & Optimizations
Mobile Scroll Behavior
Before: On mobile devices, the parallax effect was likely jittery and unresponsive. The combination of state updates on scroll and layout thrashing caused visible lag, where the element's position would "stutter" or trail noticeably behind the scroll action.
After: The animation is now decoupled from the React render cycle and smoothed via spring physics. Scrolling on mobile feels buttery smooth, with the parallax element gliding naturally relative to the scroll position without blocking the main thread.
Important
Optimizes
BrandComplianceBlockparallax effect using Framer Motion for smoother animations and improved performance, especially on mobile.useScrollinParallaxContainerfor better performance.useSpringto smooth transform values, enhancing animation fluidity.will-change-transformfor hardware acceleration, reducing repaints.ParallaxContainerinbrand-compliance.tsxto usemotion/reactfor scroll tracking and animation.ParallaxContainerprops to includeclassNamefor styling flexibility.This description was created by
for 8e0b2e0. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Style
Localization
✏️ Tip: You can customize this high-level summary in your review settings.