feat: add landing page with 3D animation and interactive components #605
Conversation
- Add HomeScreen page with animated 3D scene using Three.js - Add AnimatedScene with CounterBoy and ScreenshotBoy classes - Add CreateSection for interactive vibe creation - Add new components: ChatAnimation, DraggableCard, DraggableSection, VibesSwitch - Add useIsMobile and useSceneSetup hooks - Add Alte Haas Grotesk fonts - Add ignoreDarkMode prop to BrutalistCard and VibesButton - Update home route to show HomeScreen on root path 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
✅ Deploy Preview for fireproof-ai-builder ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overall the feature set is impressive, but there are a few notable issues: some Three.js code (e.g. textures, async font loading) risks memory or lifecycle leaks, dark-mode detection and mobile detection logic are duplicated, and HomeScreen has grown into a very large, hard-to-maintain component. Drag and scroll behaviors are implemented imperatively and could be simplified or centralised, and dynamic gradient backgrounds depend on fragile position calculations. None of these are immediate correctness bugs, but they will affect long-term maintainability and performance if left unaddressed.
Additional notes (17)
-
Maintainability |
use-vibes/base/components/BrutalistCard/BrutalistCard.styles.ts:96-97
getBrutalistCardStylepreviously fully determined visual styling; the inline comment says background/border are controlled via CSS classes, but those classes don't exist here and the component now hardcodes colors inline inBrutalistCard. This creates a confusing split of responsibilities and makes theming/dark-mode harder to extend. Consider centralising the color logic either in this style helper or in actual CSS classes, and keep the comment consistent with the implementation. -
Maintainability |
vibes.diy/pkg/app/classes/CounterBoy.ts:50-51
animateEnclosureThicknessreplaces the mesh geometry and disposes the old one, but it uses only the newdepthvalue without preserving any transforms from the original geometry. That’s fine here, but you already storeoriginalEnclosureGeometryand never use it; keeping unused state around makes the code harder to reason about and suggests a partially implemented feature. -
Maintainability |
vibes.diy/pkg/app/classes/CounterBoy.ts:757-777
dispose()removesthis.groupfrom its parent twice: once at lines 758–760 and again at 775–776. The second check is redundant and slightly obscures the actual cleanup flow. -
Maintainability |
vibes.diy/pkg/app/classes/CounterBoy.ts:313-333
CounterBoy.animateEnclosureThicknesscurrently performs an instantaneous geometry swap with a_durationparameter that is ignored. Given callers pass non-zero durations (e.g.ANIMATION_DURATIONS.EXPLOSION / 2), this is surprising and makes timing assumptions elsewhere fragile if you later add true interpolation.
If the intent is an immediate snap, the API should not pretend to support duration. If a smooth thickness change is desired, it should be implemented here rather than at the call site.
-
Performance |
vibes.diy/pkg/app/hooks/useIsMobile.ts:3-12
useIsMobiledirectly accesseswindowin an effect, which is fine in the browser, but if this code is ever rendered in a non-DOM environment (SSR, tests using jsdom selectively disabled), the hook will still define its type as returningbooleanwhile never updating. More importantly, each consumer adds its own resize listener, which can get expensive with many draggable components on the page. -
Maintainability |
vibes.diy/pkg/app/components/DraggableCard/DraggableCard.tsx:16-71
BothDraggableCardandDraggableSectionmanually imperatively updatestyle.transformand share very similar drag logic (trackingdragStart,cardPos, document listeners). This duplication increases the risk of bugs (e.g., one being updated without the other) and makes it harder to add features like keyboard accessibility or touch support uniformly. -
Compatibility |
vibes.diy/pkg/app/components/DraggableSection/DraggableSection.tsx:32-61
BothDraggableCardandDraggableSectiononly handle mouse events, so drag behaviour won't work on touch devices, even thoughuseIsMobileswitches layout. That’s consistent with disabling drag on mobile, but there’s no clear affordance indicating that the cards are fixed there. If the future plan is to support touch-dragging, this is a gap; if not, it may be worth explicitly documenting or visually hinting that dragging is desktop-only. -
Maintainability |
vibes.diy/pkg/app/components/ChatAnimation/ChatAnimation.tsx:48-112
ChatAnimationinjects a<style>tag intodocument.headon every mount. On pages where this component appears multiple times (or gets remounted), you’ll repeatedly add/remove identical keyframe definitions. While not catastrophic, this is unnecessary work and makes styles harder to trace. -
Maintainability |
vibes.diy/pkg/app/pages/HomeScreen/AnimatedScene.tsx:90-324
AnimatedScene’s master timeline stores references to anime.js animations and seeks them on eachprogresschange, but those animations are created insidemakeScoreevery timemasterTimelineSegmentsRefis empty, i.e. after mount and again after cleanup. If props or scene objects change, there’s no way to rebuild the score without leaking stale animations, and the linear array of segments makes it hard to inspect or adjust individual phases. -
Maintainability |
vibes.diy/pkg/app/pages/HomeScreen/AnimatedScene.tsx:90-365
AnimatedScenebuilds and caches a list ofanimetimeline segments, but there’s no defensive handling for the case wheremakeScorereturns an empty array (e.g. because refs aren’t ready yet). In that situation,seekTimelinesilently does nothing whileprogresscontinues to update, which can be very hard to debug. Also,lastPositionRefis never reset when a new score is built, which can cause triggers to be skipped ifmakeScoreis rebuilt after progress has already advanced. -
Readability |
vibes.diy/pkg/app/pages/HomeScreen/CreateSection.tsx:102-115
CreateWithStreaming’s auto-scroll effect callswindow.scrollToon every change inchatState.docs. For long-running sessions this may fight with user scroll intent (e.g., when someone scrolls up to read earlier content while a new token arrives). It also assumeswindowscrolling rather than a dedicated scroll container. -
Maintainability |
vibes.diy/pkg/app/pages/HomeScreen/CreateSection.tsx:266-371
CreateSectionmixes navigation, database writes, auth flow, and UI state in one very large component. TheCreateWithStreamingsub-component helps, but there is still substantial complexity (pending submit, auto-submit after login, routing state). This makes it harder to test and reason about the different flows (unauthenticated → login → create session → stream → preview). -
Maintainability |
vibes.diy/pkg/app/pages/HomeScreen/HomeScreen.tsx:72-1020
HomeScreenis a single ~1000-line component with many responsibilities: global style injection, scroll hijacking, section layout, animated-scene orchestration, and dynamic background computation. While it works, this size and scope will be hard to maintain, especially as you iterate on the landing page content. -
Performance |
vibes.diy/pkg/app/pages/HomeScreen/HomeScreen.tsx:292-387
Thewheelhandler inHomeScreenis attached with{ passive: false }and conditionally callspreventDefaultbased on various container checks. On some browsers, non-passive wheel listeners can negatively impact scroll performance, and the logic that tries to manage scroll inside multiple containers is quite intricate and potentially brittle (e.g., edge cases when one container cannot scroll further but another could). -
Maintainability |
vibes.diy/pkg/app/pages/HomeScreen/HomeScreen.styles.ts:333-563
The dynamic section background helpers (getSection1BackgroundStyle…getSection8BackgroundStyle) rely on computing absolute positions usingoffsetTopand a customgetAbsoluteTopfunction. These only recompute on mount and on a debounced resize, but if content height changes for any section (e.g. async data, font load differences), the backgrounds can drift and no longer align precisely with the sections. -
Maintainability |
vibes.diy/pkg/app/routes/home.tsx:113-117
The newHomeScreenis injected into the root route and will render whenever there’s nosessionIdandpathname === '/'. That’s fine, but it slightly overloadshome.tsxwith responsibilities (session-routing + marketing page). If you later add more routes or want to reuseHomeScreenelsewhere, this coupling may get in the way. -
Maintainability |
use-vibes/base/components/VibesButton/VibesButton.tsx:29-29
You’ve duplicated the dark-mode detection and style-merging logic fromBrutalistCardhere. This increases the chance of subtle behavior drift (e.g. border color or thresholds) between the two components over time. Given both components share the same visual system, they should share the same dark-mode implementation rather than copy-pasting.
Summary of changes
Summary of Changes
- Updated
BrutalistCardandVibesButtonto support dark mode viaignoreDarkModeprops and dynamicprefers-color-schemedetection. - Introduced a rich 3D scene system using Three.js:
- New
CounterBoyandScreenshotBoyclasses encapsulate scene logic, animations, and cleanup. - Added comprehensive scene configuration in
constants/scene.tsand object factories infactories/sceneObjects.ts. - Implemented
useSceneSetuphook to initialize, render, and dispose the Three.js scene.
- New
- Added interactive UI components:
ChatAnimationwith scroll-triggered animations.DraggableCardandDraggableSectionsupporting desktop drag and mobile/static modes.VibesSwitchSVG-based toggle control.
- Implemented a new
HomeScreenlanding page featuring:- Complex scroll-tied 3D
AnimatedScenetimeline. - Narrative sections with dynamically computed gradient backgrounds.
CreateSectionthat wires into existing auth, Fireproof DB, and chat flows, usingBrutalistCardandVibesButton.
- Complex scroll-tied 3D
- Added
useIsMobileanduseSceneSetuphooks, Alte Haas Grotesk web fonts, and wired the root route to renderHomeScreenwhen no session is active. - Added various assets (screenshots, noise textures, GIF) and font files for the new visual design.
| // Detect dark mode | ||
| const [isDark, setIsDark] = React.useState(false); | ||
|
|
||
| React.useEffect(() => { | ||
| if (ignoreDarkMode) return; | ||
|
|
||
| const checkDarkMode = () => { | ||
| setIsDark(window.matchMedia('(prefers-color-scheme: dark)').matches); | ||
| }; | ||
|
|
||
| checkDarkMode(); | ||
| const mediaQuery = window.matchMedia('(prefers-color-scheme: dark)'); | ||
| mediaQuery.addEventListener('change', checkDarkMode); | ||
|
|
||
| return () => mediaQuery.removeEventListener('change', checkDarkMode); | ||
| }, [ignoreDarkMode]); |
There was a problem hiding this comment.
This dark-mode detection runs in every BrutalistCard instance, each creating its own matchMedia listener. On pages like HomeScreen that render many cards, this can become wasteful and harder to keep consistent. A shared hook (or context) for color-scheme would avoid duplicated listeners and centralise behavior.
Suggestion
Extract this logic into a reusable hook, e.g. usePrefersDarkMode(ignore?: boolean), that sets up exactly one matchMedia listener per mount and returns isDark. Both BrutalistCard and VibesButton can then consume the hook instead of duplicating the effect logic. Reply with "@CharlieHelps yes please" if you want me to sketch that hook and wire it into both components.
| private updateDisplayTexture() { | ||
| // Create canvas for display content | ||
| const canvas = document.createElement("canvas"); | ||
| canvas.width = SCENE_DIMENSIONS.DISPLAY.CANVAS.WIDTH; | ||
| canvas.height = SCENE_DIMENSIONS.DISPLAY.CANVAS.HEIGHT; | ||
| const ctx = canvas.getContext("2d"); | ||
| if (!ctx) return; | ||
|
|
||
| // Draw simple content on canvas | ||
| ctx.fillStyle = "#ffffff"; | ||
| ctx.fillRect(0, 0, canvas.width, canvas.height); | ||
|
|
||
| // Add some text | ||
| ctx.fillStyle = COLORS.DISPLAY_TEXT; | ||
| ctx.font = `${SCENE_DIMENSIONS.DISPLAY.FONT_SIZE}px dseg7_classic_miniregular`; | ||
| // right | ||
| ctx.textAlign = "right"; | ||
| ctx.fillText( | ||
| this.encryptedBlocksCount.toString(), | ||
| SCENE_DIMENSIONS.DISPLAY.TEXT_X, | ||
| canvas.height / 2 + SCENE_DIMENSIONS.DISPLAY.FONT_SIZE / 2, | ||
| ); | ||
|
|
||
| // Update texture | ||
| const texture = new THREE.CanvasTexture(canvas); | ||
| texture.needsUpdate = true; | ||
|
|
||
| // Handle material array (display uses an array of materials with texture on index 1 and 2) | ||
| if (Array.isArray(this.display.material)) { | ||
| const textMaterial2 = this.display.material[2] as THREE.MeshToonMaterial; | ||
| if (textMaterial2) { | ||
| textMaterial2.map = texture; | ||
| textMaterial2.needsUpdate = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
updateDisplayTexture allocates a new <canvas> and THREE.CanvasTexture every time the encrypted block count changes, but the previous texture is never disposed. Over many updates this will leak GPU memory. Since the display is updated frequently as blocks are encrypted, you should either reuse a single texture or explicitly dispose the old one.
Suggestion
Keep a THREE.CanvasTexture instance on the class (e.g. private displayTexture?: THREE.CanvasTexture) and update its image rather than creating a new texture each time:
if (!this.displayTexture) {
this.displayTexture = new THREE.CanvasTexture(canvas);
} else {
this.displayTexture.image = canvas;
this.displayTexture.needsUpdate = true;
}
// assign this.displayTexture to the material once in constructorIf you prefer keeping the current pattern, ensure you dispose the old texture before replacing it: textMaterial2.map?.dispose(). Reply with "@CharlieHelps yes please" if you want me to add that texture reuse/disposal logic.
| private startTextureAnimation(encryptedBlock: THREE.Mesh) { | ||
| let frameCount = 0; | ||
| const material = encryptedBlock.material as THREE.MeshToonMaterial; | ||
|
|
||
| // Store initial offsets for deterministic animation | ||
| const initialX = material?.map?.offset.x || 0; | ||
| const initialY = material?.map?.offset.y || 0; | ||
|
|
||
| const animateTexture = () => { | ||
| if (!this.isBlockAnimating) return; | ||
|
|
||
| // Deterministic texture animation based on frame count and initial offset | ||
| frameCount++; | ||
| if (frameCount % 3 === 0 && material && material.map) { | ||
| // Create deterministic but random-looking pattern using sine waves | ||
| const time = frameCount * 0.1; | ||
| material.map.offset.x = (initialX + Math.sin(time * 0.7) * 0.2) % 1; | ||
| material.map.offset.y = (initialY + Math.cos(time * 0.5) * 0.2) % 1; | ||
| material.map.needsUpdate = true; | ||
| } | ||
|
|
||
| if (this.isBlockAnimating) { | ||
| requestAnimationFrame(animateTexture); | ||
| } | ||
| }; | ||
| animateTexture(); | ||
| } | ||
|
|
||
| private stopTextureAnimation() { | ||
| // Texture animation will stop when isBlockAnimating becomes false | ||
| } |
There was a problem hiding this comment.
CounterBoy mixes anime.js animations with requestAnimationFrame-driven texture tweens, but there is no central mechanism to cancel the texture animation loop. stopTextureAnimation relies on isBlockAnimating being set to false, which is fine in the happy path, but:
- If the block animation is canceled or errors, the RAF loop may continue indefinitely.
- There is no reference to the RAF id, so it can’t be explicitly canceled during
dispose().
In a long-lived landing page, this can easily turn into a subtle memory/performance leak.
Suggestion
Track the requestAnimationFrame id and clear it explicitly on stop/dispose:
private textureAnimationFrame: number | null = null;
private startTextureAnimation(encryptedBlock: THREE.Mesh) {
const loop = () => {
if (!this.isBlockAnimating) return;
// ...update texture...
this.textureAnimationFrame = requestAnimationFrame(loop);
};
this.textureAnimationFrame = requestAnimationFrame(loop);
}
private stopTextureAnimation() {
if (this.textureAnimationFrame != null) {
cancelAnimationFrame(this.textureAnimationFrame);
this.textureAnimationFrame = null;
}
}
public dispose() {
this.stopTextureAnimation();
// existing cleanup...
}This ensures no stray RAF loops survive after animations or component unmount. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this fix.
| export function makeCid(hexPair: string) { | ||
| const loader = new FontLoader(); | ||
| const textGroup = new THREE.Group(); | ||
|
|
||
| loader.load(EXTERNAL_URLS.HELVETICA_FONT, function (font) { | ||
| const textGeometry = new TextGeometry(hexPair, { | ||
| font: font, | ||
| size: 0.3, | ||
| depth: 0.02, | ||
| curveSegments: 8, | ||
| bevelEnabled: false, | ||
| }); | ||
|
|
||
| // Center the text | ||
| // textGeometry.computeBoundingBox() | ||
| // const textWidth = textGeometry.boundingBox!.max.x - textGeometry.boundingBox!.min.x | ||
| // const textHeight = textGeometry.boundingBox!.max.y - textGeometry.boundingBox!.min.y | ||
| // textGeometry.translate(-textWidth / 2, -textHeight / 2, 0) | ||
|
|
||
| const textMaterial = new THREE.MeshToonMaterial({ color: 0x444444 }); | ||
| const textMesh = new THREE.Mesh(textGeometry, textMaterial); | ||
|
|
||
| // Position on the grid behind the encrypted block (now that blocks are flat) | ||
| textMesh.rotation.z = -Math.PI / 2; | ||
| textMesh.position.x = -0.25; | ||
| textMesh.position.y = -1; | ||
| textGroup.add(textMesh); | ||
| }); | ||
|
|
||
| return textGroup; | ||
| } |
There was a problem hiding this comment.
makeCid uses FontLoader.load asynchronously and attaches the resulting mesh to textGroup, but the returned group is added to the scene immediately. If the parent CounterBoy/scene is disposed before the font finishes loading, the loader callback will still run and attach meshes into a group that may no longer be referenced or rendered, and their materials/geometries won’t be disposed. This is a subtle lifecycle leak around async loading.
Suggestion
Track a disposed flag or accept an optional AbortSignal so you can bail out of the loader callback when the owning object is torn down:
export function makeCid(hexPair: string, options?: { signal?: AbortSignal }) {
const textGroup = new THREE.Group();
const loader = new FontLoader();
loader.load(EXTERNAL_URLS.HELVETICA_FONT, font => {
if (options?.signal?.aborted) return;
// ...create textMesh and add to textGroup
});
return textGroup;
}Then pass an AbortController from CounterBoy and abort it in dispose. Reply with "@CharlieHelps yes please" if you’d like me to introduce this pattern and propagate the signal.
| useEffect(() => { | ||
| if (isMobile) return; // No ejecutar en móvil | ||
|
|
||
| const innerContainer = innerContainerRef.current; | ||
| const chatContainer = chatContainerRef.current; | ||
| const animatedContainer = animatedSceneContainerRef.current; | ||
| const animatedSection4Container = animatedSceneSection4Ref.current; | ||
| const animatedSection6Container = animatedSceneSection6Ref.current; | ||
| if (!innerContainer || !chatContainer) return; | ||
|
|
||
| const handleWheel = (e: WheelEvent) => { | ||
| // Check if the scroll event is happening inside the Section 2 animated scene container | ||
| const isInsideSection2AnimatedScene = | ||
| animatedSection4Container && | ||
| animatedSection4Container.contains(e.target as Node); | ||
|
|
||
| // Check if the scroll event is happening inside the Section 4 animated scene container | ||
| const isInsideSection4AnimatedScene = | ||
| animatedSection6Container && | ||
| animatedSection6Container.contains(e.target as Node); | ||
|
|
||
| // Check if the scroll event is happening inside the animated scene container | ||
| const isInsideAnimatedScene = | ||
| animatedContainer && animatedContainer.contains(e.target as Node); | ||
|
|
||
| // For Section 2 and 4, let them scroll naturally - the scroll event listener will handle progress updates | ||
| if (isInsideSection2AnimatedScene || isInsideSection4AnimatedScene) { | ||
| // Don't prevent default, let the native scroll happen | ||
| return; | ||
| } else if (isInsideAnimatedScene && animatedContainer) { | ||
| // Handle animated scene scrolling | ||
| const { scrollTop, scrollHeight, clientHeight } = animatedContainer; | ||
| const isScrollable = scrollHeight > clientHeight; | ||
| const isAtTop = scrollTop <= 0; | ||
| const isAtBottom = scrollTop + clientHeight >= scrollHeight - 1; | ||
|
|
||
| if (isScrollable) { | ||
| if (e.deltaY > 0 && !isAtBottom) { | ||
| e.preventDefault(); | ||
| animatedContainer.scrollTop += e.deltaY; | ||
|
|
||
| // Update progress immediately | ||
| const newScrollTop = animatedContainer.scrollTop; | ||
| const scrollProgress = | ||
| scrollHeight > clientHeight | ||
| ? (newScrollTop / (scrollHeight - clientHeight)) * 100 | ||
| : 0; | ||
| setAnimationProgress(Math.max(0, Math.min(100, scrollProgress))); | ||
| return; | ||
| } | ||
|
|
||
| if (e.deltaY < 0 && !isAtTop) { | ||
| e.preventDefault(); | ||
| animatedContainer.scrollTop += e.deltaY; | ||
|
|
||
| // Update progress immediately | ||
| const newScrollTop = animatedContainer.scrollTop; | ||
| const scrollProgress = | ||
| scrollHeight > clientHeight | ||
| ? (newScrollTop / (scrollHeight - clientHeight)) * 100 | ||
| : 0; | ||
| setAnimationProgress(Math.max(0, Math.min(100, scrollProgress))); | ||
| return; | ||
| } | ||
| } | ||
| } else if (chatContainer) { | ||
| // Handle chat scrolling | ||
| const { scrollTop, scrollHeight, clientHeight } = chatContainer; | ||
| const isScrollable = scrollHeight > clientHeight; | ||
| const isAtTop = scrollTop <= 0; | ||
| const isAtBottom = scrollTop + clientHeight >= scrollHeight - 1; | ||
|
|
||
| if (isScrollable) { | ||
| if (e.deltaY > 0 && !isAtBottom) { | ||
| e.preventDefault(); | ||
| chatContainer.scrollTop += e.deltaY; | ||
| return; | ||
| } | ||
|
|
||
| if (e.deltaY < 0 && !isAtTop) { | ||
| e.preventDefault(); | ||
| chatContainer.scrollTop += e.deltaY; | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Si el chat ya llegó al final o principio, dejamos que el scroll continúe naturalmente | ||
| }; | ||
|
|
||
| // Escuchar el evento de rueda desde *cualquier parte del innerContainer* | ||
| innerContainer.addEventListener("wheel", handleWheel, { passive: false }); | ||
| return () => { | ||
| innerContainer.removeEventListener("wheel", handleWheel); | ||
| }; | ||
| }, [isMobile]); | ||
|
|
||
| // Direct scroll listener for Section 2 AnimatedScene (0-50) | ||
| useEffect(() => { | ||
| const animatedSection4Container = animatedSceneSection4Ref.current; | ||
| const section4Element = section4Ref.current; | ||
| if (!animatedSection4Container || !section4Element) { | ||
| return; | ||
| } | ||
|
|
||
| let isScrolling = false; | ||
|
|
||
| const handleScroll = () => { | ||
| const { scrollTop, scrollHeight, clientHeight } = | ||
| animatedSection4Container; | ||
| const scrollProgress = | ||
| scrollHeight > clientHeight | ||
| ? (scrollTop / (scrollHeight - clientHeight)) * 50 | ||
| : 0; | ||
| setAnimationProgress(Math.max(0, Math.min(50, scrollProgress))); | ||
|
|
||
| // Center the section when scrolling starts | ||
| if (!isScrolling) { | ||
| isScrolling = true; | ||
| section4Element.scrollIntoView({ | ||
| behavior: "smooth", | ||
| block: "center", | ||
| inline: "center", | ||
| }); | ||
|
|
||
| // Reset flag after scroll animation completes | ||
| setTimeout(() => { | ||
| isScrolling = false; | ||
| }, 1000); | ||
| } | ||
| }; | ||
|
|
||
| animatedSection4Container.addEventListener("scroll", handleScroll, { | ||
| passive: true, | ||
| }); | ||
| return () => { | ||
| animatedSection4Container.removeEventListener("scroll", handleScroll); | ||
| }; | ||
| }, []); | ||
|
|
||
| // Direct scroll listener for Section 4 AnimatedScene (50-100) | ||
| useEffect(() => { | ||
| const animatedSection6Container = animatedSceneSection6Ref.current; | ||
| const section6Element = section6Ref.current; | ||
| if (!animatedSection6Container || !section6Element) { | ||
| return; | ||
| } | ||
|
|
||
| let isScrolling = false; | ||
|
|
||
| const handleScroll = () => { | ||
| const { scrollTop, scrollHeight, clientHeight } = | ||
| animatedSection6Container; | ||
| const scrollProgress = | ||
| scrollHeight > clientHeight | ||
| ? 50 + (scrollTop / (scrollHeight - clientHeight)) * 50 | ||
| : 50; | ||
| setAnimationProgress(Math.max(50, Math.min(100, scrollProgress))); | ||
|
|
||
| // Center the section when scrolling starts | ||
| if (!isScrolling) { | ||
| isScrolling = true; | ||
| section6Element.scrollIntoView({ | ||
| behavior: "smooth", | ||
| block: "center", | ||
| inline: "center", | ||
| }); | ||
|
|
||
| // Reset flag after scroll animation completes | ||
| setTimeout(() => { | ||
| isScrolling = false; | ||
| }, 1000); | ||
| } | ||
| }; | ||
|
|
||
| animatedSection6Container.addEventListener("scroll", handleScroll, { | ||
| passive: true, | ||
| }); | ||
| return () => { | ||
| animatedSection6Container.removeEventListener("scroll", handleScroll); | ||
| }; |
There was a problem hiding this comment.
The scroll handling in HomeScreen is quite complex: you’re mixing global wheel listeners, nested scroll containers, and multiple scroll handlers that all update animationProgress. The current logic:
- Greedily calls
scrollIntoView({ behavior: 'smooth' })on first scroll within sections 4 and 6, which can fight the user’s own scroll momentum. - Duplicates the same
AnimatedSceneinstance for sections 4 and 6, both driven by a sharedanimationProgressstate, making it difficult to reason which scroll container is authoritative. - Uses magic numbers for scroll ranges (0–50, 50–100) without centralizing them.
This is very brittle and will be painful to maintain or tweak.
Suggestion
Consider simplifying the scroll → animation mapping:
- Use a single
IntersectionObserverorgetBoundingClientRect-based calculation relative to the viewport to derive a normalized progress for each section, instead of hard-coded 0–50/50–100 splits and nested scroll containers. - Avoid calling
scrollIntoViewin response to scroll events; instead, use layout (e.g. tall sticky container) to keep the 3D scene centered while the user scrolls naturally. - Restrict the
wheelhijacking to only the animated-scene container, and let the rest of the page use native scroll behavior.
This will yield a much smoother UX and a clearer mental model for how scroll controls the timeline. Reply with "@CharlieHelps yes please" if you'd like me to sketch a refactor for this interaction.
|
@CharlieHelps apply your suggestions |
Expand this to see my work.
|
charliecreates
bot
force-pushed
the
landing-page-essentials
branch
from
20a74a6 to
0216f76
Compare
|
I’ve applied the concrete suggestions from my earlier review and tightened up the 3D scene lifecycles. Summary below. Changes
Verification# Monorepo full check (format, build, lint, tests, extra hosting tests)
$ pnpm check
# Result: ❌ fails (pre-existing / cross-package issues), with key points:
# - Format: Prettier ran successfully; no formatting diffs after final changes.
# - Build/typecheck: core CLI tsc reports multiple issues outside the landing-page changes:
# - prompts/tests/vitest.browser.config.ts:
# * Vitest/Vite config type incompatibilities due to multiple versions.
# - use-vibes/base/index.ts:
# * Type mismatch: ManualRedirectStrategy not assignable to TokenStrategie
# (Logger/URI type incompatibilities between @adviser/cement 0.4.64 vs 0.4.53).
# - use-vibes/tests/utils/iframe-mocks.ts:
# * vi.Mock assigned to iframe addEventListener/removeEventListener fields (signature mismatch).
# - vibes.diy/tests/app/useViewState.test.tsx:
# * vi.Mock passed where React Router’s NavigateFunction is expected (several locations).
# - hosting/tests/unit/endpoints/appCreate.test.ts:
# * Context/route types and JSON response shapes no longer matching updated Hono/OpenAPI types.
# - The check run eventually hits a Node “JavaScript heap out of memory” error while reporting
# the large cross-package type mismatch set.
# Scoped TypeScript check for vibes.diy app
$ pnpm exec tsc -p vibes.diy/pkg/tsconfig.dev.json --noEmit
# Result: ❌ fails, but all reported errors are in hosting-tests and shared test configs,
# plus the existing ManualRedirectStrategy/TokenStrategie mismatch and test mocks
# noted above. The updated landing page files (CounterBoy, ScreenshotBoy,
# sceneObjects, AnimatedScene, HomeScreen) and the new usePrefersDarkMode hook
# compile cleanly after the three/animejs/@types/three fixes.
# vibes.diy app tests only
$ cd vibes.diy/pkg && pnpm test
# Result: ❌ Vitest exits with “No test files found”, which is expected here since the
# app package itself has no local test files (tests live in vibes.diy/tests/*).Notes on verification scope
If you’d like, I can follow up with a separate PR to tackle the broader type-cleanup items (ManualRedirectStrategy type wiring, test helper signatures, and the hosting endpoint mocks), but for this branch the landing page and 3D scene code should now be much safer and closer to production-ready. |
🤖 Generated with Claude Code