iDad5 Posted February 7, 2023 Share Posted February 7, 2023 The topic has the same context as this one and that one, but is not directly connected to those, as far as I can tell. It occurs on desktop Windows Chrome and Firefox, as well as Mac Safari. I did not verify other platforms, but have no reason to suspect that it is platform related. The CodePen is as minimal as I could make it without making is useless for me. It is in Typescript, which makes it a lot easier to read for me, unfortunately I'm aware that most others don't see it that way—even if I changed the coding style it would have still been rather complex. In fact, I have a working version with the exact same code, except for three lines. So, probably working through all the code will not be necessary anyhow. To reproduce the issue, scroll to any other panel than the first, and then resize the window. Expected behavior: after a rebuild of the page, the same panel as before the resize should be shown. Actual behavior: the first panel is always displayed after a resize. What is going on: After a resize, ScrollTrigger is killed and newly built. Then an attempt is made to send the new ScrollTrigger to the equivalent position the old one was in before the resize. The function used is ScrollTrigger.scroll(toValue); Line 55 in the code. While this works in one version, it does not in the other version. (I'll post the working version in the follow-up post) The difference between those versions is the method of creating (and recreating) the ScrollTrigger, it is to be found in lines 65, 68, and 69. Simply going with the working version is not an option, as this version has issues on mobile Safari, which this version has not. Moreover, I'm convinced that ScrollTrigger.scroll() should work in this version of creating the ScrollTrigger, which is working fine in any other respect as far as I can tell. Therefore, I suspect that either I do something 'illegal' in my code, which I'm not yet aware of, or there is a bug in ScrollTrigger. See the Pen VwBNONV by iDad5 (@iDad5) on CodePen 1 Link to comment Share on other sites More sharing options...
iDad5 Posted February 7, 2023 Author Share Posted February 7, 2023 Here is the working version: See the Pen JjBVqmL by iDad5 (@iDad5) on CodePen 1 Link to comment Share on other sites More sharing options...
iDad5 Posted February 7, 2023 Author Share Posted February 7, 2023 Further details: Both versions use the beta of 3.11.5. I tested the problematic one with 3.11.4 to make sure it is not beta related. The behavior was the same. The lines 133 to 158 are necessary as the label reported from line 84 often is not correct. I don't think this is connected, but might be worth a look from someone above my mental pay grade. I needed to delay the resize trigger, as browsers fire the resize event much too frequently for the complex operations here. (I do the same but much more sophisticated in the production version.) 1 Link to comment Share on other sites More sharing options...
Solution GreenSock Posted February 7, 2023 Solution Share Posted February 7, 2023 Thanks for the improved clarity on exactly what the issue is, how to reproduce, etc. ? I believe this boils down to several things: As I explained in a previous thread, when you associate a ScrollTrigger with a timeline (rather than a tween), it must WAIT 1 tick to do its refresh (when it calculates start/end values and applies pinSpacing, etc.) because timelines are almost always created and THEN populated and it's super common to assign the ScrollTrigger in the constructor like gsap.timeline({ scrollTrigger: {...}}). But in your scenario, you're creating the ScrollTrigger and immediately trying to set the scroll position which actually depends on a bunch of extra pinSpacing being implemented at that point which leads me to point 2: When you're trying to set the scroll position, it doesn't exist. For example, let's say you're trying to set the scroll position to 500...but the page literally has NO scrolling space whatsoever. So you can't scroll to 500 when the maximum you can scroll to at that point is 0. I think you've got logic problems in your spacing. Your code seems to be written assuming that each panel's scroll position corresponds to 100vh further down, but you're setting up your ScrollTrigger with a trigger of "container" and you don't set an end value at all, thus it defaults to "bottom top". Since the container is 100vh itself, that means the ENTIRE ScrollTrigger (across ALL panels) will only last 100vh. You're also doing a findCurrentSlide() inside an onToggle() but when you resize that may not be entirely accurate based on the new measurements. For example, if you scroll all the way down, progress is 1 on the timeline but if you resize that scroll position may actually correspond to a progress of 0.998745, thus currentLabel() will be one less. You might want to just get the closest label instead. In my fork I've included a function for that. It's not a bug in ScrollTrigger.scroll() - you could manually do it with window.scroll() and you'd see the same result. it's just logic and timing stuff. And for point #1, you can force a .refresh() call on your individual ScrollTrigger immediately after creating it so you don't have to wait 1 tick (since you know you're done populating it at that point). See the Pen JjBqjEe?editors=0010 by GreenSock (@GreenSock) on CodePen Does that clear things up? 4 Link to comment Share on other sites More sharing options...
iDad5 Posted February 7, 2023 Author Share Posted February 7, 2023 Thank you very much for your time and the great lesson. 24 minutes ago, GreenSock said: As I explained in a previous thread, when you associate a ScrollTrigger with a timeline (rather than a tween), it must WAIT 1 tick to do its refresh (when it calculates start/end values and applies pinSpacing, etc.) because timelines are almost always created and THEN populated and it's super common to assign the ScrollTrigger in the constructor like gsap.timeline({ scrollTrigger: {...}}). But in your scenario, you're creating the ScrollTrigger and immediately trying to set the scroll position which actually depends on a bunch of extra pinSpacing being implemented at that point which leads me to point 2: I don't doubt your word for a second, but as I do not destroy the timeline, I thought I was covered on that point. I will have to think that one through. But I accept your word for it as the gospel. 27 minutes ago, GreenSock said: When you're trying to set the scroll position, it doesn't exist. For example, let's say you're trying to set the scroll position to 500...but the page literally has NO scrolling space whatsoever. So you can't scroll to 500 when the maximum you can scroll to at that point is 0. That is so true and now that you tell me it is blindingly obvious. The only thing I can say in my defense is, that I was somewhat mislead by ScrollTrigger reporting to me, that it had set the scroll to the desired value—but that shouldn't have let me overlook my logical blunder. But wait, I'm still slow, I guess: In the case that worked, I set the height of the container to a multiple of the panel height right before the creation of the ScrollTrigger, that makes it possible to scroll the page. (True?) In the not working version originally, I had not lost the end value line, but set the end value to the same number I set the container height in the working version. (I corrected that again). As far as I understand and can see, when testing, that end value gives the pin-spacer the extra padding, to make the page scrollable. And the ScrollTrigger.scroll() is executed after the creation of ScrollTrigger. Even with that end value, it is not working and has never been.Is it possible, that the actual creation of the pin-spacer is only happening one refresh after the execution of the ScrollTrigger creation? 31 minutes ago, GreenSock said: I think you've got logic problems in your spacing. Your code seems to be written assuming that each panel's scroll position corresponds to 100vh further down, but you're setting up your ScrollTrigger with a trigger of "container" and you don't set an end value at all, thus it defaults to "bottom top". Since the container is 100vh itself, that means the ENTIRE ScrollTrigger (across ALL panels) will only last 100vh. You're also doing a findCurrentSlide() inside an onToggle() but when you resize that may not be entirely accurate based on the new measurements. For example, if you scroll all the way down, progress is 1 on the timeline but if you resize that scroll position may actually correspond to a progress of 0.998745, thus currentLabel() will be one less. You might want to just get the closest label instead. In my fork I've included a function for that. Thank you for that. That in the CodePen I somehow lost the line that had been in all the time while I worked on it* makes me angry with myself. I had set it to 'scrollDistance' in all my local test cases and in the first version of the CodePen. (See above.) The misreporting on the label also happens without resize and on desktop, but all evidence points to a (timing) error on my side in this case too, I will look for that. 44 minutes ago, GreenSock said: And for point #1, you can force a .refresh() call on your individual ScrollTrigger immediately after creating it so you don't have to wait 1 tick (since you know you're done populating it at that point). Good to know, thanks. 45 minutes ago, GreenSock said: Does that clear things up? Yes, it does clear up a lot. Foremost, that you are a genius and that I want and need to understand thing better than I do. Thanks again!!!! Link to comment Share on other sites More sharing options...
GreenSock Posted February 7, 2023 Share Posted February 7, 2023 1 minute ago, iDad5 said: Is it possible, that the actual creation of the pin-spacer is only happening one refresh after the execution of the ScrollTrigger creation? That's what I've tried to explain several times, yes. ScrollTrigger does all of its measuring, pin spacer adjustments, etc. inside refresh() which is delayed by one tick when you've got a ScrollTrigger attached to a timeline (for the reasons I stated earlier). But you can force it if you know your timeline is done being populated (as I did in my fork). 2 minutes ago, iDad5 said: The only thing I can say in my defense is, that I was somewhat mislead by ScrollTrigger reporting to me, that it had set the scroll to the desired value—but that shouldn't have let me overlook my logical blunder. That's a caching performance optimization. We've put a ton of effort into making ScrollTrigger as fast as possible. Reading the scroll position can actually trigger paint/reflow in some browsers and since it's something we read a LOT, we cache it whenever possible. So technically I could add code to check it against the maximum value and cap it, but that goes directly against the performance optimization strategy. It's an edge case and I think it's reasonable to expect that people shouldn't be setting the scroll to an invalid value. It's a tradeoff, honestly. Do we want to force everyone to pay a performance penalty in order to prevent an edge case where someone sets an invalid value and tries reading it back via ScrollTrigger.scroll() and get the non-capped value? In my opinion, it's an obvious choice (performance). 7 minutes ago, iDad5 said: Thanks again!!!! No problem. Glad things are less fuzzy now. ? 1 Link to comment Share on other sites More sharing options...
iDad5 Posted February 7, 2023 Author Share Posted February 7, 2023 Thank you again, now I do not feel like a complete idiot anymore, only a normal one. 7 minutes ago, GreenSock said: ScrollTrigger does all of its measuring, pin spacer adjustments, etc. inside refresh() I read that, but had not really internalized its consequence for 'my logic'. Now I learned. Against my better knowledge on a conscious level, it seems that under pressure I'm still deeply rooted in the assumption that all code execution in JavaScript is linear. Live and learn. Thanks! 1 Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now