image sequence#212
Conversation
andrre-ls
left a comment
andrre-ls
left a comment
There was a problem hiding this comment.
Some important things to refactor, but good first start to the component
| } | ||
|
|
||
| interface ImageSequenceProps extends ComponentPropsWithoutRef<"canvas"> { | ||
| frames: ImageMetadata[]; |
There was a problem hiding this comment.
We should not be using astro/vite types in this component. Please define the component's own type internally.
| import type { ComponentPropsWithoutRef, Ref } from "react"; | ||
| import { useCallback, useEffect, useImperativeHandle, useRef } from "react"; | ||
|
|
||
| const clamp = (min: number, value: number, max: number) => Math.max(min, Math.min(value, max)); |
There was a problem hiding this comment.
There's already a clamp foundations utility: https://foundations.significa.co/utils/math#clamp
| ```tsx | ||
| const frames = Object.values( | ||
| import.meta.glob<{ default: ImageMetadata }>("./path/to/frames/*.webp", { eager: true }) | ||
| ).map((m) => m.default); | ||
|
|
||
| const MyComponent = () => { | ||
| const sequenceRef = useRef<ImageSequenceRef>(null); | ||
|
|
||
| useEffect(() => { | ||
| const onScroll = () => { | ||
| const maxScroll = document.body.scrollHeight - window.innerHeight; | ||
| const progress = maxScroll > 0 ? window.scrollY / maxScroll : 0; | ||
| sequenceRef.current?.scrub(progress); | ||
| }; | ||
|
|
||
| window.addEventListener("scroll", onScroll, { passive: true }); | ||
| return () => window.removeEventListener("scroll", onScroll); | ||
| }, []); | ||
|
|
||
| return <ImageSequence frames={frames} ref={sequenceRef} />; | ||
| }; | ||
| ``` |
There was a problem hiding this comment.
I think we should simplify this example. First, the should not use vite's glob import and instead pass in a "regular" array of images sources. Secondly, I think it'd be better to use motion's useScroll hook to drive the scroll progress.
| ## Limitations | ||
|
|
||
| - Depends on `ImageMetadata` from Astro — the `width` and `height` properties are used to size the canvas at the native aspect ratio. | ||
| - `import.meta.glob` patterns must be static string literals, so the glob cannot be passed as a prop. Define it in the consuming file and pass the resulting array as `frames`. |
There was a problem hiding this comment.
I already alluded this in previous comments, but these limitations should not exist. Both are coupling the component to the build framework, which is something no foundations component should ever do.
| interface ImageSequenceRef { | ||
| scrub: (progress: number) => void; | ||
| } |
There was a problem hiding this comment.
Not a big fan of this. Binding methods to refs is somewhat anti-pattern in react. We should brainstorm a better way of doing this, maybe through a context provider.
| const sources = frames | ||
| .map(({ src }) => src) | ||
| .sort((a, b) => { | ||
| const basenameA = a.split("/").pop(); | ||
| const basenameB = b.split("/").pop(); | ||
|
|
||
| const indexA = parseInt(basenameA?.split("-")[0] ?? "0", 10); | ||
| const indexB = parseInt(basenameB?.split("-")[0] ?? "0", 10); | ||
|
|
||
| return indexA - indexB; | ||
| }); |
There was a problem hiding this comment.
The component should not rely on frame filename formatting. The component should assume the frames passed in via the frames prop are already sorted.
2 participants
No description provided.