TylerBarnes / gatsby-plugin-transition-link

A link component for page transitions in gatsby
537 stars 71 forks source link

Layouts and position absolute #27

Open alanonthegit opened 5 years ago

alanonthegit commented 5 years ago

Hey @TylerBarnes thanks again for the awesome plugin.

I have it configured and running with the option of a persistent layout (as described in your docs) and it's working properly.

However, I'm new to this and not sure if there is any way to pass in classes to the following div. As you can see, I have a flex parent with a left navigation that takes up 1/4 width, and I need to assign a class to the second child, but I am unable to do so as it is part of the TransitionGroup. Any way to do that?

image

Also I noticed that tl-wrapper is set to position: absolute and this places my content above my navigation if I add a left: 0.

image

If I don't set it to left: 0, then the frame is in the right area, but the div now goes past the viewport and introduces horizontal overflow/scroll.

image

Is there any reason this can't be position: relative? I haven't actually started doing animations just yet but trying to get the scaffolding working as to how it was prior to switching to this Layout implementation.

If it was position: relative, and I could access the div above to assign a width value, then this layout would be fixed. Open to ideas/suggestions!

alanonthegit commented 5 years ago

Also, if that div wasn't there (as part of a react fragment), I could directly target tl-wrapper as a child of my flex parent and set its width that way

TylerBarnes commented 5 years ago

Hey, this is a little tricky because position absolute is added by react-transition-group. I believe RTG has an option to specify what the container element should be so I can add a plugin option for that (if I'm remembering correctly). I should be able to have a fix in for this fairly soon. What do you think the most optimal solution would be? Do you want to be able to remove the div, change it to a different element, or add a class to it? Or all three?

alanonthegit commented 5 years ago

Hey Tyler, thanks for the prompt response!

I think ideally all 3 would be useful but I was able to "fix" the layout with more markup on my end. But it feels counterintuitive to the default behavior of position: absolute that is set by tl-wrapper since I am forcing it to be relative instead.

I haven't tried to do animations yet, so I think that's where it might come back to bite me hahah.

TylerBarnes commented 5 years ago

Haha, yeah I think if it's relative the entering page will be at the very bottom below the exiting page if they're both on the page at the same time. I'll play around a bit and see if I can add those plugin options. I think that should make it flexible enough to account for this type of issue.

TylerBarnes commented 5 years ago

hey @ar-stackrox I have a beta release out which I think solves your problems. I've removed position absolute. I found a way to make position relative work with floats, of all things! I also simply removed the div wrapper in transition group. I think a plugin option isn't necessary because you can wrap a div around using a layout if you'd like. If you have time to try it out I'd love to hear your thoughts and if it works for you. npm i gatsby-plugin-transition-link@1.5.0-beta.1 or yarn add gatsby-plugin-transition-link@1.5.0-beta.1

TylerBarnes commented 5 years ago

Looks like there was a big bug in beta.1. If you give it a try please install 1.5.0-beta.3 instead.

alanonthegit commented 5 years ago

So awesome! Thanks @TylerBarnes I'll give this a try soon

alanonthegit commented 5 years ago

hey @TylerBarnes I discovered a bug that was present in the current release and beta 3, so I unfortunately had to roll back to gatsby-plugin-layout (which is working).

I had TransitionLink replacing gatsby-plugin-layout in my gatsby-config file, and it was pointing to my index.js template. In my index.js file, I am using context to determine which layout to render with the kind of pages that gatsby-node is generating.

image

While both your plugin and gatsby-plugin-layout seemed to initially work the same (aside from the tweaks to the markup), I discovered when running Gatsby build that when I first load a Docs page, the layout that was being used to render the page was buggy. It could have been that it wasn't catching my context/condition somehow and maybe defaulted to my normal layout.

The weird thing is, if I navigated to the page from some other page (using gatby's SSR), it would render the page as intended similarly to how it worked on gatsby develop, letting me assume that on initial page load, the proper layout was being ignored.

Anyways, it works fine on gatsby-plugin-layout in build/prod mode. Just thought I'd let you know!

TylerBarnes commented 5 years ago

@ar-stackrox thanks for the detailed description! I should be able to reproduce and try it out from that. I'm super busy right now but I should have some more time to dedicate to this project a few weeks out as much as I'd like to work on it now.

alanonthegit commented 5 years ago

Thanks Tyler! No worries. I also spotted one more bug a while back that I am not encountering on gatsby-layout-plugin. When clicking around, my pages would no longer scroll back to the top. I resorted to doing this hacky thing in gatsby-browser but it's not ideal

image

TylerBarnes commented 5 years ago

v1.5.1 fixes the original issue here.

For the scroll updating I believe it may be related to mixing gatsby link with transition link since I had to disable shouldUpdateScroll as you noticed. Do you recall if you were mixing the two?

I think also for the layout issue you had it's likely due to the very minor difference between my layout implementation and gatsby-plugin-layout. I'm conditionally rendering a layout with <Layout>{children}</Layout> or just {children} where gatsby-plugin-layout isn't conditionally rendering but is changing the layout component on the fly. Thanks for pointing that out. I should have a fix for those other two issues in the next release!

tterb commented 5 years ago

Just a note, 1.5.0-beta.3 also fixes a similar issue I encountered when using the Parallax scroll container from react-spring.
Thanks for the great work!

TylerBarnes commented 5 years ago

@tterb interesting! The change from 1.5.0-beta made it into the latest release recently. Do you know if 1.5.1 also fixes that issue for you?

tterb commented 5 years ago

@TylerBarnes I just checked and the issue appears to exist in 1.5.0 and 1.5.1, though is somehow being fixed in 1.5.0-beta.3. I tried deleting the .cache directory and reinstalling the package just to be sure but the issue is still present.
Although the issue is a bit different than the one that @ar-stackrox was experiencing, where in this case, basically anything that is within the Parallax scroll container is entirely hidden.

TylerBarnes commented 5 years ago

That's super weird! Is it possible for me to debug using your site? I actually never merged that beta, I ended up rewriting the code while I was making a couple other adjustments so it could be that something is just slightly different.

tterb commented 5 years ago

@TylerBarnes Yeah, I just updated the repo with what I'm currently working with.
Some parts are a little bit of a mess since I've mainly just been using it to familiarize myself with Gatsby, but I'm currently only using gatsby-plugin-transition-link in the src/components/Nav.jsx for the site navbar.

Additionally, it also appears that AniLink using the paintDrip and swipe transitions will still cause the issue on the destination page after the transition with 1.5.0-beta.3, whereas the fade and cover transitions don't.

TylerBarnes commented 5 years ago

@tterb thanks for the link but it looks like I can't run develop on your site because I'm missing your env vars.

tterb commented 5 years ago

@TylerBarnes I'll create a page-transitions branch on the repo that you can use to debug the issue that doesn't require the env variables since they aren't necessary to observe the behavior.

TylerBarnes commented 5 years ago

@tterb thanks for the demo repo. Your issue was unrelated. It's because your parallax component is measuring the entering page on mount and making all your parallax elements position absolute based on that measurement. When using the swipe and paintDrip animations the entering page is being animated and your parallax component seems to measure the entering page as having a height of 0px for some reason.

Upgrade to the latest version for a workaround for the paintDrip animation. You can now set direction for the entering page in paintDrip. if you set it to an empty string it wont slide the incoming page in. Your parallax component can then measure the page. So add paintDrip direction="". There isn't a workaround for the swipe animation. Since it's a fairly unique situation I would recommend creating a custom TransitionLink if you need that kind of animation. You'll have to set the parallax wrapper height manually in the trigger prop at the right time for the parallax component.

There seems to be a second conflict with your parallax component and TL which is that the .tl-wrapper component needs to be position: relative but that seems to interfere with parallax. I can't remove position relative as it breaks the z-index control of the entering and exiting pages. You can add .tl-wrapper { position: unset; } (or maybe absolute) to your project to fix the issue.

For everyone else, the original issue has been solved and this issue has become a bit of a catch all. I'm not really sure if any of the other issues are still problems as there have been a number of changes to transition link since this issue was opened. Closing this since the original issue is solved (the transition wrapper is position relative).

Please open a new issue if any of you are still having issues!

Thanks all

TylerBarnes commented 5 years ago

By the way @tterb your site looks awesome! I love how the cover animation looks with your design. Really great stuff!!

tterb commented 5 years ago

@TylerBarnes Thanks for helping me troubleshoot the issue! I'll try out the latest release and experiment with your suggestions and see if I can figure out why the parallax component is setting height: 0px on the page.

tqwewe commented 4 years ago

Any updates to this? I'm also having an issue and using react spring.

Edit: 1.5.0-beta.3 seems to fix the issue.

TylerBarnes commented 4 years ago

@Acidic9 can you share your repo? I've been thinking on adding an option to the latest release to change how pages overlap and I think that may help you. It would be good to use your site to debug and see if it fixes your issue.

tqwewe commented 4 years ago

Sure @TylerBarnes I've made you a collaborator as it's currently a private repo for now.

https://github.com/Acidic9/portfolio/invitations

I've reproduced the bug in a seperate branch: bug/gatsby-plugin-transition-link

git clone git@github.com:Acidic9/portfolio.git
cd portfolio
git checkout bug/gatsby-plugin-transition-link
yarn install
yarn run dev

You should just see a blank white page. Master has this fixed by simply using 1.5.0-beta.3.

TylerBarnes commented 4 years ago

That's great! thanks @Acidic9 !