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] Support scroll-padding on all elements. #7931

Open
flackr opened this issue Oct 20, 2022 · 6 comments
Open

[css-scroll-snap] Support scroll-padding on all elements. #7931

flackr opened this issue Oct 20, 2022 · 6 comments

Comments

@flackr
Copy link
Contributor

flackr commented Oct 20, 2022

The css-scroll-snap-1 spec specifies that scroll padding specifies offsets that define the optimal viewing region, however currently this is limited to scroll containers which implies a fixed optimal viewing region for all content in the scroll container.

A common use of position: sticky is to replace fixed position header bars. Consider the demo at:
https://flackr.github.io/web-demos/sticky/scrollIntoView/header-input.html

In this example, the optimal viewing region for most of the content is the entire scroller, however in order to ensure that the anchor links to the comments scroll to the correct place, scroll-padding needs to be added to the scroller (the root) to avoid them being under the header.

This works, however, if you scroll down and type into the input box the browser tries to scroll it into view (which should scroll it into the optimal viewing region). This results in each keypress scrolling up.

I propose that if we supported (and accumulated) scroll-padding and scroll-margin from all elements between the target and the scrolling container, the site could be redesigned so that the scroll padding to avoid the sticky header was only defined for the comments section which is obscured by that sticky header. This would result in the correct scrolling for the comments as well as correct scrolling for content outside of that sub view.

@tabatkins
Copy link
Member

Oh that "scroll the (sticky, already visible) input into view" behavior is pretty bad, yeah.

Hm, your idea does seem pretty reasonable at first glance.

(I'm still bothered by us not having a way to set scroll-padding to respond to the actual height of some element; having to hardcode 51px in your example is such a code smell.)

@SebastianZ
Copy link
Contributor

(I'm still bothered by us not having a way to set scroll-padding to respond to the actual height of some element; having to hardcode 51px in your example is such a code smell.)

Well, the spec. already allows UAs to choose the height of the sticky element as offset via the initial auto value. It even provides a similar example.

So maybe the description of the auto value should be more explicit about sticky-positioned and fixed-positioned elements and provide some algorithm to determine the offset. Anything beyond that may be left to the heuristics of UAs.

Sebastian

@fantasai
Copy link
Collaborator

fantasai commented Dec 6, 2022

I propose that if we supported (and accumulated) scroll-padding and scroll-margin from all elements between the target and the scrolling container [...]

scroll-padding applies to the scroll container, not to the individual elements, so I don't understand how accumulating it makes any sense...? If you want per-element padding, use scroll-margin.

Wrt accumulating scroll-margin, this would not give good results on e.g. nested sections. Consider a document which has section { scroll-margin: 1em; } and the TOC links to its corresponding section elements. If we accumulated scroll-margin, a section nested three levels deep would have 3em of scroll margin, which is too much.

however, if you scroll down and type into the input box the browser tries to scroll it into view (which should scroll it into the optimal viewing region). This results in each keypress scrolling up.

I think a reasonable thing to do here would be to except fixed and sticky elements from trying to honor scroll-padding / scroll-margin. (For stickypos, probably we'd want it to take the minimum of the scroll-margin and the sticky offset.)

@flackr
Copy link
Contributor Author

flackr commented Dec 6, 2022

I propose that if we supported (and accumulated) scroll-padding and scroll-margin from all elements between the target and the scrolling container [...]

scroll-padding applies to the scroll container, not to the individual elements, so I don't understand how accumulating it makes any sense...? If you want per-element padding, use scroll-margin.

This padding conceptually should be added inside the scrollport box not expanding the target edges - hence why I think padding makes more sense. That said, I think the math works out the same either way so if I'm the minority on this POV we can go with margin. Though using scroll-padding has the advantage that developers cannot use it on non-scrollers today so it would be unlikely to already be specified on non-scrollers reducing compat risk.

Wrt accumulating scroll-margin, this would not give good results on e.g. nested sections. Consider a document which has section { scroll-margin: 1em; } and the TOC links to its corresponding section elements. If we accumulated scroll-margin, a section nested three levels deep would have 3em of scroll margin, which is too much.

If you only need 1em of scroll margin then only the top section should add scroll-margin. However, if the top section needs 1em of scroll-margin, then scrolling anything within it should also need 1em of scroll margin.

however, if you scroll down and type into the input box the browser tries to scroll it into view (which should scroll it into the optimal viewing region). This results in each keypress scrolling up.

I think a reasonable thing to do here would be to except fixed and sticky elements from trying to honor scroll-padding / scroll-margin. (For stickypos, probably we'd want it to take the minimum of the scroll-margin and the sticky offset.)

This causes bugs when you have sticky elements which are occluded by other sticky or fixed pos headers where the scroll margin was intentional to ensure they avoid those headers. I'll add a change to my demo to make this use case clear.

@jonjohnjohnson
Copy link

Could we also keep this case in mind when solving the issue?

While scrolling to a target that is continually within its stuck state, the scroller doesn't consistently take scroll-margin into account.

@flackr
Copy link
Contributor Author

flackr commented Mar 10, 2023

Could we also keep this case in mind when solving the issue?

While scrolling to a target that is continually within its stuck state, the scroller doesn't consistently take scroll-margin into account.

That is exactly one of the cases exemplified in my demo and being solved here - the need to have a different scroll-padding for the content below the sticky header indicating that only this content following the sticky header needs additional scroll padding so that scrolling the input in the sticky header does not add the padding.

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

5 participants