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

[Proposal] center elements for scrollTo #398

Closed
RobbieTheWagner opened this issue Jun 12, 2019 · 4 comments · Fixed by #402
Closed

[Proposal] center elements for scrollTo #398

RobbieTheWagner opened this issue Jun 12, 2019 · 4 comments · Fixed by #402

Comments

@RobbieTheWagner
Copy link
Member

Currently, we use https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView with the default options, which scrolls until the element is aligned with the top of the viewport. This is configurable, and there appears to be an option where the element is centered instead. Perhaps centered would be a more sane default, and not so extreme? We could also make this configurable and allow the user to pass top, bottom, center etc. or possible let them pass options directly to the call to scrollIntoView.

@chuckcarpenter thoughts?

cc @hheexx

@hheexx
Copy link
Contributor

hheexx commented Jun 12, 2019

I have checked the APIs.
Passing scrollIntoViewOptions from tour would be very easy for you to implement and it would be much better then now. But not perfect.
I recommend you implement this first.

tour.addStep('step5', {
			title: 'Tutorial',
			text: 'some text...',
			attachTo: '#element',
			scrollTo: {behavior: "smooth", block: "center"},
			buttons: [
				{ text: 'Off', action: tour.cancel },
				{ text: 'Next', action: tour.next }
			]
		});

Perfect solution you can add later is scrolling element to bottom 1/3 of the screen and then showing tooltip in the top 2/3 of the screen.
The most sexy would be to scroll element to the center (you can use code above to do it), then wait like 100ms, then scroll element a little bit more down to bottom part of screen, and then tooltip faces in in the top part of the screen.

What do you think?

@RobbieTheWagner
Copy link
Member Author

@hheexx

Passing scrollIntoViewOptions from tour would be very easy for you to implement and it would be much better then now.

If it is easy to implement, would you like to submit a PR for it? I think having scrollTo accept true to keep the behavior the same as now or allowing passing options like {behavior: "smooth", block: "center"}, would be a good API.

@hheexx
Copy link
Contributor

hheexx commented Jun 12, 2019

Here: #401

I just finished but now I found out that scrollIntoViewOptions is not very vell supported. Safari still does not support it.
https://caniuse.com/#feat=scrollintoview

btw there is polyfill http://iamdustan.com/smoothscroll/

not sure what to do.

@RobbieTheWagner
Copy link
Member Author

We can use the polyfill. I will work on this implementation.

RobbieTheWagner added a commit that referenced this issue Jun 12, 2019
This should enable users to pass options like `scrollTo: { behavior: 'smooth', block: 'center' },` which will change the scrolling behavior to center the object and to scroll smoothly. These options come from https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView

The `behavior` option is not supported in Safari or IE, so we added a polyfill for them.

Closes #398
Closes #401
RobbieTheWagner added a commit that referenced this issue Jun 12, 2019
This should enable users to pass options like `scrollTo: { behavior: 'smooth', block: 'center' },` which will change the scrolling behavior to center the object and to scroll smoothly. These options come from https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView

The `behavior` option is not supported in Safari or IE, so we added a polyfill for them.

Closes #398
Closes #401
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 a pull request may close this issue.

2 participants