w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 661 forks source link

[cssom-view-1] Option for scrollIntoView that doesn't propagate to root #9452

Open flackr opened 1 year ago

flackr commented 1 year ago

The scrollIntoView API invokes the behavior to scroll an element into view which:

means to run these steps for each ancestor element or viewport that establishes a scrolling box scrolling box, in order of innermost to outermost scrolling box

However, in many circumstances it can be counter-intuitive that a scroll into view call scrolls ancestor scrollers. E.g. If a slideshow or carousel uses a scrolling box for slides and uses scrollIntoView to advance to the next slide, this will also force the top level scroller to scroll the slideshow into view. This is often surprising, especially when scrollIntoView is used as a convenience to automatically calculate the scroll position rather than using scrollTo on the scroller. @argyleink has examples where this has been problematic.

As such, I propose we add the ability to scroll an element into view that only scrolls the nearest scrolling box. There are two ways to do this:

  1. Add an option to ScrollIntoViewOptions, e.g. scroll = "nearest" | "ancestors" with a default of ancestors or specify a boundary scroller (happy to bikeshed names), or
  2. Add the necessary options to ScrollToOptions to scroll a particular element into view. E.g. we could provide a target and alignment options (similar to ScrollIntoViewOptions). It would be hard to define reasonable interactions when for example a target is specified and a left value is specified. Further, it would be odd when there is an intermediate scroller that does not have the element visible, do we scroll to where the element is outside of the intermediate scroller's scrollport or do we scroll to the intermediate scroller?

I think option 1 is simpler and a reasonable api.

flackr commented 1 year ago

Developers could also use scrollParent from #1522 if they wanted to for example scroll every scroller from the target to some particular ancestor. E.g.

function scrollElementIntoView(ancestor, target) {
  while (target && target != ancestor) {
    target.scrollIntoView({scroll: "nearest"});
    target = target.scrollParent;
  }
}
argyleink commented 1 year ago
Watch a video reproducing the issue https://github.com/w3c/csswg-drafts/assets/1134620/de79fadc-807a-44bd-aec0-e75671fce0bf

Try this demo here

Since scrollIntoView() does so much jumping and aggressive recursive scroll adjusting, the API was avoided entirely in this carousel demo, where I manually (and included logical directionality) used scrollTo().

As mentioned by Michael here on Twitter, scrollIntoView() as it currently is, is hardly usable.

I prefer option 1 as the solution 👍🏻 would make the API usable for me in a component driven environment

bakura10 commented 1 year ago

Hi,

I am Michael from Twitter. The scrollIntoView API has indeed be problematic for us since the start, and most of the use cases where I wanted to use it, I could not because of the full page scroll.

I have developed a scroll-snap based carousel and due to our constraints (we are creating Shopify themes) we have to support a lot of different scenario: various alignments (start, end, center), support for scroll-margin/padding, taking into account borders, full support for RTL...

All of this is possible by using scrollTo and manually calculating the offset, but it quickly becomes extremely complex (especially RTL, where things like scroll offsets behave very strange). Having a way to simply reuse scrollIntoView and making sure the scroll does not propagate to the whole page would help to remove ton of code, and also make it much more robust.

Lately, I started using this library (https://github.com/scroll-into-view/compute-scroll-into-view) to calculate all the offset (I recently pushed a PR to add RTL support and it was messy) and apply scrollTo, and I really like the boundary setting, I think it could be used in a similar way:

slideCell.scrollIntoView({ inline: 'start', boundary: carousel });

By doing this, the scroll will propagate until the carousel itself and stop there. This is by far the most flexible approach and would fix all my grips with scrollIntoView :D.

thunderfreak commented 7 months ago

You can also do this to reset the window to the position it was before, once the item is scrolled into view.

const y = window.scrollY;
const x = window.scrollX;
if (isFocused && item.current) {
  item.current.scrollIntoView({ block: 'center' });
}
window.scroll(x, y);
nickcoury commented 1 month ago

I just ran into this. I have a navigational carousel at the top of the page that centers the clicked item and loads a new page. On back/forward button press, I center the previously selected item using { inline:"center" }. If the destination page was previously scrolled, the page will unexpectedly jump to the top and conflict with scroll restoration, which is disorienting.

It wouldn't be as flexible as other solutions here, but I found myself just wanting { inline: "center", block: "none" } to prevent scrolling in one direction but allow it in the other.

flackr commented 1 week ago

I propose we go with option 1, and for flexibility add a container option to ScrollIntoViewOptions:

dictionary ScrollIntoViewOptions : [ScrollOptions](https://www.w3.org/TR/cssom-view-1/#dictdef-scrolloptions) {
  ScrollLogicalPosition block = "center";
  ScrollLogicalPosition inline = "center";
  Element? container = null,
};

By default, the null container will scroll all scroll containers all the way to the root, however if container is specified, it will only scroll the containers from the target element up to container inclusive.

Additionally, I think it would be useful to add scrollParent so that you could easily get the scroll container for the target, e.g.

target.scrollIntoView({container: target.scrollParent});