Skip to content
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

[css-overflow-5] Allowing markers to be active even when not scrollable to aligned position #10738

Closed
flackr opened this issue Aug 13, 2024 · 8 comments · Fixed by #11320
Closed

Comments

@flackr
Copy link
Contributor

flackr commented Aug 13, 2024

There are many situations in which it may not be possible to scroll to a position that aligns with the natural scrolling position of every marker. For example, in https://flackr.github.io/carousel/examples/carousel/thumbnails/ is is not possible to scroll to a position that centers the first two or last two items. E.g.

In the following scrolling element, it is not possible to scroll the first two boxes to their center snap aligned position:
scroller

This means that if the active marker is determined solely by the item closest to being aligned https://drafts.csswg.org/css-overflow-5/#scroll-container-scroll it will not be possible to show the first two / last two as active.

Should we consider a model where the browser tracks the actual element that has requested to be scrolled into view? This would allow the edge elements to show as active when clicking on those markers. This could have additional benefits if we treat this element as the anchor node for anchor node selection as well. Note that subsequent user scrolling would likely undo the active styling.

Alternately, if we can't solve this in a nice way we could add a note recommending authors avoid these situations by:

  • adding padding to the edges of their scrolling element, or
  • ensuring their scroll marker elements are scrollport sized, or
  • enabling infinite / cyclical scrolling as in [css-overflow] Infinite scrolling #5411
@tabatkins
Copy link
Member

Hm, so the suggestion would be that, if a scroll-to-element operation is performed, we properly indicate that element as the active one, but if you scroll in any way that's not directly to an element (manually, or via scrollTo or scrollBy), we re-select the active element by "closest to aligned" as normal?

That actually sounds pretty reasonable to me. Playing with your example, it's somewhat confusing/weird that clicking on the last several items doesn't do anything, even change the scroll marker. I think it would be a better UI to have the clicked marker styled as active, even if it doesn't actually move the scroller at all.

@flackr
Copy link
Contributor Author

flackr commented Oct 30, 2024

Yup, and further, we could use the scrolled-to-element, when set, as the anchor node which would make navigating to a page with a fragment always keep the targeted fragment visible rather than potentially anchoring to some other element as page content loads in.

@flackr
Copy link
Contributor Author

flackr commented Oct 30, 2024

So the proposal is:
We track the last scroll targeted element within each scrolling container, initially null.

When any scroll of a scroll container occurs:

  1. If the scroll targets a particular element, (e.g. scrollIntoView, fragment navigation), then that element is used as the last scroll targeted element.
  2. Otherwise, if the element snaps to a particular element, that element is selected as the last scroll targeted element.
  3. Otherwise, it is set to null.

This last scroll targeted element is used as the anchor node for scroll anchoring, and as the start point to search for the current scroll target for scroll markers.

Some details:

  1. If the target is in a nested scroller, should we track the nested scroller instead? I think it's strictly more powerful if we track the targeted element but could see an argument for this.
  2. If the target node is removed, presumably we want to forget about it at that point, but this does mean a moved node would be lost.

@flackr flackr added the Agenda+ label Oct 30, 2024
@flackr flackr changed the title [overflow-5] Allowing markers to be active even when not scrollable to aligned position. [css-overflow-5] Allowing markers to be active even when not scrollable to aligned position. Nov 1, 2024
@flackr flackr changed the title [css-overflow-5] Allowing markers to be active even when not scrollable to aligned position. [css-overflow-5] Allowing markers to be active even when not scrollable to aligned position Nov 1, 2024
@kizu
Copy link
Member

kizu commented Nov 20, 2024

For Table of Contents cases, I can generally remember 3 different types of designs that are related to this issue:

  1. The “only the last visible scroll-target is highlighted” (the common use case)
  2. The “all visible before and including the last visible scroll-target are highlighted” (the one in this issue + cases when there are many small sections in ToC and we want to mark all of them)
  3. All previous scroll-targets are highlighted (when all “already read” sections of a ToC are dimmed, for example).

I wonder if we'd want to have a control over which we choose, and maybe more than one pseudo-class for this? So not just :target-current, but two more (needs bikeshedding, but basically :target-current-visible for 2 and :target-previous for 3 (and, I guess, we could want :target-next? alongside))

We could have some logic similar to the one in the latest comment, but I wonder if having more explicit way to control it could be better? (alternative to multiple pseudoclasses could be a separate property that says how the :target-current should be selected, I guess?)

@flackr
Copy link
Contributor Author

flackr commented Nov 20, 2024

@kizu this issue is focused specifically on enabling :target-current to have a target that best represents the common case of only one target is highlighted. I'm happy to pursue adding other pseudo-classes but I don't think it changes that we should resolve on behavior for this pseudo-class to be able to select a particular element where mutliple share the same scroll aligned position.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-overflow-5] Allowing markers to be active even when not scrollable to aligned position, and agreed to the following:

  • RESOLVED: Adopt proposal in https://github.com/w3c/csswg-drafts/issues/10738#issuecomment-2448204794 to anchor to identified elements
The full IRC log of that discussion <TabAtkins> q+
<dbaron> flackr: It's easy to create a design where you can't reach the natural scroll-aligned position of the first or last elements in your scroller.
<dbaron> flackr: this leads to an odd usability issue that clicking on them doesn't make the mrakers active
<dbaron> flackr: we have the same problem today where if you follow an anchor link to something whose scroll position can't be reached, we could scroll you away from that link as a resul tof scroll anchoring
<dbaron> flackr: My proposal here is that browsers track when the scroll in a scroll container trageted a particular element, and then use that element as the targeted item until the user does another scroll.
<astearns> ack TabAtkins
<dbaron> flackr: for the purposes of markers this would mean that the marker is the first marker at or before that targeted element
<TabAtkins> https://github.com//issues/11165
<dbaron> TabAtkins: I was wondering how this would interact with 11165 which is about indicating markers in the unscrollable region that you can't reach... but I don't think it does, so I'm good with what's suggested here.
<astearns> ack fantasai
<dbaron> fantasai: Overall I agree with the direction... one thing is if we've targeted a particular element that's not the one we'd select at this scroll position because we're urther down towards the end... the user would have to scroll back up to shift focus to something else?
<dbaron> flackr: right
<dbaron> fantasai: so if they continued to scroll down it would not cause it to recalculate
<dbaron> flackr: they're not changing their scroll position by scrolling down
<dbaron> fantasai: then this seems fine to me
<dbaron> +1 to element tracking from me to -- something I wanted to do a very long time ago well before scroll anchoring was a thing :-)
<fantasai> PROPOSED: Adopt proposal in https://github.com//issues/10738#issuecomment-2448204794 to anchor to identified elements
<dbaron> Proposed resolution from flackr: each scrolling container tracks the last element scrolled into view until that container's scroll position is otherwise changed
<dbaron> RESOLVED: Adopt proposal in https://github.com//issues/10738#issuecomment-2448204794 to anchor to identified elements
<TabAtkins> ScribeNick: TabAtkins

@dbaron
Copy link
Member

dbaron commented Nov 20, 2024

FWIW, I like the idea of targeting elements for scroll anchoring; I suggested it a very long time ago but never got to implementing it.

flackr added a commit to flackr/csswg-drafts that referenced this issue Dec 3, 2024
When a targeted scroll is performed, use that target to determine the active scroll marker. Fixes w3c#10738.
flackr added a commit that referenced this issue Dec 6, 2024
When a targeted scroll is performed, use that target to determine the active scroll marker. Fixes #10738.
@DavMila
Copy link
Contributor

DavMila commented Dec 20, 2024

As we're implementing this in chromium, just wanted to make a note on one detail regarding ::columns, which I don't think has been explicitly talked about but perhaps flows naturally from what's spec'd.
In the following scenario where ::scroll-markers are created both on elements within the container and on the columns of the container:

<div class=columns>
  <fragment col1> ::scroll-marker
    <p>1</p> 
    <p>2</p> ::scroll-marker
    <p>3</p>
  </fragment>
  <fragment col2> ::scroll-marker
    <p>4</p>
    <p>5</p> ::scroll-marker
    <p>6</p>
  </fragment>
</div>

if scrollIntoView is called on <p>6</p>, then <p>5</p>'s scroll-marker should be selected and not col2's. But, if scrollIntoView is called on <p>4</p>, then col2 should be the selected scroll-marker and not <p>2</p>.

aarongable pushed a commit to chromium/chromium that referenced this issue Dec 20, 2024
The CSS working group resolved[1] that when a scroll operation is aimed
at an element, i.e. Element.scrollIntoView, the associated
scroll-marker-group should select the active scroll-marker based on the
element which the operation is intending to scroll to. In such cases,
the scroll-marker that should be selected is the scroll-marker
associated with the first scroll target (a scroll target is an element
which generates a scroll-marker) found through a search starting from
the target of the scrollIntoView itself and backwards in tree-order.
As soon as some other type of scroll occurs, e.g. Element.scrollTo, or
a user gesture scroll, the scroll-marker-group should no longer
consider its active marker pinned, i.e. it should be based on the
scroll position.

This patch implements this for elements in general, but not for
::column pseudo elements which may also act as scroll targets since
::column::scroll-marker is allowed. The ::column case needs to be
handled specially as ::column pseudos are not parents of the elements
which are flowed into them in the DOM tree. This will be done in a
follow-up patch.

[1] w3c/csswg-drafts#10738 (comment)

Bug: 380062280
Change-Id: I363e0f055f9791ead0b35f4bbe037db91f299624
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6089232
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: David Awogbemila <awogbemila@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399197}
aarongable pushed a commit to chromium/chromium that referenced this issue Dec 20, 2024
::column pseudos[1] may create ::scroll-markers, but do not exist in the
DOM tree as parents of the elements which are flowed into them.
They also do not appear to have LayoutObjects and are not found[2]
in the layout tree. So, when searching for the appropriate scroll-marker
to select for a targeted scroll we need to consider whether the
scroll-marker-generating element (marker-generating-element) found in
the layout tree is also flowed into the same scroll-marker-generating
::column (marker-generating-column) as the target of the scrollIntoView
operation. If the target and marker-generating-element are both in
marker-generating-column, marker-generating-element is the preferred[3]
choice. The only means we have of checking whether an element is flowed
into a ::column is by looking at the rect associated with that ::column.

[1] https://drafts.csswg.org/css-multicol-2/#column-pseudo
[2] https://source.chromium.org/chromium/chromium/src/+/165f5ddad25b289f886a248ca810d075b1e7bb87:third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc;l=254-255
[3] w3c/csswg-drafts#10738 (comment)

Bug: 380062280
Change-Id: Ie2ad58c3beca13a4945325d9aa9c4aec6a5fc332
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6088285
Commit-Queue: David Awogbemila <awogbemila@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399198}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 20, 2024
The CSS working group resolved[1] that when a scroll operation is aimed
at an element, i.e. Element.scrollIntoView, the associated
scroll-marker-group should select the active scroll-marker based on the
element which the operation is intending to scroll to. In such cases,
the scroll-marker that should be selected is the scroll-marker
associated with the first scroll target (a scroll target is an element
which generates a scroll-marker) found through a search starting from
the target of the scrollIntoView itself and backwards in tree-order.
As soon as some other type of scroll occurs, e.g. Element.scrollTo, or
a user gesture scroll, the scroll-marker-group should no longer
consider its active marker pinned, i.e. it should be based on the
scroll position.

This patch implements this for elements in general, but not for
::column pseudo elements which may also act as scroll targets since
::column::scroll-marker is allowed. The ::column case needs to be
handled specially as ::column pseudos are not parents of the elements
which are flowed into them in the DOM tree. This will be done in a
follow-up patch.

[1] w3c/csswg-drafts#10738 (comment)

Bug: 380062280
Change-Id: I363e0f055f9791ead0b35f4bbe037db91f299624
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6089232
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: David Awogbemila <awogbemila@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399197}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 20, 2024
::column pseudos[1] may create ::scroll-markers, but do not exist in the
DOM tree as parents of the elements which are flowed into them.
They also do not appear to have LayoutObjects and are not found[2]
in the layout tree. So, when searching for the appropriate scroll-marker
to select for a targeted scroll we need to consider whether the
scroll-marker-generating element (marker-generating-element) found in
the layout tree is also flowed into the same scroll-marker-generating
::column (marker-generating-column) as the target of the scrollIntoView
operation. If the target and marker-generating-element are both in
marker-generating-column, marker-generating-element is the preferred[3]
choice. The only means we have of checking whether an element is flowed
into a ::column is by looking at the rect associated with that ::column.

[1] https://drafts.csswg.org/css-multicol-2/#column-pseudo
[2] https://source.chromium.org/chromium/chromium/src/+/165f5ddad25b289f886a248ca810d075b1e7bb87:third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc;l=254-255
[3] w3c/csswg-drafts#10738 (comment)

Bug: 380062280
Change-Id: Ie2ad58c3beca13a4945325d9aa9c4aec6a5fc332
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6088285
Commit-Queue: David Awogbemila <awogbemila@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399198}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 20, 2024
The CSS working group resolved[1] that when a scroll operation is aimed
at an element, i.e. Element.scrollIntoView, the associated
scroll-marker-group should select the active scroll-marker based on the
element which the operation is intending to scroll to. In such cases,
the scroll-marker that should be selected is the scroll-marker
associated with the first scroll target (a scroll target is an element
which generates a scroll-marker) found through a search starting from
the target of the scrollIntoView itself and backwards in tree-order.
As soon as some other type of scroll occurs, e.g. Element.scrollTo, or
a user gesture scroll, the scroll-marker-group should no longer
consider its active marker pinned, i.e. it should be based on the
scroll position.

This patch implements this for elements in general, but not for
::column pseudo elements which may also act as scroll targets since
::column::scroll-marker is allowed. The ::column case needs to be
handled specially as ::column pseudos are not parents of the elements
which are flowed into them in the DOM tree. This will be done in a
follow-up patch.

[1] w3c/csswg-drafts#10738 (comment)

Bug: 380062280
Change-Id: I363e0f055f9791ead0b35f4bbe037db91f299624
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6089232
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: David Awogbemila <awogbemila@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399197}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Dec 20, 2024
::column pseudos[1] may create ::scroll-markers, but do not exist in the
DOM tree as parents of the elements which are flowed into them.
They also do not appear to have LayoutObjects and are not found[2]
in the layout tree. So, when searching for the appropriate scroll-marker
to select for a targeted scroll we need to consider whether the
scroll-marker-generating element (marker-generating-element) found in
the layout tree is also flowed into the same scroll-marker-generating
::column (marker-generating-column) as the target of the scrollIntoView
operation. If the target and marker-generating-element are both in
marker-generating-column, marker-generating-element is the preferred[3]
choice. The only means we have of checking whether an element is flowed
into a ::column is by looking at the rect associated with that ::column.

[1] https://drafts.csswg.org/css-multicol-2/#column-pseudo
[2] https://source.chromium.org/chromium/chromium/src/+/165f5ddad25b289f886a248ca810d075b1e7bb87:third_party/blink/renderer/core/page/scrolling/snap_coordinator.cc;l=254-255
[3] w3c/csswg-drafts#10738 (comment)

Bug: 380062280
Change-Id: Ie2ad58c3beca13a4945325d9aa9c4aec6a5fc332
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6088285
Commit-Queue: David Awogbemila <awogbemila@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1399198}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants