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

New scroll-to modifier #259

Open
ygongdev opened this issue Jan 26, 2021 · 3 comments
Open

New scroll-to modifier #259

ygongdev opened this issue Jan 26, 2021 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@ygongdev
Copy link
Collaborator

ygongdev commented Jan 26, 2021

Description
Adding a new scroll-to functional modifier that mimics the native scrollTo.
It is possible that we only want to perform a scroll-to if a certain condition is satisfied, so this modifier will optionally take a boolean to know when to trigger its scroll-to

API
The arguments are chosen to closely mimc the API of the native scrollTo, which supports 2 positional arguments or a named option. We just add one more API to support the optional boolean.

Positional arguments: [top, left, shouldScroll] or scrollOptions
Named arguments: { top, left, behavior, shouldScroll }

Examples
Debating between explicit positional arguments and a single positional object containing the config, which gives more flexibility if the native scrollOptions API happens to change.

<div {{scroll-to 100 100 true}}></div>

vs

<div {{scroll-to scrollOptions true}}></div>

Named

<div {{scroll-to top=this.top left=this.left behavior=this.behavior shouldScroll=this.shouldScroll}}></div>
@ygongdev ygongdev added the enhancement New feature or request label Jan 26, 2021
@elwayman02
Copy link
Owner

Instead of a boolean, how about a Promise? We can execute the scrollTo behavior when that Promise resolves, which can be done programmatically as easy as flipping a boolean flag.

As for argument style, I prefer named but also think we should stick with the options object, like so:

<div {{scroll-to options=this.scrollOptions shouldScroll=this.scrollPromise}}></div>

I would rather support the full, easiest API rather than have to mimic each and every param, and like you said, it's more future-proof. It's easy to define an inline hash in the template if people want to do it that way, as well.

@ygongdev
Copy link
Collaborator Author

Sounds good, I should adapt similar API for the others as well

@ygongdev ygongdev added the good first issue Good for newcomers label Jan 26, 2021
@ygongdev
Copy link
Collaborator Author

@elwayman02 Oh I just realized the difference between window.scrollTo and element.scrollTo. I think element.scrollTo is meant for within a scrollable element, which is one use case.

If our intention is to scroll to this element, we should be either be using a scrollIntoView modifier or using window.scrollTo and change API to take relative offset values instead of absolute coordinates, so we can take advantage of this.element?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants