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-scroll-snap-2] Specify scroll-start-target #10227

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

DavMila
Copy link
Contributor

@DavMila DavMila commented Apr 18, 2024

This adds details to the scroll-snap-2 spec[1] to specify scroll-start-target.

[1] https://drafts.csswg.org/css-scroll-snap-2

This adds details to the scroll-snap-2 spec[1] to specify
scroll-start-target.

[1] https://drafts.csswg.org/css-scroll-snap-2
@DavMila DavMila requested a review from flackr April 18, 2024 18:09
<h4 id="initial-scroll-target">Initial scroll target</h4>

The <dfn>initial scroll target</dfn> of a <a>scroll container</a> |scrollcontainer|
with respect to a particular axis is an element or pseudo-element whose {{scroll-start-target}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there use cases for wanting to initially scroll to a different target in each axis? It seems like this adds a fair bit of complexity that could be avoided by having a single element.

For example, what happens if you have a blocktarget that snaps in the inline direction and an inline target that snaps in the block direction?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've been asking the community and hunting for examples, here's the current thoughts:

  1. many examples of wanting to set the block/inline scroll start targets can be achieved by specifying a single intersecting element. but, knowing which element that is this at this intersection might not be known.
  2. consider the example where a scroll-start-target is desired on x but not y. so even tho it's a scroller in both axes, it'd be desirable to only scroll-start one axis at a provided element. like a table and wanting to scroll-start at a row and not having it move the x scroll position.

Copy link
Contributor Author

@DavMila DavMila Apr 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking around on this Adam. Regarding 1., I was wondering, if an author wants to scroll to whatever element is at a particular position, then I guess, in a sense, it's not so much the element they want to scroll to as it is the position. In this case, perhaps an acceptable pattern would be to put a placeholder element at that location and have it be the scroll-start-target. I guess there are multiple ways of constructing a grid and I'm not yet sure if this would be a feasible/acceptable approach to all(any?) of them.

On concern 2, I think that making a row a scroll-start-target wouldn't result in scrolling within that row. I think if you wanted to set something within a row as the scroll-start-target but you only wanted to scroll to the row, not the thing inside the row, then the row should be the scroll-start-target?

css-scroll-snap-2/Overview.bs Outdated Show resolved Hide resolved
|scrollcontainer| with the axis parameter as “inline” and the target parameter
as |inlinetarget|.

The steps to <dfn>get initial scroll offset</dfn> for a <a>scroll container</a> |scrollcontianer|,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be calling into a common algorithm used by the spec rules for scrolling an element into view as we would expect this to behave the same position as you would get by calling scrollIntoView.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we intend for scroll-start-target to only affect its nearest scroll ancestor. Invoking "scrolling an element into view" as written would result in the scrolling of all ancestor scrolling boxes. I've modified it to define an optional nearest_only parameter for this algorithm which will default to false.

css-scroll-snap-2/Overview.bs Outdated Show resolved Hide resolved
css-scroll-snap-2/Overview.bs Outdated Show resolved Hide resolved
|scrollcontainer|. Where multiple such elements or pseudo-elements exist, user-agents
should select the one which comes first in <a href="https://www.w3.org/TR/dom/#concept-tree-order">tree order</a>. Where no such element
or pseudo-element exists, |scrollcontainer|’s <a>initial scroll target</a> in
that axis is null.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you'll also need to define the interaction with the try to scroll to the fragment step of navigation. In particular, this is the part that deals with cases where the element to scroll to is not in the initial render. I think we'll want this to also detect cases where a scroll-start-target was not originally present but shows up in the document.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would still be covered by what you pointed out earlier: scroll-start-target only determines what the initial scroll position should be. So it's implied that a scroll-start-target that shows up later only changes the initial scroll position of some affected scroller but not (necessarily) its current scroll position, similar to place-content affecting the initial scroll position. Or perhaps you mean that all this is somewhat too implicit and needs to be called out directly?

Copy link
Contributor Author

@DavMila DavMila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made suggested changes, ptal, thanks.

@DavMila DavMila requested a review from flackr April 26, 2024 15:13
can be used to change the [=initial scroll position=],
see [[css-align-3#overflow-scroll-position]].
However, some CSS properties can be used to change the initial position. In decreasing order of
precedence, these are:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is just a callout to other properties that can affect the initial scroll position, it probably shouldn't establish the precedence order here (as the other locations should explain how they interact), just point to the list of properties which can then establish how they interact. For example https://www.w3.org/TR/css-align-3/#overflow-scroll-position could say the presence of a scroll-start-target will override the initial scroll position.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, done.

to determine the <a>initial scroll position</a> of |scrollcontainer| along each
axis by running the steps to <a>apply initial scroll targets</a>
User-agents should use the <a>initial scroll target</a> for a <a>scroll container</a>
to determine its <a>initial scroll position</a> by running the steps to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more clear if we have the condition of only doing this when the target is non-null. Also I think it may be worth a mention that this overrides the initial scroll position determined by the align-content and justify-content properties. Alternatively maybe CSS Box Alignment 3 § 5.3 Overflow and Scroll Positions should point to this, but I think just saying this overrides the other is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. The section "scroll-start-target-with-place-content" covers that it overrides place-content which is the shorthand for align-content and justify-content.

1. Let |target| be the <a>initial scroll target</a> for |scrollcontainer|.
1. If |target| is null, return.
1. Run the steps to <a spec="cssom-view-1" lt="scroll a target into view">scroll |target| into view</a>
with |behavior| set to "auto", |block| set to "start", |inline| set to "nearest" and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think we want to share like 95% of the algorithm, except for the last step where it says:

  1. Perform a scroll of box to position, element as the associated element and behavior as the scroll behavior.

As we don't want this to be an actual scroll, but rather just the initial position of the scroller. Note it can actually perform a scroll if the scroll target turns up late in the loading process where that's unavoidable.

As such, we should pull out all of the steps which determine the target position into an algorithm we can invoke here to calculate the initial scroll position. I think this might also resolve the issue of needing to only scroll the nearest scroller without needing to add the flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I've taken a stab at extracting the portion of the algorithm that determines the position into a set of steps to "determine the scroll-view position". Lmk wyt.

Scrolling to the fragment for a Document</a> might set the <a>scroll position</a> of a
<a>scroll container</a>. This might <a spec="cssom-view-1" lt="scroll an element">scroll</a>
the scroll container away from its <a>initial scroll position</a>.
It might occur that an element which is a {{scroll-start-target}} for that <a>scroll container</a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace tab character after {{scroll-start-target}} with a space.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

{{scroll-start-target}} does set the <a>initial scroll position</a> of the
<a>scroll container</a> but does not affect the current <a>scroll position</a> of the
<a>scroll container</a> i.e. does not cause it to be scrolled in response to
the late arrival of the {{scroll-start-target}}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I think this is a little vague and should be rewritten to say when this will scroll to the scroll-start-target. I.e. if a scroll-start-target is discovered and we have not scrolled to a fragment. We don't need to mention the initial scroll position part, since scrolling to a fragment doesn't change the initial position but just scrolls away from the initial position, but we do want scroll-start-target to scroll to late discovered targets as long as we haven't done a fragment scroll.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've tried to make it a bit clearer by pointing to the html spec on updating the document to cover arrivals of scroll-start-targets after the first layout of a scroll container.

David Awogbemila added 2 commits May 16, 2024 10:52
Define initial scroll position precedence in css-overflow-3.
De-vaguify post-first layout scroll-start-target & point to document
update process.
Extract scroll-view position algorithm for standalone use.
@DavMila DavMila requested a review from flackr May 16, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants