Open
Bug 403510
Opened 17 years ago
Updated 1 year ago
Implement scrollIntoViewIfNeeded
Categories
(Core :: DOM: CSS Object Model, enhancement)
Core
DOM: CSS Object Model
Tracking
()
NEW
People
(Reporter: erik, Unassigned)
References
Details
(Keywords: dev-doc-needed, parity-chrome, parity-safari)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
Newer WebKit builds has a very useful method on HTMLElement called:
scrollIntoViewIfNeeded(opt_center : boolean)
It will scroll an element into view if it is outside the viewport. The param, if true or left out will center the element and if false it will scroll as little as possible to make the element visible in the viewport.
This is very similar to scrollIntoView(opt_top) except that it will not scroll any elements if the element is already fully visible in the viewport.
Comment 1•13 years ago
|
||
Is there a specification for this method?
If not, can we consider improving it?
> scrollIntoViewIfNeeded(alignWithTop, center);
if center is true, and if the node is shorter than the page, the node is centered in the middle of the page. And that would keep it compatible with Webkit.
Comment 3•13 years ago
|
||
Attachment #594312 -
Flags: feedback?(paul)
Comment 4•13 years ago
|
||
Comment on attachment 594312 [details] [diff] [review]
Proposed fix.
Great start :)
>+ void scrollIntoViewIfNeeded([optional] in boolean top, [optional] in boolean centerIfNeeded);
The signature should be:
[optional_argc] void scrollIntoView([optional] in boolean top);
Look at description.
Attachment #594312 -
Flags: feedback+
Comment 5•13 years ago
|
||
Comment on attachment 594312 [details] [diff] [review]
Proposed fix.
Great start :)
>+ void scrollIntoViewIfNeeded([optional] in boolean top, [optional] in boolean centerIfNeeded);
The signature should be:
[optional_argc] void scrollIntoView([optional] in boolean center);
Look at description.
Attachment #594312 -
Flags: feedback+
Comment 6•13 years ago
|
||
Ignore comment #4, comment #5 is the one.
Comment 7•13 years ago
|
||
New fix taking into account Cedric's comments.
Attachment #594312 -
Attachment is obsolete: true
Attachment #594427 -
Flags: feedback?(paul)
Attachment #594312 -
Flags: feedback?(paul)
Comment 8•13 years ago
|
||
Comment on attachment 594427 [details] [diff] [review]
Fix more inline with Webkit's implementation.
Review of attachment 594427 [details] [diff] [review]:
-----------------------------------------------------------------
::: content/html/content/src/nsGenericHTMLElement.cpp
@@ +1009,5 @@
> + nsCOMPtr<nsIPresShell> presShell = document->GetShell();
> + if (!presShell) {
> + return NS_OK;
> + }
> +
We need to return early if the element is already fully visible (that's the main rationale for the function: to reduce unnecessarily scrolling).
Comment 9•13 years ago
|
||
This scrolls the page to make it visible on the center of the page, but it doesn't address the "IfNeeded" part.
Comment 10•13 years ago
|
||
We should not take a patch for this bug before there is a specification. Please send email to www-style to propose it.
Comment 11•13 years ago
|
||
Agreed - please don't implement our random features that haven't had any external discussion yet.
(This feature sucks as implemented, for example - it continues the horrible practice of boolean flags. We could handle this much better by extending scrollIntoView with an options object.)
Comment 12•13 years ago
|
||
Having a way to scroll the page to center an element is needed for a couple of our Devtools.
This method happens to do what we are looking for (in webkit).
This doesn't have to be exposed to the content.
I opened a more generic bug where we can discuss the best approach: bug 724585 - We need a way to scroll a page to center an element if the element is not visible
Updated•13 years ago
|
Attachment #594427 -
Flags: feedback?(paul)
Comment 13•13 years ago
|
||
If it can help for some people I tried a little JS implementation of how WebKit does it : https://gist.github.com/2581101
Comment 14•12 years ago
|
||
I have suggested an implementation as a CSSOM-view bug report: <https://www.w3.org/Bugs/Public/show_bug.cgi?id=17152>.
I'd love to have feedback!
Comment 15•12 years ago
|
||
(In reply to Thaddee Tyl [:espadrine] from comment #14)
> I'd love to have feedback!
As a meta feedback, I would suggest you post that to www-style, where discussions happen. You don't need to subscribe the list in order to post to it, but some people might forget to put you in the Cc list. So, *shrug*.
Comment 16•12 years ago
|
||
Kang-Hao: thanks for the suggestion! I ended up subscribing to the list. I've started a thread, too!
Updated•8 years ago
|
Whiteboard: [parity-chrome][parity-safari]
Updated•8 years ago
|
Keywords: dev-doc-needed
Comment 17•8 years ago
|
||
Chrome usage metrics for this feature: https://www.chromestatus.com/metrics/feature/timeline/popularity/389
Comment 18•7 years ago
|
||
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Keywords: parity-chrome,
parity-safari
Whiteboard: [parity-chrome][parity-safari]
Updated•6 years ago
|
Priority: -- → P5
Updated•4 years ago
|
Severity: normal → --
Component: DOM: Core & HTML → DOM: CSS Object Model
Priority: P5 → --
See Also: → https://github.com/w3c/csswg-drafts/pull/5677
You need to log in
before you can comment on or make changes to this bug.
Description
•