Skip to content

Add useAnimateScroll hook #435

Open
LauraBeatris wants to merge 2 commits into
fisshy:masterfrom
LauraBeatris:master
Open

Add useAnimateScroll hook #435
LauraBeatris wants to merge 2 commits into
fisshy:masterfrom
LauraBeatris:master

Conversation

@LauraBeatris

@LauraBeatris LauraBeatris commented Jun 11, 2020

Copy link
Copy Markdown

As discussed i issue #430, the animateScroll module isn't working when it's called in a useEffect hook on the component mount.

It's a better approach to create a hook to fix that problem instead of relying on the need of creating a ref to verify if the tree has loaded, and then execute the effect with the animateScroll method.

The hook will return a ref that will be used as the scroll container, and behind the scenes it will take care of calling it inside of a useEffect call

useEffect(() => {
    if (containerRef && containerRef.current) {
      method({
        container: containerRef.current,
        ...options,
      });
    }
  }, [
    containerRef,
    method,
    options,
  ]);

We must discuss some points here, this library uses the version ^16.0.0 of React, and hooks only are supported from version 16.8 onwards.

I upgraded React to 16.13.1, but we have to think if it's going to lead to some kind of breaking changes in other projects.

Also, I added a babel plugin to support spread operator.

@fisshy

fisshy commented Jun 12, 2020

Copy link
Copy Markdown
Owner

Great! Will look into it, otherwise we will release it as 1.8 with breaking changes, it's time to upgrade it anyway.

@alexrabin

Copy link
Copy Markdown

@fisshy why hasn't this been merged yet? Been over a year and a half

@fisshy

fisshy commented Jan 27, 2023

Copy link
Copy Markdown
Owner

@fisshy why hasn't this been merged yet? Been over a year and a half

I think I began working on a hook-first approach but ended up not doing it, and now its been forgotten until you woke it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants