Implement scrollIntoViewIfNeeded

NEW
Unassigned

Status

()

Core
DOM: Core & HTML
--
enhancement
10 years ago
8 months ago

People

(Reporter: Erik Arvidsson, Unassigned)

Tracking

({dev-doc-needed})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [parity-chrome][parity-safari])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
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.

Updated

5 years ago
Blocks: 703346

Comment 1

5 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 2

5 years ago
Disregard comment 1. It's already addressed in the Description.

Updated

5 years ago
Blocks: 724071

Comment 3

5 years ago
Created attachment 594312 [details] [diff] [review]
Proposed fix.
Attachment #594312 - Flags: feedback?(paul)
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 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+
Ignore comment #4, comment #5 is the one.

Comment 7

5 years ago
Created attachment 594427 [details] [diff] [review]
Fix more inline with Webkit's implementation.

New fix taking into account Cedric's comments.
Attachment #594312 - Attachment is obsolete: true
Attachment #594427 - Flags: feedback?(paul)
Attachment #594312 - Flags: feedback?(paul)
Blocks: 724515
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

5 years ago
This scrolls the page to make it visible on the center of the page, but it doesn't address the "IfNeeded" part.
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

5 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

5 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

5 years ago
Attachment #594427 - Flags: feedback?(paul)

Comment 13

5 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
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!
(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*.
Kang-Hao: thanks for the suggestion! I ended up subscribing to the list. I've started a thread, too!

Updated

5 years ago
No longer blocks: 703346

Updated

11 months ago
Whiteboard: [parity-chrome][parity-safari]
Keywords: dev-doc-needed

Comment 17

8 months ago
Chrome usage metrics for this feature: https://www.chromestatus.com/metrics/feature/timeline/popularity/389
You need to log in before you can comment on or make changes to this bug.