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

fix: scrollIntoView ending earlier than the duration given as an option #35

Merged
merged 2 commits into from
Jun 2, 2023

Conversation

malangfox
Copy link
Contributor

@malangfox malangfox commented May 25, 2023

Details

https://codepen.io/malangfox/pen/MWPLWvV
In the above demo, when the scrollIntoView is triggered by pressing the button, the scroll moves for 5000ms, but when item 11 is placed at the far right end and the button is pressed, the time it takes to move to the end of the scroll area is not 5000ms.

Fixing this as a bug could lead to a new issue of scrollIntoView being slow at the end of the scroll area after updating the version of Conveyer, so I would like to support the existing behavior but add an option to check the minimum/maximum scrollable distance when calculating the distance for scrollBy in scrollIntoView so that it always has the same scroll animation time.

I'm currently using the fixedDuration name as a temporary name, but I'm trying to think of another appropriate name.

@malangfox
Copy link
Contributor Author

I looked at the behavior of the methods on the @egjs/flicking, and Flicking do not end the animation earlier than the duration passed as an option, so I want to unify the behavior here.

If there are any issues with the original behavior in the future, perhaps we'll add an option to support the original behavior.

Copy link
Member

@daybrush daybrush left a comment

Choose a reason for hiding this comment

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

LGTM

@malangfox malangfox changed the title feat: add fixedDuration(temporary name) option fix: scrollIntoView ending earlier than the duration given as an option Jun 2, 2023
@malangfox malangfox merged commit d9d11c0 into naver:main Jun 2, 2023
malangfox added a commit to malangfox/egjs-conveyer that referenced this pull request Jun 2, 2023
…on (naver#35)

* feat: add fixedDuration option

* fix: make fixedDuration option to regular feature
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