-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Update scrollIntoView + scrollIntoViewOptions #4866
Conversation
Update scrollIntoView + scrollIntoViewOptions compatibility for Microsoft Edge and Opera for PC. Opera fully supports scrollIntoViewOptions (in newest version 63), but Edge doesn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @XTard and welcome to BCD! Thank you for opening this PR. This is a good start, though I have a couple of questions in line.
Also, do you have any more information on where your data came from? Did you read this in release notes, an issue tracker, or did you do some testing? It'd be great to have some more info backing this up so we can be ready to merge. Thank you!
@@ -6511,7 +6511,7 @@ | |||
"edge": [ | |||
{ | |||
"version_added": "18", | |||
"notes": "No support for <code>smooth</code> behavior." | |||
"notes": "Parameter <code>alignToTop</code> is supported. No support for <code>smooth</code> behavior." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand the data correctly, the note about the smooth option is redundant with the scrollIntoViewOptions
feature data you updated below. In that case, I'd suggest rephrasing this
"notes": "Parameter <code>alignToTop</code> is supported. No support for <code>smooth</code> behavior." | |
"notes": "The only parameter supported is <code>alignToTop</code>." |
"notes": "The <code>block</code> and <code>inline</code> options support the values <code>start</code>, <code>center</code>, <code>end</code>, <code>nearest</code>." | ||
}, | ||
{ | ||
"version_added": true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data as proposed here suggests that
- This feature was supported in some version before 48
- From version 48 onward, the note applies
That doesn't really match up with your PR description. Are you're trying to say that, at some point after version 48, support for the behavior
option was added too?
Hey @XTard, do you plan to return to this PR? |
Hey again -- since we haven't heard back in a few weeks about this, I've created another pull request to move this along. Thanks, @XTard! |
Update scrollIntoView + scrollIntoViewOptions compatibility for Microsoft Edge and Opera for PC.
Opera fully supports scrollIntoViewOptions (in newest version 63). Edge only supports alignToTop parameter.