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-contain-2] apply containment on content-visibility: visible ; use normal as initial value #5695

Open
tabatkins opened this issue Nov 4, 2020 · 8 comments

Comments

@tabatkins
Copy link
Member

tabatkins commented Nov 4, 2020

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
Copy link
Member

vmpstr commented Nov 5, 2020

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
Copy link
Collaborator

fantasai commented Nov 5, 2020

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

Why?

@vmpstr
Copy link
Member

vmpstr commented Nov 5, 2020

Why?

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

@chrishtr
Copy link
Contributor

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
Copy link
Collaborator

@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
Copy link
Contributor

@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
Copy link
Member

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:

  • RESOLVED: Have separate keywords for the visbile and contained value vs initial value
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//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
Copy link
Member

vmpstr commented Sep 19, 2022

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

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