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

Percentage alignment options #49

Closed
nikrowell opened this issue Feb 27, 2020 · 7 comments
Closed

Percentage alignment options #49

nikrowell opened this issue Feb 27, 2020 · 7 comments
Labels
feature request New feature or request resolved This issue is resolved

Comments

@nikrowell
Copy link
Contributor

nikrowell commented Feb 27, 2020

Hi @davidcetinkaya - thanks for your work on this - love your vision for a lightweight and flexible carousel!

I'm working on a project where it would be useful to specify the align option as a percentage from the left edge (let me know if helpful to have a visual / example of my use case). Is this something you could see being added to Embla?

@nikrowell
Copy link
Contributor Author

Curious to learn more about how Embla works under the hood, I took a quick pass at this on my feature/alignment-percent branch but did not create a PR. No worries if this isn't something you'd like to include in Embla (or my implementation has issues I'm not seeing 😆)

@davidjerleke davidjerleke added the feature request New feature or request label Feb 28, 2020
@davidjerleke
Copy link
Owner

davidjerleke commented Feb 28, 2020

Hello Nik (@nikrowell),

I'm happy you like the Embla Carousel initiative 🙂. And thank you for your feature request. The answer to your question is yes, I've considered adding percentage to the align option.

I appreciate your contribution, it's definitively a good starting point! It requires additional refactoring in order for it to work properly though. The main part that needs attention is the logic that determines which scroll snap is selected depending on the scroll position, which is located in the scrollTarget component.

This is how Embla works today 👇
align center

Depending on the chosen alignment, Embla checks which scroll snap has the alignment line (the red one on the screenshot) within its left and right bound (the green lines on the screenshot). On the screenshot, you can see that scroll snap number 3 is the selected scroll snap.

I'm afraid that the current implementation only works for start, center and end alignments and won't work properly for percentage alignment. I've been wanting to refactor the logic that determines which scroll snap is selected depending on the scroll position, but simply haven't had the time to do so yet.

The vision is the following 👇

align - vision

Where we only need to compare the center of each scroll snap in order to determine which one is closest to the alignment line. This will work as intended whether the chosen alignment is start, center, end or percentage.

At the time of writing I have a huge backlog including everything from a lot of feature requests to writing documentation (like finishing the event section, see issue #45) and setting up CodeSandbox demonstrations etc. Unfortunately, I rarely get any contributions on any of these tasks, so things are progressing slow. With that said, I'm hoping to be able to investigate this soon, but can't give you an estimate on when this will happen. I hope you understand!

I took the liberty to change the issue title to a more specific one, I hope you don't mind.

Best,
David

@davidjerleke davidjerleke changed the title Additional alignment options Percentage alignment options Feb 28, 2020
@nikrowell
Copy link
Contributor Author

Hi David,

Thank you for the feedback and detailed explanation - very helpful! I hadn't thought about the impact of this on logic for selected states, dragFree mode etc, and your proposed change using scroll snap center vs bounds makes sense.

I completely understand on the backlog and would be happy to contribute to those tasks as well. I'll take a look and see where I can help.

Cheers!

@davidjerleke
Copy link
Owner

davidjerleke commented Mar 24, 2020

Hello Nik (@nikrowell),

An update on this feature request 👇

I just wanted to let you know that I'm working on refactoring the scrollTarget logic as mentioned in this conversation, in order to implement the percentage alignment feature. It's going quite well and most of the ground work is done. Although I can't give you an estimate yet, I'll get back to you as soon as I have something.

Kindly
David

@davidjerleke davidjerleke added resolved This issue is resolved awaiting response Issue is awaiting feedback labels Mar 27, 2020
@davidjerleke
Copy link
Owner

davidjerleke commented Mar 27, 2020

Hello again Nik (@nikrowell),

I'm happy to announce that percent alignment is now a feature of Embla v2.9.0 🎉. You can read more about it in the release description or under the Options section in the README.

I actually managed to reduce the bundle size when I refactored scrollTarget so this feature comes with a reduced bundle size 😊. Thanks a lot for your code changes (in your feature/alignment-percent branch) in the alignment component, you really did most of the work there 💪.

I'd very much appreciate if you could confirm if this feature is working as expected.

Cheers!
David

@nikrowell
Copy link
Contributor Author

Thank you, David (@davidcetinkaya)! 🎉 I tested things out and it's working as expected on my end - love that it also reduced the bundle size!

@davidjerleke davidjerleke removed the awaiting response Issue is awaiting feedback label Mar 27, 2020
@davidjerleke
Copy link
Owner

I'm happy to hear that Nik (@nikrowell), enjoy 😊!

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