[cssom-view] Rename Element.isVisible to Element.isHidden?

[chrishtr]

In Issue #6850 we decided to add isVisible. Should we rename it to isHidden and flip the return value?

The only difference I can see is that it flips what is considered a "positive" result, and therefore maybe developers would be less likely to misinterpret the results. i.e. developers might interpret isVisible as a stronger assertion from the browser than "not isHidden". Since isVisible does not actually mean "guaranteed to be visible right now to the user", maybe isHidden is better?

[yisibl]

How about isCSSHidden?

[css-meeting-bot]

The CSS Working Group just discussed [cssom-view] Rename Element.isVisible to Element.isHidden?, and agreed to the following:

• RESOLVED: Rename isVisible to isHidden and flipping default value

The full IRC log of that discussion

<dael> Topic: [cssom-view] Rename Element.isVisible to Element.isHidden?
<dael> github: https://github.com//issues/7317
<dael> joeyarhar: prop is to rename :isvisible to :isHidden
<fantasai> s/:isvisible/.isVisible/
<dael> joeyarhar: Got some feedback that isVisible is more about if it's painted but this is about DOM and style so we thought isHidden would make more sense
<dael> joeyarhar: That's pretty much it
<TabAtkins> I'm fine with the name either way, both seem reasonable to me.
<fantasai> s/:isHidden/.isHidden/
<dael> joeyarhar: The return value would be flipped since visible and hidden are opposite
<dael> Rossen_: Default for hidden is false?
<dael> joeyarhar: Yes, isHidden would return false b/c is not hidden
<dael> Rossen_: Seems straightforward proposal
<dael> Rossen_: Since we settled on isVisible previously, isHidden seems best choice. There is one proposal in the issue about naming isCssHidden but that goes against some of our naming schema
<dael> Rossen_: Additional thoughts? Suggestions?
<Rossen_> q
<dael> Rossen_: Obj to Rename isVisible to isHidden and flipping default value
<dael> RESOLVED: Rename isVisible to isHidden and flipping default value
<dael> fantasai: Should we ask authors for feedback?
<dael> Rossen_: We can. In what way?
<TabAtkins> Yeah, not that we should block the change one way or the other, just might be useful to vibe-check the direction of the name
<dael> fantasai: Ask authors in group or posters on twitter. We got no feedback from WG so seems like no opinion
<dael> vmpstr: Also worth pointing out isVisible is not used so fine to change
<dael> fantasai: Fine to change
<astearns> prior art (from linked issue) for 'visible' https://github.com//issues/6850#issuecomment-1011388347
<dael> chris: I think it's a better name. If something is hidden tells you truer information. If not hidden it depends on how big page is. Doesn't make incorrect promise.
<dael> Rossen_: Agree stronger promise
<fantasai> s/Fine to change/Fine to change, just nobody here seems to have expressed an opinion/
<dholbert> +1 to what Chris said
<dael> Rossen_: To fantasai point if we want someone to run a survey that would be great
<dael> jensimmons: I posted a tweet
<jensimmons> I posted this tweet: https://twitter.com/jensimmons/status/1532137408418004994
<dael> jensimmons: A bit late to hear from my usual audience but maybe over next day or so. We can decide now and reverse later
<chrishtr> I recommend making a decision now and we can update it if we hear other evidence
<dael> Rossen_: There's a well articulated reason to change. If we hear anything to the opposite we can reverse.
<fantasai> wfm
<dael> Rossen_: Thank you

[ziyunfei]

How about isCSSHidden?

inert isn't about CSS, and an off-document element also not hidden by CSS.

[manuelmeister]

What does isHidden do? I'm not sure what this would return on sr-only elements.

[tabatkins]

Twitter polls are weak evidence, but my poll seemed to record a pretty strong (3:1) preference for isVisible(), with 29 votes.

[vmpstr]

To me, this isn't just about the name but what information the function is providing.

isVisible: We check a fixed, not necessarily comprehensive, set of conditions (opacity is 0, visibility is not visible, etc) and if none of those conditions are true, then we seemingly jump to the conclusion that the element must be visible -> return true
isHidden: We check the same fixed set of conditions and if none of those conditions are true, we say that as far as we know this element is not hidden, but we're not promising that it's visible -> return false

Am I reading too much into the name?

[vmpstr]

Maybe a better options is having a neutral name, something like checkVisibilityState with possibly a non-optional dictionary which would return a string/enum indicating the result of requested checks. This is obviously less ergonomic, but maybe less confusing

[tabatkins]

This function is not, strictly, telling you whether the element is hidden or visible, since it also cares about the inert state (and previously, and perhaps in the future, about other things beyond visibility).

But yes, I think you're reading too much into it. ^_^ Neither "isVisible" nor "isHidden" are more or less authoritative-sounding than the other; the fact that in either case we have to mean "as far as we can tell, using some relatively limited tests" is equally apparent (or not) in either.

I don't think a less ergonomic version is any better, especially if it continues to just return two values. That's just a less convenient version of a bool, for no benefit afaict. And I don't think returning an object with bools for each possible test is worthwhile; that just means authors now have to write code to coalesce those results into a single boolean. It doesn't help them at all, since if they wanted individual test results they can ask for them via the options argument already, potentially via separate calls if they want to check multiple things. All this does is make the common case more annoying.

[Loirooriol]

Some comments mention inert, but #7274 resolved to remove that.

[heycam]

element.isHidden() is pretty similar to element.hidden, and these are different things. I think there is a chance of confusion between the two.

[vmpstr]

There is also IntersectionObserver v2 which has isVisible which disallows false positives but allows false negatives. The proposed isVisible here would compute a different value, allow false positives and disallow false negatives

Admittedly, it's not on the element interface so maybe that's ok

[css-meeting-bot]

The CSS Working Group just discussed [cssom-view] Rename Element.isVisible to Element.isHidden?.

The full IRC log of that discussion

<fantasai> Topic: [cssom-view] Rename Element.isVisible to Element.isHidden?
<fantasai> github: https://github.com//issues/7317
<fantasai> chrishtr: Bringing up naming of isVisible/isHidden
<fantasai> chrishtr: Last week we resolved to change to isHidden
<fantasai> chrishtr: but also spun up some Twitter polls
<fantasai> chrishtr: Tab's poll indicated isVisible got more votes
<fantasai> chrishtr: Jen's poll seemed inconclusive?
<jarhar> https://twitter.com/jensimmons/status/1532137408418004994
<vmpstr> https://twitter.com/tabatkins/status/1532138513704943617
<fantasai> chrishtr: Also original reasons for isVisible
<fantasai> chrishtr: basically, doesn't guarantee it's visible
<fantasai> chrishtr: There's some conflicts with isVisible idea from IntersectionObserver
<fantasai> chrishtr: And conflicts with element.hidden
<fantasai> chrishtr: So that's the rundown
<fantasai> chrishtr: There's data both ways, so maybe leave the way it is?
<emilio> q+
<emilio> fantasai: there's also a comment suggesting checkVisibility
<emilio> ... maybe that would be possible?
<emilio> fantasai: I think the mixups between isVisible/isHidden in the twitter poll make me think we need to do a bit better
<fantasai> fantasai: and in the issue
<Rossen_> ack fantasai
<Rossen_> ack emilio
<fantasai> emilio: Was going to say something along the same lines
<fantasai> emilio: make it less confusing
<fantasai> emilio: I'm not sure I'm satisfied with either isVisible or isHidden
<fantasai> emilio: checkVisibility sounds reasonable
<chrishtr> checkVisibility is fine with me too
<smfr> not a fan of “check”
<fantasai> fantasai: checkVisibility with args and defaulting to a set of argus would maybe be less confusing
<fantasai> fantasai: you're checking for these specific things, not for total visibility
<emilio> computeVisibility?
<fantasai> Rossen_: Maybe investigate longer? Is there any pressure for this?
<fantasai> chrishtr: We'd like to implement in Chromium, so hoping we can come to an agreement
<fantasai> Rossen_: We have a name for now
<fantasai> Rossen_: I would say let's continue working in the thread, bikeshedding on the call not so helpful unless something comes up that's obvious
<fantasai> Rossen_: and don't have anything better atm
<fantasai> Rossen_: sounds like you're going to start writing code, changing name should be fairly straightforward
<fantasai> chrishtr: Have code written, want to ship it
<Rossen_> q?
<fantasai> chrishtr: Don't want to ship something the group disagrees with
<fantasai> fantasai: It's not so much about group disagreeing, it's about getting something we're confident that authors will find usable
<fantasai> fantasai: and the current information we have indicates that we maybe don't have that yet
<fantasai> fantasai: so I think we should work on it a bit more, so that when we ship, it's something authors find natural and easy to use
<fantasai> Rossen_: Should we continue to discuss here or take it back to the issue?
<emilio> Rossen_: is there anything we can do here right now or should we continue discussing in the issue?
<fantasai> Rossen_: Suggest engaging on the issue
<fantasai> Rossen_: Let's collect ideas and organize them and take the winner

[vmpstr]

To summarize, the current options that we’ve seen are the following:

• isHidden - current name
  • May be confused with Element.hidden attribute
  • Tab’s twitter poll indicates that isVisible is more preferred
  • Semantically, it would have no false positives but may have false negatives
• isVisible
  • May be confused with IntersectionObserver’s isVisible
  • Preferred in Tab’s twitter poll
  • Semantically, it would have false positives, but no false negatives
• checkVisibility
  • Is not biased towards “is visible” or “is hidden”
• computeVisibility, checkVisibilityState
  • Same arguments as checkVisibility, just different name

For posterity, Jen’s twitter poll seems to be mixed with roughly similar support for both isHidden and isVisible.

Are there other suggestions?

[foolip]

@vmpstr for isVisible, I think your observation in #7317 (comment) is also relevant. That's an attribute on another interface, but has the opposite behavior in terms of false positives/negatives.

[vmpstr]

Since there hasn't been much engagement here so far, maybe we can emoji-vote on the existing proposed names? And please comment with other suggestions if you can think of something better

👍 - isHidden
😄 - isVisible
🎉 - checkVisibility
❤️ - computeVisibility
🚀 - checkVisibilityState

[bkardell]

Sorry I know this has been discussed a ton and I have said very little but it's because I haven't felt good about any of them and didn't really have any other ideas -- but... is there a possibility of using some synonym that is not so overloaded as visible/hidden? Like perceivable or revealed (or concealed if we go in the opposite direction) or something? I don't want to hang this up but the truth is I also feel like all of these are degrees of bad rather than something I feel great about.

[vmpstr]

I empathize with this. I haven't been able to find something that is better though. The particular set of properties we'd like to check are indeed related to visibility which is why this seems to be sticking around. I find that an earlier proposal for CSSVisibility (like isCSSHidden or something) is a better fit here, but I understand there is precedent for disallowing/discouraging using CSS in such names.

[chrishtr]

checkVisibility has 8 votes and in the clear lead. I propose we resolve with that name and close the issue.

[css-meeting-bot]

The CSS Working Group just discussed element.isvisible, and agreed to the following:

• RESOLVED: Rename to checkVisibility()

The full IRC log of that discussion

<TabAtkins> Topic: element.isvisible
<TabAtkins> github: https://github.com//issues/7317
<TabAtkins> chrishtr: I think w'ere ready for the name. We had a vote up for the last week
<bkardell_> q+
<TabAtkins> chrishtr: 9 votes for checkVisibility() and only a few on anythign else, suggest we resolve on that
<TabAtkins> astearns: Small number of votes but it was the clear winner
<astearns> ack bkardell_
<TabAtkins> bkardell_: Don't want to block, but all feel like less-bad rather than good
<TabAtkins> bkardell_: I gave some ideas in the thread but it doesn't seem to have gone anywhere
<TabAtkins> bkardell_: Seems worth to take at least one more beat to be sure we consider that
<TabAtkins> astearns: That said, we've gone over this several times and solicited better names several times
<TabAtkins> astearns: Your suggestions in particular I'm not fond of due to spelling
<TabAtkins> astearns: Would think we'd've found a better term in the previous discussions if one existed
<bramus> q+
<astearns> ack bramus
<TabAtkins> bramus: If checkVisibility() is th ename, what does it return?
<TabAtkins> chrishtr: Still true/false, true if visible
<TabAtkins> fantasai: Agree with bkardell that this name isn't amazing, but think it's the best we have, and it doesn't have quite as much confusing downside as isVisible/Hidden. Fine with going for it since we don't have a better option and it's not too overloaded of a term.
<TabAtkins> astearns: Suppose we could resolve this as "best of everything we have so far", leaving it open for new suggestions but closing it to all the ones we've considered.
<TabAtkins> chrishtr: I think we can just resolve.
<TabAtkins> +1 to just resolving, fine with this name
<TabAtkins> florian: Also not huge fan of the name but found others problematic.
<TabAtkins> florian: This one might not be lovely but it doesn't cause problems
<TabAtkins> astearns: brian did you want to block?
<TabAtkins> bkardell_: Do not, it just didn't get much discussion.
<TabAtkins> bkardell_: Like you said, we need to make progress. If people think we won't do better, I support that.
<TabAtkins> astearns: So proposed resolution is checkVisibility()
<TabAtkins> RESOLVED: Rename to checkVisibility()

[alystair]

Where do I go to see actual spec discussion? I accidentally came across the checkVisibility function whilst playing around in Chrome Canary and noticed (what I feel is) a false positive.

When an element is styled with

pointer-events: none;
visibility: hidden;
content-visibility: hidden;

and its width/height are 0px, it seems nonsensical to me that the function returns true. I'd think 0px would signify no visibility?

[josepharhar]

Where do I go to see actual spec discussion?

#6850 (comment)
#7317 (comment)
#7317 (comment)

I'd think 0px would signify no visibility?

I'm not sure if 0x0 sized elements were ever discussed in the spec, but yes that is considered visible by checkVisibility because checkVisibility does not look at the actual size of the element.

content-visibility: hidden only makes child elements hidden, so if you call checkVisibility directly on an element with content-visibility: hidden it will be considered visible.

visibility: hidden is only checked for if you explicitly ask like this: element.checkVisibility({checkVisibilityCSS: true})

[tabatkins]

Where do I go to see actual spec discussion?

You're looking at it!

it seems nonsensical to me that the function returns true. I'd think 0px would signify no visibility?

A 0x0 element can still absolutely be visible, for example if it has outline or box-shadow.

Now, your specific example includes visibility:hidden, which would make the element invisible (suppressing the effect of outline/etc), and this can be activated with the .checkVisibility({checkVisibilityCSS:true}) option. We don't include this by default because the most common type of "invisibility" is actually avoiding generating any boxes (such as by display:none), and whether or not you want to include other types (like opacity or visibility) isn't something we can reliably infer one way or the other, so instead we give you the option.

[alystair]

Thanks for the clarification! My source of confusion likely stemmed from not spotting the options list. I guess discoverability via the DevTools console can't be underestimated :^)

[smfr]

How does checkVisibility relate to Intersection Observer's trackVisibility? Ideally they would use identical underlying concepts so an implementation could share code.