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-1] Snap area trapping behavior of non scrollable elements #4496

Closed
majido opened this issue Nov 7, 2019 · 9 comments
Closed

Comments

@majido
Copy link
Contributor

majido commented Nov 7, 2019

Here is the current spec text related to an ancestor element's trapping behavior for snap areas.

A box captures snap positions if it is a scroll container or has a value other than none for scroll-snap-type. If a box’s nearest snap-position capturing ancestor on its containing block chain is a scroll container with a non-none value for scroll-snap-type, that is the box’s scroll snap container. Otherwise, the box has no scroll snap container, and its snap positions do not trigger snapping.

This allows any regular element which is not a scroll container to trap snap areas.

I tested this on latest Chrome, Safari, Firefox and as far as I can tell no engine does this. Here is a simple test page that shows this. Also I don't think we have a wpt test case for it.

Beside, one can simply achieve the same trapping behavior just by having an element with overflow auto or hidden that does not actually overflow. This works correctly across all engines.

Given that no engine implements this, and the behavior can be achieved without much problem, I suggest we drop this requirement.

I am not really sure what is the motivation behind allowing none scroll container to trap snap areas so maybe I am missing something.

@majido majido added the css-scroll-snap-1 Current Work label Nov 7, 2019
@majido
Copy link
Contributor Author

majido commented Nov 18, 2019

@tabatkins @fantasai wdyt about this?

@fantasai
Copy link
Collaborator

I'm not entirely convinced that we should be requiring to clip overflow in order to trap snap points.

Also wondering if contain: layout should be invoking this behavior? It's not quite layout, but to the extent that snaps within such an element could interfere with user interactions outside it... //cc @tabatkins

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Snap area trapping behavior of non scrollable elements, and agreed to the following:

  • RESOLVED: No change to scroll-snap-type, but contain:layout must trap snaps.
The full IRC log of that discussion <heycam> Topic: Snap area trapping behavior of non scrollable elements
<heycam> github: https://github.com//issues/4496
<heycam> fantasai: snapping is on any descendant
<heycam> ... currently spec says if scroll-snap-type is non-none, it normally only applies to the scroll container
<heycam> ... but if you set it on a descendant element, it has the same affect as a scroll container in that snap areas in that box will no longer affect the ancestor scroll container
<heycam> ... Majid points out this hasn't been implemented
<heycam> ... this use case of wanting to trap any of these snap areas is already handeld by setting overflow to auto or hidden
<heycam> ... which means we can remove this
<heycam> fantasai: not convinced on requiriing overflow be set to do this
<heycam> ... but we have the contain property, which likes to contain things, maybe you want contain layout on it -- and you might want contain:layout to trap snap areas
<heycam> TabAtkins: yes it should
<heycam> fantasai: if we're implementing the possibility of trapping snap areas via contain:layout, seems to make sense we maintain the ability to have this independent switch through the scroll-snap-type property
<heycam> astearns: adding a value to scroll-snap-type?
<heycam> TabAtkins: no, it's just any value other than none
<heycam> TabAtkins: the idea would be there's not enough stuff depending on this not trapping
<heycam> heycam: I kidn of find it weird to re-use scroll-snap-type for this different meaning
<heycam> fantasai: you can think of it as scroll-snap-type always captures snaps
<heycam> fantasai: this is already in the spec
<heycam> ... we might want to consider re-tagging this issue against contain, so that we can make contain:layout do this snapturing
<heycam> RESOLVED: No change to scroll-snap-type, but contain:layout must trap snaps.

@frivoal
Copy link
Collaborator

frivoal commented Apr 23, 2020

I agree with the intent of this resolution, but would like to discuss the precise way to spec it a little further, due to a few principles I think ought to be true:

  • the css-contain spec should not need to be (normatively) revised every time another spec adds a new feature. (Adding notes to highlight the effects of other specs is fine)
  • the css-contain spec should state the requirements in a way that is generic enough to be always true. It's goal is to create a mode where correctness can be guaranteed, even if some otherwise necessary steps are skipped for the sake of optimizations. If we find classes of effects that ought to be affected by containment, that's worth revising css-contain for, but the revision should capture the whole class.
  • other specs which introduce new features ought to clarify how these features interact with containment when that isn't obvious.

So, what I think we should do here is:

  • in the contain spec, add an additional effect to layout containment about scrolling:

    • The content, layout, or scroll position of an element with scroll containment or any of its descendants must not affect the scroll position of any box that is not a descendant of containing box for layout containment.

    On top of that, we could add a note in css-contain pointing readers to css-scroll-snap for details, for convenience.

    Note: See "capturing snap positions" in [[css-scroll-snap]] for a specific mechanism affected by this requirement.

  • in the scroll-snap spec, be specific about the implication for snapping:

    A box captures snap positions if it is a scroll container, or has a value other than none for scroll-snap-type, or if it has layout containment.

This more generic phrasing in the css-contain spec would not only apply to scroll snap, but also would cover potential future features with similar effects. Let's say we later introduce scroll-locking, where you can somehow declare that the scroll position of one box must continuously be adjusted to track the scroll position of another one, so that when you scroll one, the other follows. The generic phrasing I'm proposing above for css-contain would catch that, while phrasing that is specific to scroll snapping would not.

This would be similar to the second effect of layout containment, about fragmentainers with layout containment trapping the rest fragmented flow. It is stated in generic terms, and leaves it up to spec which would run into that possibility, like css-regions would, to clarify what it would mean for them.

@css-meeting-bot
Copy link
Member

css-meeting-bot commented May 6, 2020

The CSS Working Group just discussed scroll-snap.

RESOLVED: revert the previous resolution about layout containment trapping snapping points

The full IRC log of that discussion <fremy> Topic: scroll-snap
<astearns> github: https://github.com//issues/4496
<fremy> florian: a while back we resolve the layout containment traps snapping
<fremy> florian: but it's not clear to me as to what spec should define this behavior
<fremy> florian: I don't think that css-contain should contain all the rules in details for all the other specs
<fremy> florian: I would rather this be added to the snapping spec, because this is where you are most likely to need this
<fremy> florian: but in this particular case
<fremy> florian: scroll snapping discovered a whole class of things we forgot to mention
<florian> The content, layout, or scroll position of an element with scroll containment or any of its descendants must not affect the scroll position of any box that is not a descendant of containing box for layout containment.
<fremy> florian: so I would like to suggest an edit
<fremy> florian: (just typed it above)
<fremy> florian: an example would be scroll synchronization or scroll locking
<astearns> s/of containing/of the containing/
<astearns> (I think)
<fremy> florian: like if you scroll one, the other follows
<fremy> florian: but if you contain the layout, this probably shouldn't work
<fremy> florian: i think this should be specified further in scroll-snap
<fremy> florian: because this slows down the progress of containment
<fremy> florian: and I would rather not
<astearns> ack dbaron
<fremy> dbaron: two contains
<fremy> dbaron: you wrote "scroll containment"
<fremy> dbaron: did you mean to write that?
<dbaron> s/contains/questions/
<fremy> dbaron: is that a new type of containment?
<fremy> florian: no, its wrong.
<fremy> florian: I meant layout containment, good catch
<fremy> dbaron: second question then
<fremy> dbaron: the text seems to imply that if you use the #hash part of the url to target an element, then if it is inside something that has layout containment
<fremy> dbaron: it shouldn't work
<fremy> dbaron: but I don't see why
<fremy> florian: I didn't mean to say that
<fremy> florian: I should fix the text if that incorrectly conveys that meaning
<fremy> fantasai: I would also argue that scroll is not layout
<fremy> TabAtkins: but you need some layout to know what is scrollable
<fremy> florian: but you don't need the layout for the effect
<fremy> florian: I agree it's not strictly layout
<fremy> florian: but you can't delay recomputing a subtree if you have these interactions
<fremy> astearns: so if I understand your proposed text would add further requirements on layout containment
<fremy> astearns: if we don't that means that scroll effects would be have side-effects of forcing layout
<fremy> florian: yes, but we can remove from scroll-snapping all cases we don't like
<fremy> florian: that said, of course, if you focus from js etc then you will have to do layout anyway
<fremy> florian: and that seems reasonable
<fremy> florian: this is "contain" property, not "isolate"
<fremy> florian: I think we mostly want to limit interactions, not especially outside>inside interactions
<fremy> TabAtkins: no
<fremy> TabAtkins: the containment is mostly from the layout perspective, the rest of the page is still allowed to affect
<fremy> fantasai: I think scroll and layout in general are two different things
<fremy> fantasai: maybe we should have a "scroll" containment not bundled in layout containment
<fremy> fantasai: this might need a bit more thought
<fremy> florian: implementors, what's your opinion?
<fremy> florian: is scrolling more like paint, or more like layout?
<fremy> fantasai: well, if you have a widget with 10 elements in it
<fremy> fantasai: and I contain their layout
<fremy> fantasai: for perf reasons
<fremy> fantasai: but I might still want snapping to work
<fremy> florian: if you have mandatory scroll snapping
<fremy> florian: none-of-which has scroll-snap-position
<fremy> florian: but the last one, has one
<fremy> florian: but is off-page
<fremy> florian: if the scroll-snapping is required to work, then you need the layout of all the children to know where to snap
<fremy> fantasai: but if you target it, you still do right?
<fremy> florian: yes?
<fremy> florian: I guess it maybe doesn't break if the elements inside don't have snap points inside
<fremy> TabAtkins: I think this is a particular case
<fremy> TabAtkins: but now that I think about it
<fremy> TabAtkins: but maybe that's not the most common case, maybe we can make an exception
<fremy> florian: then we need a test
<fremy> astearns: are you saying we should undo the previous resolution tab?
<fremy> TabAtkins: yes
<fremy> astearns: heycam felt it was weird anyway last time
<fremy> astearns: so, ok, let's resolve to revert that resolution?
<fremy> astearns: any objection?
<TabAtkins> Except for the case Florian brought up (of an off-screen element being the only snap point in a mandatory container), you only need to care about snaps when the element is on-screen (and you're doing layout anyway), so unless an implementor has a good reason why layout containment trapping snaps would be good for optimization, I think we should *not* trap snaps from layout containment.
<fremy> astearns: RESOLVED: revert the previous resolution about layout containment trapping snapping points
<fremy> ???: which resolution are we reverting?
<fremy> astearns: (restates_
<cbiesinger> s/???/cbiesinger/
<fremy> cbiesinger: ok
<fremy> florian: so we reverted and resolved in the opposite direction right?
<fremy> astearns: yes
<fremy> astearns: also, no engine did trap snap before
<fremy> astearns: so we revert back to what implementations do right now, if I understand

@fantasai
Copy link
Collaborator

I believe the current state of this issue is Rejected No Change based on the CSSWG resolutions. Reasoning was, you might want to prevent snap areas within a container from affecting its scroll position without having side-effects on overflow.

@majido Do you think this would be difficult to implement? Do you think there's something else we should consider?

@majido
Copy link
Contributor Author

majido commented Oct 9, 2020

FWIW when filed this issue the motivation was mostly from implementation perspective.

From Blink perspective the most logical thing for us to do was to update snap element mappings as part of the scroll tree. Because simply adding and removing a scroll container has well-defined hooks which we use to redistribute the snap areas to new scroll containers. Basically treating snapping as a scrolling feature allows us to reuse lots of pre-existing machinery that exists for scrolling.

If any element can capture snap areas then you need to track their add/removal (and possibly maintain a separate tree for them 🤔 ). This is not impossible but I am not sure if it the usecase justifies the implementation cost in Blink. I am no longer have the most up to date information on this feature so take my opinion with a grain of salt. /cc @flackr who may provide better guidance.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-scroll-snap-1] Snap area trapping behavior of non scrollable elements, and agreed to the following:

  • RESOLVED: Mark this property at-risk
The full IRC log of that discussion <dael> Topic: [css-scroll-snap-1] Snap area trapping behavior of non scrollable elements
<dael> github: https://github.com//issues/4496
<dael> fantasai: Added b/c we had original set up scroll-snap-type where if it's non-inital it traps snaps. If elements inside asking for snap position we ignore. scroll cotnainers have to trap. Added additional behavior for scroll-snap-type so if someone wants to say in this area ignore a snap position they could do so
<dael> fantasai: Seems this is difficult to impl for Blink. Do we want to not have the behavior? Would mean only way to prevent contents from having snap behavior is put in a scroll container
<dael> Rossen_: It's a hack. The hack will work. People will hate the hack. It'll probably end up in css hacks books.
<fantasai> https://github.com//issues/4496#issuecomment-706333521
<dael> Rossen_: Is it really that difficult that we should go to that extent.
<dael> fantasai: Here's comment from impl ^
<dael> fantasai: I'm not aware of any requests for the behavior
<dael> Rossen_: non-Blink impl with opinion?
<dael> smfr: My hunch is it doesn't make sense for non-scrollable things to trap snapping
<dael> fantasai: Within that element we don't track snap position
<dael> Rossen_: Image in a carosel scenario?
<dael> fantasai: No. a section and in that section there's properties setting snap points, you can turn that off. You can say this element ignore the snap positions. We can just not have that behavior and see if someone complaints
<fantasai> s/positions/positions inside/
<dael> Rossen_: In interest of time we can resolve here. If there are no strong arguments for keeping it I'm fine with that
<dael> smfr: Looked at WK and would be easy to impl
<dael> Rossen_: Argument for reverting doesn't seem to be a problem for WK
<dael> Rossen_: What if we keep issue open and when we get to actual impl and get experience I think that's when we come back and decide
<dael> fantasai: Mark at-risk?
<dael> Rossen_: Sensible
<dael> Rossen_: Objections?
<dael> RESOLVED: Mark this property at-risk

@fantasai
Copy link
Collaborator

@majido Based on comments from WebKit, we ended up just marking this behavior at-risk. That means, if it's not implemented but everything else is, we'll drop the requirement; if it turns out to get implemented, it'll stay in the spec.

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

6 participants