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

Scroll By #23

Closed
davidjerleke opened this issue Sep 26, 2019 · 8 comments
Closed

Scroll By #23

davidjerleke opened this issue Sep 26, 2019 · 8 comments
Labels
feature request New feature or request resolved This issue is resolved

Comments

@davidjerleke
Copy link
Owner

davidjerleke commented Sep 26, 2019

👉 Specification (implemented)

// adds to current scroll progress
// accepts a number from 0 - 1 representing % to add to the current scroll progress
// scroll to target is smooth

embla.scrollBy(0.5)

As discussed here and here in issue #21.

Special thanks

@xiel for this feature request.

Please react with --> 👍 if you want this to be implemented.

@davidjerleke davidjerleke added the feature request New feature or request label Sep 26, 2019
@davidjerleke davidjerleke changed the title ScrollBy Scroll by Sep 26, 2019
@davidjerleke davidjerleke changed the title Scroll by Scroll By Sep 30, 2019
@davidjerleke davidjerleke added the discussion This is a conversation or discussion about something label Oct 1, 2019
@davidjerleke davidjerleke removed the discussion This is a conversation or discussion about something label Dec 4, 2019
@davidjerleke davidjerleke added the investigating Issue is being looked into label Dec 30, 2019
@davidjerleke
Copy link
Owner Author

Hello Felix (@xiel),

I'm looking into implementing this and I'd like to discuss the feature request further with you. By passing a fraction representing % to the embla.scrollBy(0.5) function, do you mean that the percentage to scroll should be calculated from the total scrollable distance or based on the current position?

Thank you in advance.

Kindly,
David

@xiel
Copy link
Contributor

xiel commented Jan 22, 2020

Hi David (@davidjerleke),

cool, looking forward to this feature!

I would expect scrollBy to work additive with scrollProgress. So when scrollProgress() is returning 0.5 and I would scrollBy(0.25), scrollProgress() would now return 0.75.

Cheers, Felix

@davidjerleke
Copy link
Owner Author

Hi again Felix (@xiel),

Thanks for getting back to me and I appreciate your clear explanation. We're on the same page then, so I'll let you know when I have something.

Best,
David

@davidjerleke davidjerleke added upcoming A feature or bug fix is on its way for this issue and removed investigating Issue is being looked into labels Jan 23, 2020
@davidjerleke davidjerleke added resolved This issue is resolved and removed upcoming A feature or bug fix is on its way for this issue labels Jan 31, 2020
@davidjerleke
Copy link
Owner Author

davidjerleke commented Jan 31, 2020

Hello Felix (@xiel),

I'm happy to announce that I've released the scrollBy() feature with version 2.7.0 🚀. I've not had time to add it to the README though, but I'll add it as soon as possible.

How it works:

  • ScrollBy adds to scrollProgress as we discussed.
  • ScrollBy respects the scroll bounds when loop: false.

Let me know if it works as intended.

Cheers,
David

@xiel
Copy link
Contributor

xiel commented Jan 31, 2020

Thanks! Will check out soon

@davidjerleke davidjerleke added the awaiting response Issue is awaiting feedback label Jan 31, 2020
@davidjerleke
Copy link
Owner Author

davidjerleke commented Feb 1, 2020

@xiel I’ve added the CodeSandbox to the Readme now. Make sure that you download the latest patch version before trying it out. Enjoy!

@xiel
Copy link
Contributor

xiel commented Feb 9, 2020

Really cool! Was super easy to build a gallery with preview with this, so the previews scroll at the same time as the big images. I think I even like the softer movement as you still apply the spring when setting with scrollBy right?

https://codesandbox.io/s/embla-carousel-scrollby-we9j1

Maybe in the future the scrollBy method could take a second options object, in for

unit: 'percentage' | 'px' // percentage default
and hard: false | true //  false default

Because when I would like to control the gallery connected to a user gesture, it would be nicer to have the set hard I think.

@davidjerleke
Copy link
Owner Author

Hi Felix (@xiel),

I'm glad you like it. That's correct, scrollBy() will scroll smoothly to target as opposed to an instant setter. Thank you for your demonstration and ideas! I think you've mentioned these feature requests some time ago and I think they could be interesting additions to the API.

The unit part you mention should be a piece of cake because I already map px to percentage in the code. Regarding the hard part, I'm going to do some guessing but I think you're describing the feature request in issue #26, where I've asked you a question about the implementation. Please have a look at it when you have a moment to spare 🙂.

I'm going to close this issue and treat it as resolved. Feel free to open an issue about the unit part.

Best,
David

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request resolved This issue is resolved
Projects
None yet
Development

No branches or pull requests

2 participants