Swanson-Tyler / react-text-reveal

A react text reveal tool.
17 stars 3 forks source link

Height is collapsed until render #4

Open ndimatteo opened 4 years ago

ndimatteo commented 4 years ago

Hey there @Swanson-Tyler!

  1. I'm using the CharacterReveal in a Gatsby-powered site.
  2. The animation (canPlay) is triggered via state, which is subsequently set through a useEffect after a short delay.

However, I'm noticing on my production server that there is a small "jump" where before the reveal element is built there is nothing visible, collapsing my page height briefly until react-text-reveal renders to the page:

jump-reveal

Here's a live version for you to preview as well: https://the-fold-web.netlify.app/

And here's my code for the react-text-reveal component:

<CharacterReveal
  animateOpacity
  canPlay={isRevealed}
  characterOffsetDelay={60} // ms
  characterWordSpacing={'.25em'}
  copy={[
    'A collection of',
    'cutting edge homes',
    'in the heart of Shaw'
  ]}
  direction={'bottom'}
  duration={600} // ms
  ease={
    'cubic-bezier(0.5848375451263538,-0.003374999999999906,0.16606498194945848,1.012625)'
  }
  offset={'1em'}
  multilineMasking
  multilineOffsetDelay={350} // ms
/>

I think simply adding an onLoad prop where I can add some loading logic to render the text by default, and then replace it when react-text-reveal kicks in, would solve this problem?

Let me know what you think, happy to provide more info if needed ✌️

ndimatteo commented 4 years ago

For your convenience, I also just submitted a pull request with this option added!

Swanson-Tyler commented 4 years ago

@ndimatteo Thanks for the detailed description and the link! Sorry for the delayed response. Also, I know debugging gatsby production vs develop is a huge pain, so nice work debugging.

It looks like your link has the height placeholder working. Did you end up adding your own code to the node_modules, or did you fix some other way?

For the height issue, I think the logic for rendering the text reveal animation vs the standard copy placeholder can be done outside of the etc. My only reasoning is that in gatsby sites, I generally end up using a similar piece of 'isMounted' logic in many different spots.

This vid gives a good explanation: https://egghead.io/lessons/react-avoiding-state-flickers-in-gatsby-applications

I think you could fix the height issue with some code like this: {isMounted ? <CharacterReveal ... /> : <div style="opacity:0;">The same copy as the animation here</div> I think your onLoaded prop may be helpful for situations where you want to just have the animation immediately play without having to force some state change just to make it happen. Maybe we could call it 'playOnLoad' or something.

Let me know if this helps or if Im misunderstanding this issue

ndimatteo commented 4 years ago

Hey @Swanson-Tyler no worries at all!

I ended up using my fork with the added code you'll see in that PR to solve temporarily, but I also like your idea of using the isMounted approach, I feel silly for not thinking of that! That's basically what I did with that PR, but I just scoped it to the reveal component directly.

That being said, I do wonder if a playOnLoad would help with having it play automatically. Right now I'm having to do the following to instigate it auto-playing on page load:

useEffect(() => {
  setTimeout(function() {
    setIsRevealed(true)
  }, 50)
}, [])

I like the idea of keeping the loading logic outside of this now that I'm seeing it, but I do think adding a way to have it autoplay on load might make it feel a bit "cleaner" since that setTimeout seems a bit dirty to me.

What do you think?

Swanson-Tyler commented 4 years ago

@ndimatteo Sorry for the delayed response AGAIN ha. Yeah, I end up having to use the same setTimeout event loop technique when trying to animate on mount. I think adding the playOnLoad (name pending) will be really helpful for anyone using this package the first time. I may even add it by default/include it in the output code from the demo site.

ndimatteo commented 4 years ago

No problem at all @Swanson-Tyler ! Perhaps a better name for it is just autoPlay? That maybe makes more sense, especially for newbies using it for the first time. Otherwise, sounds good!