w3c / csswg-drafts

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

[css-contain][css-sizing] ScrollIntoView a descendant of element with content-visibility:auto #9337

Open cathiechen opened 1 year ago

cathiechen commented 1 year ago

In [1], point 5 defines the behavior of scrollIntoView targeting on an element with content-visibility:auto.

For the purposes of scrolling operations, such as scrollIntoView(), an element with content-visibility: auto that is skipping its contents has its size and location determined with size containment still active.

However, it does not mention the behavior of scrollIntoView targeting on a descendant with content-visibility:auto. Tested in Chromium and WebKit, the content-visibility:auto is treated as visible before scrolling, it makes sense to me. The related test cases are content-visibility-075.html and content-visibility-076.html. Should we make it clear in the specification? Maybe something like this?

  1. Before scrolling, if the element with c-v: auto has descendant as the target of scrolling, change the relevancy to visible, so that the element could be laid out as if it were relevancy to user. Here we could add a test with nested c-v auto, see test
    content-visibility-vs-scrollIntoView-001.html in the FireFox patch.
  2. Performance scrolling.
  3. After scrolling, we need to check the relevancy again in the intersection observer, if the element with c-v auto is not visible, we should hide its content again. This could happen if there is overflow: clip. See test case
    content-visibility-vs-scrollIntoView-002.html in the FireFox patch.

@emilio @vmpstr @rwlbuis WDYT?

[1] https://drafts.csswg.org/css-contain-2/#cv-notes

rwlbuis commented 1 year ago

@cathiechen note, the referred test names need updating above and in the Firefox patch, I introduced content-visibility-091.html recently, which clashes with the patch.

cathiechen commented 1 year ago

@cathiechen note, the referred test names need updating above and in the Firefox patch, I introduced content-visibility-091.html recently, which clashes with the patch.

Done, thanks! In the latest Firefox patch, it is *-091.html now, and I will keep updating this once the WPT test is updated.

chrishtr commented 10 months ago

In terms of spec text, I think we can just change the relevant to the user definition to add a bullet point for "is being scrolled into view via scrollIntoView".

css-meeting-bot commented 10 months ago

The CSS Working Group just discussed [css-contain][css-sizing] ScrollIntoView a descendant of element with content-visibility:auto, and agreed to the following:

The full IRC log of that discussion <Frances> Emilio: some interoperability, need to generalize to arbitrary ancestors. ScrollIntoView should update layout to scroll into view to be visible.
<Frances> Alan: Change definition relevant to the user to add a condition via scrollintoview to be relevant.
<flackr> q+
<vmpstr> q+
<astearns> ack flackr
<Frances> Emilio: Make all ancestors relevant as well.
<Frances> flack:Might not fully fix problem, scrolling away from is also visibility:auto, other content might change
<Frances> Emilio: ideally it shouldn't
<Frances> flack: scrolling may pass over other content
<Frances> Emilio: If scrolling to the hundredth item and scrolling into view, make hundredth item relevant and the ancestors. The siblings from 50 to 100 might not be relevant to the user and can change size.
<Frances> flack: We need to be adjusting the scale-target
<Frances> Alan: Not an issue with the proposed resolution
<Frances> flack: instead of calculating target, we can update the target with the location. Such as when the target scrolls into view.
<astearns> ack vmpstr
<Frances> vlad: Can scollintoview without an animation, still need the resolution. The position of the element may be locked in size-overflow, and might not approach the viewport.
<Frances> vlad: Content-visibility:auto state changes, might fire an event in those cases.
<Frances> flack: reasonable argument for accepting proposed resolution, possible other resolutions.
<Frances> Alan: any other comments?
<Frances> PROPOSAL: ScrollIntoView a descendant of element with content-visibility:auto relevant to the use, should also affect ancestors.
<astearns> RESOLVED: add ScrollIntoView to the definition of relevant to the use, should also affect ancestors.
<vmpstr> s/to the use/to the user/