feat: animated banner for imagine #2653
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request adds Threlte and Three.js packages and a GLSL module declaration, updates the Main layout to render a new TeaserBanner, and introduces three new Svelte components under src/routes/(marketing)/(components)/teaser: a Canvas wrapper (canvas.svelte), an Imagination component with a custom GLSL fragment shader, mouse-trail logic, and a capped FPS loop (imagination.svelte), and a TeaserBanner component composing the canvas and visual with a public props interface (teaser-banner.svelte). Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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
🧹 Nitpick comments (6)
src/lib/layouts/Main.svelte (1)
25-26: Remove unusedHackathonBannerimport.The
HackathonBannerimport on line 25 is no longer used since it was replaced byTeaserBanner.- import HackathonBanner from '$routes/(marketing)/(components)/(hackathon)/hackathon-banner.svelte'; import TeaserBanner from '$routes/(marketing)/(components)/teaser/teaser-banner.svelte';src/routes/(marketing)/(components)/teaser/teaser-banner.svelte (2)
72-80: Avoid fixed pixel dimensions in inline styles.The container uses hardcoded pixel values (
width: 143.384px; height: 35.785px) which may not scale well across viewports. Consider using responsive units or Tailwind classes.-<div - style="display: flex; width: 143.384px; height: 35.785px; padding: 1.348px 0.634px 1.348px 0; justify-content: center; align-items: flex-end; gap: 10.75px; flex-shrink: 0;" -> +<div class="flex shrink-0 items-end justify-center"> <img src="/images/logos/Imagine.png" alt="Imagine" - class="h-auto w-auto" + class="h-9 w-auto" /> </div>
50-54: Consider adding accessibility attributes to the external link.The banner link opens an external site. Adding
aria-labelwould improve screen reader experience.<a href="https://imagine.dev" target="_blank" rel="noopener noreferrer" + aria-label="Learn more about Imagine - opens in new tab" class="group relative mx-auto flex w-full items-center py-4 md:h-[76px] bg-black cursor-pointer" >src/routes/(marketing)/(components)/teaser/imagination.svelte (3)
290-290: Avoidas anytype cast.Casting
materialtoanybypasses TypeScript's type checking. Consider using a more specific type or addressing the underlying type mismatch.-<T.Mesh scale={[$viewport.width, $viewport.height, 1]} material={material as any}> +<T.Mesh scale={[$viewport.width, $viewport.height, 1]} material={material}>If there's a type incompatibility with Threlte, you may need to type the material state more specifically or use a type assertion to
Material.
15-15: Unusedinteractivity()call.The
interactivity()function is called but its return value is not used, and there are no interactive Threlte elements (likeon:clickhandlers on meshes) in this component. If interactivity isn't needed, this can be removed.
26-31: Consider consolidating mouse state variables.Six separate state variables for mouse tracking could be consolidated into objects for cleaner code.
let mouse = $state({ x: 0, y: 0 }); let targetMouse = $state({ x: 0, y: 0 }); let prevMouse = $state({ x: 0, y: 0 });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/routes/(marketing)/(components)/teaser/shaders/noise.glslis excluded by!**/*.glslstatic/images/logos/Imagine.pngis excluded by!**/*.png
📒 Files selected for processing (6)
package.json(3 hunks)src/global.d.ts(1 hunks)src/lib/layouts/Main.svelte(2 hunks)src/routes/(marketing)/(components)/teaser/canvas.svelte(1 hunks)src/routes/(marketing)/(components)/teaser/imagination.svelte(1 hunks)src/routes/(marketing)/(components)/teaser/teaser-banner.svelte(1 hunks)
🔇 Additional comments (4)
package.json (1)
112-112: Verify the Vite version downgrade.The Vite version appears to have been downgraded from
^7.2.10to^7.2.7. Was this intentional? If so, please document the reason (e.g., compatibility issue with Threlte).src/global.d.ts (1)
3-6: LGTM!The module declaration correctly types GLSL shader imports using Vite's
?rawquery parameter, enabling TypeScript support for the shader files used in the imagination component.src/routes/(marketing)/(components)/teaser/canvas.svelte (1)
13-20: LGTM!The canvas wrapper is well-structured with appropriate performance optimizations (
contain,transform: translate3d,backface-visibility). The fixeddpr={1}is a reasonable tradeoff for banner performance over sharpness on high-DPI displays.src/routes/(marketing)/(components)/teaser/imagination.svelte (1)
283-285: Verify the lerp smoothing factor calculation.The formula
1 - Math.pow(0.0003, actualDelta)produces values very close to 1 for typical frame deltas (~0.033s at 30fps), resulting in near-instant snapping rather than smooth interpolation. Was this intentional, or should the base/exponent be adjusted for smoother mouse following?A typical frame-rate-independent lerp uses something like:
const lerpFactor = 1 - Math.pow(smoothing, actualDelta * 60); // where smoothing is 0.8-0.95
| "svgtofont": "^4.2.3", | ||
| "tailwind-merge": "^3.4.0", | ||
| "tailwindcss": "^4.1.17", | ||
| "three": "^0.176.0", |
There was a problem hiding this comment.
three should be in dependencies, not devDependencies.
The three library is a runtime dependency used in the browser for the 3D canvas rendering. It should be moved to the dependencies section to ensure it's included in production builds.
"dependencies": {
"@sentry/sveltekit": "^10.28.0",
"h3": "^1.15.4",
- "sharp": "^0.34.5"
+ "sharp": "^0.34.5",
+ "three": "^0.176.0"
},And remove it from devDependencies.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In package.json around line 107, the "three" package is currently listed under
devDependencies but is a runtime/browser dependency; move the "three":
"^0.176.0" entry into the top-level "dependencies" object and remove it from
"devDependencies", then run your package manager (npm/yarn/pnpm) to update
node_modules and the lockfile so production builds include the library.
src/lib/layouts/Main.svelte
Outdated
| <TeaserBanner | ||
| showLabel={true} | ||
| leftText="Introducing" | ||
| logoText="Imagine" | ||
| rightText="Build something real" | ||
| /> |
There was a problem hiding this comment.
showLabel={true} appears to be a debug setting.
The showLabel prop renders a "Teaser Banner" label and code icon positioned above the banner (visible at lines 28-46 in teaser-banner.svelte). This looks like development/debug UI that should likely be disabled for production.
<TeaserBanner
- showLabel={true}
+ showLabel={false}
leftText="Introducing"
logoText="Imagine"
rightText="Build something real"
/>📝 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.
| <TeaserBanner | |
| showLabel={true} | |
| leftText="Introducing" | |
| logoText="Imagine" | |
| rightText="Build something real" | |
| /> | |
| <TeaserBanner | |
| showLabel={false} | |
| leftText="Introducing" | |
| logoText="Imagine" | |
| rightText="Build something real" | |
| /> |
🤖 Prompt for AI Agents
In src/lib/layouts/Main.svelte around lines 168 to 173, the TeaserBanner is
passing showLabel={true} which enables a development/debug label and icon;
change this to showLabel={false} (or remove the prop if the default is false) so
the debug UI is disabled in production, then verify the teaser-banner no longer
renders the label above the banner.
| }); | ||
| </script> | ||
|
|
||
| <T.OrthographicCamera position={[0, 0, 10]} fov={10} near={0.1} far={1000} makeDefault /> |
There was a problem hiding this comment.
fov is not a valid prop for OrthographicCamera.
OrthographicCamera uses left, right, top, bottom (or relies on viewport sizing), not fov which is specific to PerspectiveCamera. This prop will be silently ignored.
-<T.OrthographicCamera position={[0, 0, 10]} fov={10} near={0.1} far={1000} makeDefault />
+<T.OrthographicCamera position={[0, 0, 10]} near={0.1} far={1000} makeDefault />📝 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.
| <T.OrthographicCamera position={[0, 0, 10]} fov={10} near={0.1} far={1000} makeDefault /> | |
| <T.OrthographicCamera position={[0, 0, 10]} near={0.1} far={1000} makeDefault /> |
🤖 Prompt for AI Agents
In src/routes/(marketing)/(components)/teaser/imagination.svelte around line
289, the OrthographicCamera is being passed a non‑existent fov prop (fov is for
PerspectiveCamera); remove the fov prop and either supply appropriate
orthographic parameters (left, right, top, bottom, near, far) or replace
OrthographicCamera with PerspectiveCamera if a field‑of‑view is required; update
the JSX/props accordingly so the camera receives only valid props and the
intended projection is achieved.
| <svg | ||
| class="h-5 w-5 transition group-hover:-translate-x-0.5" | ||
| fill="none" | ||
| viewBox="0 0 24 24" | ||
| stroke="currentColor" | ||
| stroke-width="2" | ||
| > | ||
| <path stroke-linecap="round" stroke-linejoin="round" d="M17 8l4 4m0 0l-4 4m4-4H3" /> | ||
| </svg> |
There was a problem hiding this comment.
Arrow hover animation moves in wrong direction.
The arrow points right (→) but the hover effect translates it left (-translate-x-0.5). For a "go forward" affordance, the arrow should move right on hover.
<svg
- class="h-5 w-5 transition group-hover:-translate-x-0.5"
+ class="h-5 w-5 transition group-hover:translate-x-0.5"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
stroke-width="2"
>📝 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.
| <svg | |
| class="h-5 w-5 transition group-hover:-translate-x-0.5" | |
| fill="none" | |
| viewBox="0 0 24 24" | |
| stroke="currentColor" | |
| stroke-width="2" | |
| > | |
| <path stroke-linecap="round" stroke-linejoin="round" d="M17 8l4 4m0 0l-4 4m4-4H3" /> | |
| </svg> | |
| <svg | |
| class="h-5 w-5 transition group-hover:translate-x-0.5" | |
| fill="none" | |
| viewBox="0 0 24 24" | |
| stroke="currentColor" | |
| stroke-width="2" | |
| > | |
| <path stroke-linecap="round" stroke-linejoin="round" d="M17 8l4 4m0 0l-4 4m4-4H3" /> | |
| </svg> |
🤖 Prompt for AI Agents
In src/routes/(marketing)/(components)/teaser/teaser-banner.svelte around lines
86 to 94, the SVG arrow uses group-hover:-translate-x-0.5 which moves it left on
hover; change the hover utility to move right (e.g., replace
group-hover:-translate-x-0.5 with group-hover:translate-x-0.5) so the arrow
animates in the forward/right direction on hover, keeping the other classes (h-5
w-5 transition) intact.
What does this PR do?
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.