Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve: programmatic scrolling experience #107

Merged
merged 1 commit into from
Jun 2, 2024

Conversation

tarsisexistence
Copy link
Contributor

@tarsisexistence tarsisexistence commented May 26, 2024

This PR can be treated as a breaking change and touches on several things. Let me describe it by points.

First of all, this PR addresses the issue of subsequent setScrollPercent.
When we added setScrollPercent, it was naively working just by running a browser scroll, so the rest of the code doesn't need a big refactoring because we relied on the scroll event.

There are a couple of issues with this:

  • a collision of setScrollPercent, so scroll can't distinguish which scroll we process now. And there is no way to stop the previous scroll from progressing.
  • not supported easing. We play a video with the trackScroll setup (like linear scrolling). We can't add easing for trackScroll on event because some easings progress towards the goal non-linearly, which means you scroll to the bottom, but easing plays in the opposite direction. In addition to the connected video duration to the scroll percentage.

Also, nice to have things are:

  • internal state of videoPercentage, so we could operate this value in some scenarios,
  • support onChange

Feel free to share your thoughts.
And maybe we finally need to build a repro for advanced cases for easier testing and regression.
One more thing: I added a few lines of code to support onChange, but we can do it separately, of course. I just showed how easy it is now.

@tarsisexistence
Copy link
Contributor Author

tarsisexistence commented May 26, 2024

Implementation breakdown.

Now we have the only way to update the video percentage from outside setVideoPercentage, which is trackScroll agnostic. If it's enabled - it will scroll, and vice versa.

So, at this point, we deprecate setScrollPercent and make the previously exposed setTargetTimePercent as private for internal usage only.

When a user invokes setVideoPercentage:

  • stop previous video playback.
  • save video percentage state and run onChange
  • if trackScroll enabled - update the scroll position with window.scrollTo
  • play video to a certain position

I also added lockScroll, which prevents users from changing the result scroll position. So in case lockScroll is enabled, user still can scroll during scrollTo phase, but scroll position will come to the desired place magnetically. I added it as API because I feel some users find this undesired for their apps, so I left the possibility to turn it off.

Copy link
Owner

@dkaoster dkaoster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly this looks great and works very well as far as I've been able to test. If it's good with you I'll merge this and get it out in the next day or two!

@tarsisexistence
Copy link
Contributor Author

@dkaoster I appreciate that a lot and happy it would help this project grow and be useful :)

i did a couple of tests in my apps, so it works much better now, so i'm good to go!

@dkaoster dkaoster merged commit 9bc3478 into dkaoster:main Jun 2, 2024
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.

2 participants