Skip to content

[scroll-animations-1] Consider initial value of auto for view-timeline-inset #7747

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

Closed
flackr opened this issue Sep 15, 2022 · 5 comments
Closed

Comments

@flackr
Copy link
Contributor

flackr commented Sep 15, 2022

We previously discussed the scroll-animations-1 view-timeline-inset in the August F2F discussion here and at the time I didn't object to an initial value of 0 based on the understanding that the default for scroll-padding was to include some overlap, so we left it as that. However, looking into this, it seems that the overlap in page-down is not part of scroll-padding but in addition to it, as can be seen on this demo with scroll-padding: 0px where page down scrolls 87.5% of the optimal viewing region defined by the 0px scroll-padding.

Given that the default page-down overlap is not part of scroll-padding, and that the default scroll-padding: auto is typically 0 but allowed to avoid obscured fixed position areas I think that we should make auto the default for view-timeline-inset since most of the time a developer would want to avoid those obscured regions for view timelines as well.

@flackr flackr added the scroll-animations-1 Current Work label Sep 15, 2022
@fantasai
Copy link
Collaborator

However, looking into this, it seems that the overlap in page-down is not part of scroll-padding but in addition to it, as can be seen on this demo with scroll-padding: 0px where page down scrolls 87.5% of the optimal viewing region defined by the 0px scroll-padding.

That's a spec violation against css-scroll-snap, then. Zero should mean zero, otherwise the author doesn't have the ability to control it.

@flackr
Copy link
Contributor Author

flackr commented Oct 31, 2022

However, looking into this, it seems that the overlap in page-down is not part of scroll-padding but in addition to it, as can be seen on this demo with scroll-padding: 0px where page down scrolls 87.5% of the optimal viewing region defined by the 0px scroll-padding.

That's a spec violation against css-scroll-snap, then.

That's not clear to me. I'd suggest an update to the language if this should be the case. Right now it says to adjust the scroll container’s optimal viewing region for the purpose of paging and scroll-into-view operations, but it's not clear that page down is expected to be exactly the defined optimal viewing region. The browser interprets it as the visible content area and additionally intentionally tries to ensure overlap so that you can follow where you came from.

Though I think making this change will be breaking since sites today are used to setting scroll-padding to the region which is obscured but don't expect page down to scroll exactly the visible region. Similarly, including the 12.5% default padding in scroll-padding: auto would mean that scrollIntoView operations must scroll elements further into view when linked to / typed in / scrolled into view via JS.

Zero should mean zero, otherwise the author doesn't have the ability to control it.

So you can control the distance (at least in Chrome - looks like Firefox and Safari do not modify the scroll distance of page down. I will add a test for this) - but the page down scroll distance scrolls 87.5% of the developer defined optimal viewing area on a page down keypress. Here's an example where the optimal viewing area is 50vh (root scroller, overflow scroller) which in Chrome scrolls 87.5% of 50vh with the same goal of ensuring some overlap between the previous and next page.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [scroll-animations-1] Consider initial value of auto for view-timeline-inset, and agreed to the following:

  • RESOLVED: scroll overlap range is in addition to scroll-padding, view-timeline-inset should have an initial value of auto
The full IRC log of that discussion <emilio> flackr: we discussed this in our previous session and resolved that the initial view-timeline-inset should be 0 based on the assumption that scroll-padding was used to set a margin for page-down operations
<emilio> ... but all browsers actually scroll 87% of the remaining area after you remove scroll-padding
<emilio> ... so scroll-padding is strictly used for occluding element
<emilio> ... and changing that would break sites that use it to exclude occluding elements
<Rossen_> q?
<emilio> ... so proposal is to have auto which is scrollport minus scroll-padding
<emilio> ... to account for invisible elements
<emilio> fantasai: one of the goals for scroll-padding was to give the author the ability to control that overlap
<emilio> ... or zero, but I understand we have a compat problem but that brings the question of how we want to address that use case
<fantasai> s/or zero/or set it to zero/
<emilio> flackr: so... the issue is that devs have used it assuming that it's strictly an overlapping-element-region
<emilio> ... if we change the interpretation as offset-defining the page region you change page-down behavior
<emilio> ... I guess it's complicated wrt view-timeline-inset
<emilio> fantasai: I think for view-timeline-inset should just account for padding
<emilio> q+
<emilio> fantasai: that's what auto would do, it should not pay attention to the absolute amount of scroll
<emilio> flackr: developers are using it because we don't use that as the page value
<Rossen_> ack emilio
<fantasai> emilio: I was going to agree with fantasai
<emilio> fantasai: ua should provide some overlap for continuity purposes on top of that
<fantasai> s/ua should/then we need to update the scroll-snap spec to say the ua should/
<emilio> flackr: [on-the-spot compat check]
<fantasai> flackr: Chrome and Safari are using 87.5% of the scroll-padding inset area
<emilio> flackr: firefox is not using scroll-padding at all for page amount, webkit/blink have 87% of scroll-padding
<fantasai> flackr: Firefox is not using scroll-padding at all
<fantasai> emilio: I think scroll-padding should be accounted for
<fantasai> emilio: if we don't, it's an oversight
<fantasai> emilio: It might be we don't want to define as the whole region
<fantasai> emilio: but, we could still not have an auto value
<fantasai> emilio: and use zero and have ?? use scroll-padding area
<fantasai> emilio: I don't care either way, having a zero start or auto
<emilio> flackr: I just remembered the other complication, we use the padding area for how much additional scrolling we need to do to scroll things into view
<emilio> ... so the way it's defined it's better suited to define overlapping elements
<emilio> ... to me that implies it should be fine to have view-timeline follow that as the default
<emilio> fantasai: I think the proposed resolution is: #1, scroll overlap range is in addition to scroll-padding, that requires scroll snap changes
<emilio> ... #2 changing view-timeline-inset to have an initial value of auto
<emilio> RESOLVED: scroll overlap range is in addition to scroll-padding, view-timeline-inset should have an initial value of auto

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 24, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1824246
gecko-commit: 7b0a3e4d754ab19861a6507c0d9ec156c147e152
gecko-reviewers: emilio
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 25, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1824246
gecko-commit: 7b0a3e4d754ab19861a6507c0d9ec156c147e152
gecko-reviewers: emilio
@fantasai fantasai added the css-scroll-snap-1 Current Work label Mar 27, 2023
@fantasai
Copy link
Collaborator

Edited in the initial value; leaving open for the scroll-snap edits.

marcoscaceres pushed a commit to web-platform-tests/wpt that referenced this issue Mar 28, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1824246
gecko-commit: 7b0a3e4d754ab19861a6507c0d9ec156c147e152
gecko-reviewers: emilio
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 28, 2023
…r=emilio

Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487
cookiecrook pushed a commit to cookiecrook/wpt that referenced this issue Mar 29, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1824246
gecko-commit: 7b0a3e4d754ab19861a6507c0d9ec156c147e152
gecko-reviewers: emilio
@fantasai
Copy link
Collaborator

fantasai commented Apr 3, 2023

I think @flackr is right and this is already implied (as resolved above) in the Scroll Snap spec, so closing out.

@fantasai fantasai closed this as completed Apr 3, 2023
cookiecrook pushed a commit to cookiecrook/wpt that referenced this issue Apr 8, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1824246
gecko-commit: 7b0a3e4d754ab19861a6507c0d9ec156c147e152
gecko-reviewers: emilio
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Nov 17, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Nov 17, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Nov 18, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Nov 18, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Nov 18, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Nov 20, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Nov 20, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487
mrobinson pushed a commit to servo/servo that referenced this issue Nov 21, 2023
Per the proposal in w3c/csswg-drafts#7747,
we change view-timeline-inset to have an initial value of auto.

Differential Revision: https://phabricator.services.mozilla.com/D173487
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants