[cssom-view] checkVisibility options have inconsistent naming schemes

[nt1m]

We have checkOpacity and checkVisibilityCSS. Can we add a CSS suffix to checkOpacity ? Or rename to: checkOpacityProperty / checkVisibilityProperty

cc @josepharhar @vmpstr @smfr

[rniwa]

Why do we even need to say "check" in each one of these options anyway? The method is called checkVisibility. We don't need to repeat "check" in every option.

[nt1m]

cc @emilio / @smfr as spec editors

[smfr]

The term "check" is also problematic. It's a "compute" or "fetch" not a "check". ("Check" can also be interpreted to mean "constrain" or "slow down").

[nt1m]

So computeOpacityProperty / computeVisibilityProperty / computeContentVisibilityAuto ? or opacityProperty / visibilityProperty / contentVisibilityAuto ?

[fantasai]

I'm not sure that "compute" is adding anything useful here, so I'd be in favor of the "opacityProperty / visibilityProperty / contentVisibilityAuto" pattern, since that seems to establish a consistent pattern that we can continue.

It does seem weird to name CSS properties by strings in camelCase though.

[smfr]

See also #7317

[smfr]

how about window.getComputedVisibility() to parallel getComputedStyle()?

[vmpstr]

For posterity, the dictionary property names were (somewhat briefly) discussed here #6850 (comment)

[css-meeting-bot]

The CSS Working Group just discussed [cssom-view] checkVisibility options have inconsistent naming schemes, and agreed to the following:

• RESOLVED: don’t rename but define pattern for arguments and add aliases

The full IRC log of that discussion

<bramus> Rossen_: who can take this?
<bramus> fantasai: is ntim here?
<bramus> fantasai: 3 open Qs
<bramus> … what are we gonna call the checkContentVisibility option?
<bramus> … name for various options pretty inconsistent
<bramus> … last one was brought up by smfr
<vmpstr> q+
<fantasai> https://github.com//issues/9487#issuecomment-1769013122
<bramus> … maybe method on window, mirroring getComputedStyle
<TabAtkins> q+
<bramus> … tim’s suggestion was to go with “…VisibilityProperty”
<bramus> … and “…VisibilityAuto“
<Rossen_> ack vmpstr
<emilio> q+
<bramus> vmpstr: this is shipping both chrome and firefox, so bar to change should be a bit higher
<bramus> … for the names, we have quite a long discussion with various people and we ended with checkVisibility which isnt the greatest name
<bramus> TabAtkins: the fn name and location cant be changed at this point
<Rossen_> ack TabAtkins
<bramus> … its been stable for a while now
<bramus> … could alias the option names
<bramus> … for consistenty, but cant remove existing ones
<Rossen_> ack emilio
<bramus> emilio: we could add usecounters to track usage
<bramus> … changing from Element.checkVisibility to something else feels like a bit too much
<bramus> … separate function could query psuedoEls
<bramus> … bu we can add that option to E.checkVisibility
<bramus> … unless someone objects, we should resolve on not changing
<bramus> … we are not restricted to booleans
<bramus> … could pass in “auto” as a string
<Rossen_> q?
<bramus> fantasai: would be good to set up some consistent way to deal with the option
<bramus> … we could do passing in the name of the prop and the interested value
<bramus> … is a bit weird to have camelCase names of props and values
<bramus> … we could not have that if we wanted, e.g. 'content-visibility'
<bramus> emilio: web animations does that a lot
<bramus> … could take a look at that
<fantasai> s/values/values in strings/
<bramus> flackr: confirming
<bramus> emilio: only unfortunate thing is visiblity being in the function name and also property name
<bramus> … we could probably say that as long as you opt in to one of both … define what happens when you specifiy both
<TabAtkins> yeah, just ORing the aliases together seems reasonable imo
<vmpstr> q+
<flackr> web-animations-1 spec refers to https://drafts.csswg.org/cssom/#css-property-to-idl-attribute to convert the names
<Rossen_> ack vmpstr
<TabAtkins> I propose we take this back to the issue to figure out a proposal for a new set of values, and we can discuss it there. We should reject renaming/moving the function itself.
<bramus> vmpstr: if we add more props and alias existing ones, then is the plan to deprecate the existing props and unship? or do we ship both sets of props so devs can mix and match?
<bramus> emilio: we could try to unship, but its not a lot of effort to check two bools instead of one
<TabAtkins> q+
<bramus> TabAtkins: my proposal is that we reject the rename and return to the issue
<bramus> … and see if we can come up with a consistent set of readable names
<Rossen_> ack TabAtkins
<bramus> … to then look at, and adopt as aliases
<fantasai> fooProperty / fooPropertyValue
<bramus> fantasai: one proposal in the issue is to follow fooProperty / fooPropertyValue system
<bramus> Rossen_: going back to TabAtkins’ proposal, is there sth to resolve on?
<emilio> fooProperty / fooPropertyValue seems ok to me fwiw
<bramus> … we can do it next week when Tim et al are here
<bramus> fantasai: I suspect its OK to resolve, they can reopen
<bramus> Rossen_: SGTM
<bramus> PROPOSED RESOLUTION: reject rename
<bramus> fantasai: don’t rename but define pattern for ?? arguments and add aliases
<fantasai> s/aliases/aliases that combine with OR/
<bramus> TabAtkins: when we come up with reasonable set of names, then we can talk about it … and alias them if we add new names
<bramus> Rossen_: Objections?
<fantasai> s/rename/rename method/
<bramus> PROPOSED RESOLUTION: don’t rename but define pattern for arguments and add aliases
<bramus> RESOLVED: don’t rename but define pattern for arguments and add aliases
<bramus> Rossen_: tim can always reopen
<bramus> fantasai: comments on the proposed pattern in the issue?
<bramus> … cause there is a pattern in the issue
<bramus> TabAtkins: havent thought about it yet. will look at it in the issue

[fantasai]

Current proposal from @nt1m btw is:

• fooProperty for CSS foo property checks
• fooBar for CSS foo property checking value Bar

For the current values that translates to opacityProperty, visibilityProperty, and contentVisibilityAuto.

Note: Earlier proposals also included values for checking aria-hidden and for inert.

[emilio]

Those are not CSS properties. Presumably those would just be ariaHidden and inert?

[nt1m]

Those are not CSS properties. Presumably those would just be ariaHidden and inert?

Yeah that would make sense. Although I prefer to not add those 2 options in this function, or at least in their current form, I'd prefer something more high level for accessibility.

[emilio]

Yeah agreed. Those are orthogonal to this.

[vmpstr]

To write the full syntax for posterity,

el.checkVisibility({ contentVisibilityAuto: true })

Would mean return true iff el has a box, is not content-visibility: hidden, and also not skipped due to content-visibility: auto

(bikeshed) I feel like "check" prefix in dictionary fields played a role here that made this a little clearer

[vmpstr]

FWIW, I'm good with adding the names proposed in #9487 (comment) as aliases

[css-meeting-bot]

The CSS Working Group just discussed [cssom-view] checkVisibility options have inconsistent naming schemes, and agreed to the following:

• RESOLVED: Adopt the naming scheme for future values and as aliases for existing values

The full IRC log of that discussion

<dael> fantasai: we're pretty inconsistent where we have checkOpacity and css has checkVisibility. Proposal is to set up naming scheme where for CSS
<dael> fantasai: fooProperty for CSS foo property checks. fooBar for CSS foo property checking value Bar. For the current values that translates to opacityProperty, visibilityProperty, and contentVisibilityAuto
<dael> fantasai: Comments on issue that check makes it easier to understand, but could lead to checkcheck for things like checkVisibility
<dael> fantasai: Need some consistency so this is proposal
<dael> vmpstr: This is in addition to existing properties as aliases?
<dael> fantasai: Yeah. Others are shipping so we need to keep them
<dael> astearns: Anything new we add will follow this scheme
<dael> astearns: Seeing agreement in issue. Hearing no concerns
<TabAtkins> +1 to the proposal
<dael> astearns: Prop: Adopt the naming scheme for future values and as aliases for existing values
<dael> astearns: Obj?
<dael> RESOLVED: Adopt the naming scheme for future values and as aliases for existing values