w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 664 forks source link

[css-contain-2] apply containment on `content-visibility: visible` ; use `normal` as initial value #5695

Open tabatkins opened 4 years ago

tabatkins commented 4 years ago

Currently, content-visibility is specified to have three values: visible, which does nothing; hidden, which hides the element and applies containments; and auto, which switches between hidden behavior and a visible-with-containments behavior that is otherwise not representable via content-visibility.

This seems weird! Particularly with [hidden-matchable](), where the author is expected to flip the element into being visible, flipping it to visible is probably wrong (you lose the containments, unless you've applied them independently) and flipping it to auto feels awkward and, depending on scroll position, might not actually make the element visible.

fantasai and I think that it would be better to have an explicit value for each of auto’s states, calling them hidden and visible; and to call the initial value, which has no effect on the element, normal.

vmpstr commented 4 years ago

This makes sense to me.

I would do some minor bikeshedding to keep visible as the no-effect value, and perhaps name the auto state when not skipped can be named something like contained. So auto would toggle between contained and hidden. It also doesn't change existing meaning of visible, which is good

That being said, visible and normal is maybe OK, but less descriptive in my mind.

fantasai commented 4 years ago

I would do some minor bikeshedding to keep visible as the no-effect value

Why?

vmpstr commented 4 years ago

Why?

For consistency with visibility and to better differentiate visible and normal, both of which read like no-effect values to me

chrishtr commented 4 years ago

Two points:

  1. These changes sound not web compatible, given the feature has already shipped in one browser.

  2. Regarding:

flipping it to auto feels awkward and, depending on scroll position, might not actually make the element visible.

Is it awkward? What is the problem with skipping the element subtree if it isn't on screen anyway?

fantasai commented 3 years ago

@chrishtr We're not taking away any values, just changing 'visible' to have a containment side-effect when set explicitly (rather than as the initial value) and giving the initial value a new name.

Generally, you don't want to be flipping containment on/off anyway, right? That impacts layout in significant ways, and the point is to flip visibility without impacting layout, right? So if you're using content-visibility seems unlikely that you'd want to have containment suddenly flipped off--I'd expect most such pages would be setting contain themselves to avoid that, in which case they won't be affected by a change here.

chrishtr commented 3 years ago

@chrishtr We're not taking away any values, just changing 'visible' to have a containment side-effect when set explicitly (rather than as the initial value) and giving the initial value a new name.

Adding a new value is fine, it just needs to be a name that does not break existing content already relying on the current values in Chromium.

Generally, you don't want to be flipping containment on/off anyway, right? That impacts layout in significant ways, and the point is to flip visibility without impacting layout, right?

Agreed. The previous "plan of record" my team was going to pursue was will-change: content-visibility which would have the same effect, but this works also.

css-meeting-bot commented 3 years ago

The CSS Working Group just discussed [css-contain-2] apply containment on `content-visibility: visible` ; use `normal` as initial value, and agreed to the following:

The full IRC log of that discussion <dael> Topic: [css-contain-2] apply containment on `content-visibility: visible` ; use `normal` as initial value
<dael> github: https://github.com/w3c/csswg-drafts/issues/5695
<vmpstr> auto?
<dael> TabAtkins: The issue here is that the hidden-matchable value switches content between 2 behaviors. One idential to hidden and one that's indescribable in current. That feels weird, don't normally have auto that goes to unrepresentable value
<dael> TabAtkins: Prop the other behavior that makes things visible have an explicit keyword
<dael> TabAtkins: Original proposal in thread is name that visible and change the default to normal. Chris points out that it's been shipped in Chrome so visible may be locked.
<dael> TabAtkins: I don't specifically want a name but I think we need the property
<dael> Rossen_: Is there data if it shoudl be frozen?
<dael> TabAtkins: content-visibility as a property is 1% of page loads. That gives reasonable sense there would be problems if we change
<vmpstr> q+
<dael> fantasai: Cases where likely to break seem unusual. You have to set it away from inital value and you would have to not be using containment. Seems like most of use cases you'd use containment with this or have explicit width and height in which case no change in behavior if we made this change
<dael> fantasai: I don't know for sure, but I think there's a change its safe. If there's a better keyword fine, but I think the properties are better if we make normal the default value
<dael> TabAtkins: I prop if people think general idea of the mode having a keyword we resolve on that. Then move back to GH on name and bring it back in a week or two
<vmpstr> q-
<dael> Rossen_: Works for me
<fantasai> s/change its/good chance its/
<dael> Rossen_: Okay, let's continue there
<dael> TabAtkins: Can we get the resolution to make a keyword for this behavior?
<dael> fantasai: Prop: Have separate keywords for the visbile and contained value vs initial value
<dael> Rossen_: Objections?
<dael> RESOLVED: Have separate keywords for the visbile and contained value vs initial value
vmpstr commented 2 years ago

The resolution was to come up with a name for this mode and bring it back in a week or two, but a year or two is also good! :)

I believe there are two suggestions, so we should pick out of the following:

  1. content-visibility: normal | visible | hidden | auto.

    • normal: the default state which has no effect
    • visible: the state which applies contain: layout style paint but not size
    • hidden: the state which applies contain: layout style paint size and doesn't paint the contents of the element
    • auto: toggles between hidden and visible based on relevance to the user
  2. content-visibility: visible | contained | hidden | auto

    • visible: the default state which has no effect
    • contained: the state which applies contain: layout style paint but not size
    • hidden: the state which applies contain: layout style paint size and doesn't paint the contents of the element
    • auto: toggles between hidden and contained based on relevance to the user

I slightly prefer 2, but contained may be confusing in that it doesn't apply size containment, so it's kind of partially contained. A possible problem with 1 is that it changes the effect of visible, so we have to be a bit careful and check if that value is actually being used in developer stylesheets