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

Remove JavaScript scrollLock in favor of CSS scroll-snap property #249

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kevinfarrugia
Copy link
Contributor

The scroll-snap-type can be used to replace the scrollLock property and JavaScript required to center the element.

Breaking Change!

  • Removes scrollLockDelay property
  • Removes scrolLock behavior from JavaScript. On unsupported browsers, this will cause scrollLock to stop working.

Support: https://caniuse.com/mdn-css_properties_scroll-snap-type

@kevinfarrugia
Copy link
Contributor Author

when using the arrows or dots on a glider using scroll-snap, the animation is removed as it snaps immediately to the next/previous element

@kevinfarrugia kevinfarrugia marked this pull request as draft March 22, 2023 08:44
@dngraphisme
Copy link

Hello @kevinfarrugia, I confirm that with the css snap syntax the animation is skipped... Is it possible to correct this ? thanks again, Nicolas :)

@casimirloeber
Copy link

This is awesome! Have you used it in production @kevinfarrugia?

@kevinfarrugia
Copy link
Contributor Author

@casimirloeber Yes, I use it (CSS scroll-snap ) on production on several projects. Usually when I am not showing the arrows since clicking on the arrows skips the slide animation. Note that you should set scrollLock to false too.

@casimirloeber
Copy link

Wonderful. Thank you very much for the PR and reply. Do you think it's possible to work in any sort of animation when using arrows such as a fadein/out?

@kevinfarrugia
Copy link
Contributor Author

TBH I haven't tried as I did not need it. I was looking at experimenting with the new CSS scroll-driven animations as I think it should be possible to remove all JavaScript on browsers that support it. Need to find the time to pick it up.

@casimirloeber
Copy link

Hey @kevinfarrugia, I just noticed that you said:

Note that you should set scrollLock to false too.

Did you mean to say resizeLock?

That would be so cool to transition to pure (or closer to) CSS!! The scroll-snap version of the scrollLock functionality feels much more robust and smooth with less random glitches and such. I should have it in production soon on a fairly high traffic site. Thank you so much for your work.

And obviously many many thanks to you @NickPiscitelli for your hard work making all of this possible. It's such a refreshing take on a carousel and just makes so much sense!

@kevinfarrugia
Copy link
Contributor Author

There is also a property scrollLock that is default to false.

// Force centering slide after scroll event scrollLock: false,

IIRC it is not compatible with CSS scroll snap and you may get some glitching.

@casimirloeber
Copy link

I believe your PR removes the JS scrollLock functionality and replaces it with the scroll snap CSS implementation while reusing the scrollLock property name.

https://github.com/NickPiscitelli/Glider.js/pull/249/files#diff-f4b8488a4d72712239240832f06f48a1aaf0e2ec4775c8f877a14e4e11f3d39eR80

https://github.com/NickPiscitelli/Glider.js/pull/249/files#diff-f4b8488a4d72712239240832f06f48a1aaf0e2ec4775c8f877a14e4e11f3d39eL360

Not trying to disagree with you, just making sure I get it. It is working beautifully and is much smoother than the JS solution. Thanks again for the work!

And if you need a test subject for the scroll based animations stuff... let me know. My CSS skills are not strong enough to do it myself but I am a good tester and cheerleader!

@kevinfarrugia
Copy link
Contributor Author

Yes, that's correct. So you can either replicte the changes in this PR in a forked or local copy, or continue using the released version and add some CSS to enable scoll-snap (in which case you need to avoid using scrollLock).

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