Create carousel hooks #132
Conversation
leroykorterink
added
enhancement
packages/react-animation/src/carousel/hooks/useCarouselBounds.ts
Outdated
Show resolved
Hide resolved
| getElementOffset<T extends HTMLElement>(element: T): number; | ||
| getElementSize<T extends HTMLElement>(element: T, sizeType?: SizeType): number; |
There was a problem hiding this comment.
I might be missing something, but what's the benefit of the T generic here? It's only used in one place (the element parameter), so the type could be specified just there as well?
packages/react-animation/src/carousel/hooks/useCarouselControls.ts
Outdated
Show resolved
Hide resolved
packages/react-animation/src/carousel/hooks/useCarouselInfiniteTransform.ts
Outdated
Show resolved
Hide resolved
| const size = getCarouselSize(triggerRef.current, type); | ||
| const position = gsap.getProperty(proxyRef.current, type) as number; | ||
|
|
||
| return (position % size) / size; |
There was a problem hiding this comment.
Does this correctly handle negative values for position, since getCarouselIndex tried to use Math.abs, might be needed her as well, or at least the correct version of that.
There was a problem hiding this comment.
Yes this didn't worked for me, also the calculations were wrong because it didn't take the container width into account. This worked for me:
return Math.abs( (position % (size - triggerRef.current.offsetWidth + 0.01)) / (size - triggerRef.current.offsetWidth + 0.01), );
| /** | ||
| * Returns a number between 0 and 1 representing the progress of the target slide. | ||
| * | ||
| * -1 represents the slide being completely off the left side of the carousel. | ||
| * 1 represents the slide being completely off the right side of the carousel. | ||
| * 0 represents the slide being at the left side of the carousel. | ||
| */ |
There was a problem hiding this comment.
We probably should have a few definitions and examples to clarify this better.
For example:
- what does "the carousel" mean in this context? Is it the visible bounding box where the carousel sits on the screen (e.g. in a container, that is often full-bleed, or could have paddings)
- or is it the focussed around a single "focus" slot that would be marked as a "center" one
- or something else
And then how that would impact the progress, I presume the following, in a carousel where 3 items would fit on screen:
- if a slide moves from the left side off the screen to it's left position, that would move for example 300px from -1 to 0.
- then if the slide would continue moving to the right, it would travel the same distance each time, but instead go from 0 to 0.33 (position 2), then 0.33 to 0.66 (position 3), and finally to progress 1 (off screen).
If the above is true, this would also mean that if you resize your screen, and suddenly 4 items would fit in the carousel, the progress of a slide at position one would be updated from 0.33 to 0.25, even though it wouldn't move.
I'm not saying this is bad, it's just that this isn't clearly documented yet, and might be important to understand to make use of it.
leroykorterink
changed the title
| 0, | ||
| children.findIndex( | ||
| // prettier-ignore | ||
| (child) => getElementOffset(child) + (getElementSize(child) * alignment) > ((position % size) + size) % size, |
There was a problem hiding this comment.
This didn't seem to work for me.
Index started at 0 and then went to 7 ( max ) and then it counted down.
This seemed to fix it ( maybe there is a better way to fix it tho ):
return Math.max( 0, children.findIndex( // prettier-ignore (child) => getElementOffset(child) + (getElementSize(child) * alignment) > Math.abs((((position % size) + size) % size) - size), ), );
| console.warn("Can't set bounds, sliderRef is undefined"); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
In foundation I added an extra check for the example AutoHeight to work without an insane amount of re-renders. Please see full code changes in foundation.
if (type === CarouselType.X && containerWidth.current === sliderRef.current.offsetWidth) { return; }
| draggable.current.applyBounds(variablesRef.current?.bounds ?? null); | ||
| }, [draggable, variablesRef]); | ||
|
|
||
| const onResize = useRafCallback(() => { |
There was a problem hiding this comment.
In foundation I added an extra check for the example AutoHeight to work without an insane amount of re-renders. Please see full code changes in foundation.
if (type === CarouselType.X && containerWidth.current === triggerRef.current?.offsetWidth) { return; }
| export type CarouselControls = { | ||
| next(): Promise<void>; | ||
| previous(): Promise<void>; | ||
| index(index: number): Promise<void>; |
There was a problem hiding this comment.
I've added the option to set initial index to Foundation. See comments below for the code I've added.
Still behaves a bit buggy:
setInitial(index: number): Promise<void>;
|
|
||
| return useMemo<CarouselControls>(() => { | ||
| function getPositionUpdater<T extends keyof CarouselControls>( | ||
| control?: T, |
| await gsap.to(proxyRef.current, { | ||
| ..._tweenVariables, | ||
| [type]: position, | ||
| onUpdate() { |
There was a problem hiding this comment.
onUpdate() { if (!immediate) { update?.(); } }
onComplete() { update?.(); }
| next: getPositionUpdater('next'), | ||
| previous: getPositionUpdater('previous'), | ||
| index: getPositionUpdater(), | ||
| }; |
There was a problem hiding this comment.
setInitial: getPositionUpdater('index', true)
| @@ -12,7 +12,8 @@ export default { | |||
| }; | |||
|
|
|||
| const Amount = 6; | |||
| const Elements = Array.from({ length: Amount }); | |||
| /* eslint-disable unicorn/new-for-builtins */ | |||
| const Elements: Array<number> = [...Array(8).keys()]; | |||
There was a problem hiding this comment.
Nitpicking :), this variable should start with a lower case character.
| return; | ||
| } | ||
| [draggableRef.current = null] = draggable.create(target.current, { | ||
| ...variables, |
There was a problem hiding this comment.
Would it be possible to create a new draggable instance when any of the dependencies change? It's not reactive at all now which, this isn't in line with how the other hooks work that we create.
No description provided.