#734: Element.checkVisibility review
Discussions
Discussed
Jun 1, 2022 (See Github)
Rossen leaving note abour explainer.
Peter left comment about not being async.
Comment by @vmpstr Jun 9, 2022 (See Github)
For additional info, we're still discussing a good name for this function. The bulk of the discussion can be found in this issue https://github.com/w3c/csswg-drafts/issues/7317
Comment by @plinss Jun 21, 2022 (See Github)
Has there been any discussion of making this async? In general we'd prefer not to add more functionality that synchronously blocks for style computation (or layout).
Comment by @atanassov Jun 21, 2022 (See Github)
Also, despite the use cases being somewhat obvious, we encourage you to write a complete explainer and capture all the other important bits in a single place - other considerations, code examples, privacy, security and a11y considerations etc. Can you please write one?
Comment by @josepharhar Jun 21, 2022 (See Github)
There is already an explainer and privacy/security self review linked in the issue description. Is there something missing in those?
Comment by @vmpstr Jun 21, 2022 (See Github)
Has there been any discussion of making this async? In general we'd prefer not to add more functionality that synchronously blocks for style computation (or layout).
We haven't considered making this asynchronous. IMHO it would complicate the API and code using it, to avoid a potential style (+ possibly layout) update. If my reading of https://www.w3.org/TR/design-principles/#synchronous is correct, the general preference is for synchronous APIs unless there is substantial I/O or heavy work. Again IMHO, I don't think style and layout qualify here. There is already a number of APIs like getBoundingClientRect that have to process style + layout. In other words, a number of existing APIs already work as if style and layout is always up-to-date from script perspective. I can't see a justification why this API should be different.
Comment by @plinss Jun 21, 2022 (See Github)
The reason I ask is that the general consensus is having synchronous mechanisms that force style computation and layout were a mistake. If we were designing the DOM APIs today those would all be promise based.
Now, one can argue that horse has left the stable, and adding one more synchronous method is relatively harmless, and even better for developers as they wouldn't be faced with a mix of sync/async methods.
On the other hand, many of us would like to see new async methods to access that data, and all the existing synchronous methods and properties become deprecated. Going down that path, adding more and more synchronous methods makes the eventual transition to async more complex.
I don't believe the TAG is going to reject this for being synchronous, but we'd like to see that the option has at least been taken into consideration by the relevant WGs. We'd also like to see some traction on developing async replacements for the various other APIs that force style computation and layout. (Were those to be under active development, it would make more sense for new mechanisms to follow that path and lead authors into the async world.)
Comment by @vmpstr Jul 20, 2022 (See Github)
Sorry for the late reply.
Without considering existing features, I still believe that this function should be synchronous. The intended use-case for this feature is for the developer to check the existing element with current styles for visibility to see if a further measurement is appropriate. Something like the following:
if (e.checkVisibility())
return e.getBoundingClientRect();
else
return null;
The reason this pattern is useful is the way content-visibility: hidden
, for example, works: if one just invokes e.getBoundingClientRect()
and e
is in a subtree of a content-visibility: hidden
element then that will cause an update in an otherwise skipped subtree. The checkVisibility
function, on the other hand, would only update the rendering while still respecting skipped elements, then can check that e
is indeed under a content-visibility: hidden
and return false
Having this function be asynchronous puts a burden on the developer to ensure that the function is either called in an async function with an await, or to restructure the 'natural' flow to account for the fact that the visibility check will happen at a future time. It isn't clear to me whether the guidance to prefer asynchronous APIs to avoid forced rendering updates should take precedence. It is my humble opinion that in this case it should not, and the function call should remain synchronous.
Discussed
Aug 1, 2022 (See Github)
Dan: back with us
Peter: inclined to keep pushing back.. other things being broken is not an excuse to introduce one more broken thing. I'd like to hear from Rossen and Lea.
[next week]
Discussed
Aug 1, 2022 (See Github)
Dan: Another async / sync thing.
Peter: we want Lea's input.
punt to plenary
Discussed
Sep 1, 2022 (See Github)
Peter: sync vs async... Last we talked we wanted to get Rossen's input. I feel this is a design mistake across the platform....
Tess: Intersection observer v2 ... what does this do that intersection observer doesn't? It adds convenience.
Peter: this doesn't take the viewport into consideration
Tess: the default ... they can specify the container...
Peter: their argument against makint it async is that it's inconvenient - and it's only inconvenient because developers are used to sync - which isn't good for performance.
Tess: yes.
Dan: Do we need a finding?
Peter: maybe - it's unfair to push back on individual things... But we also have the principle of "just because something's bad don't keep making things bad in the same manner"... May be worth making a concerted effort to find these tings and raise issues in working groups.
Tess: dbaron and I had a discussion about what it means to flush layout... some issues... in medium term we will not have a definition [in CSS wg] of flushing that is something we can cite. If we want such a list to exist we'll have to do it ourselves.
Tess: Hard to say "don't do this" without a definiton of "this". We have to say what we mean by "flush layout."
Peter: we could create a task force. It's within our scope to make a list of APIs...
Tess: we already have a task force...
Dan: should we push this to Houdini?
Peter: Houdini hasn't been active... One of the chairs needs to do this.
agreed to assign this to Rossen
Discussed
Oct 1, 2022 (See Github)
Lea left comment asking for clarification.
Discussed
Oct 1, 2022 (See Github)
Dan: we've had some comments..
Lea: I'm not following the argument
Peter: the aspect that makes sensne to me is mixing sync and async functionality, some will affect layout and some won't
Lea: if the API is async it will give them stale information?
Peter: I'd think it would give you more up to date inforation
Lea: exactly
Peter: to me it's an argument that all these other APIs should be async
Lea: Perhaps there's a misunderstanding about what an async api would do if they think it will result in stale information
Peter: looks like argument is about raf(?) callbacks so .. you get the result but in the process raf is called and somebody has changed the DOM. I don't know if that would have been called before or after the async stuff would have resolved
Lea: right. Who can we ask? Seems like a low level question. Should we ask an implementer if this is accurate?
Peter: isn't Vlad an implementer who would know? Trying to work out if it really matters.. if you have raf callbacks and you're manipulating the dom.. it just changes the order of execution I guess, the information wouldn't be stale but it will c hange things anyway so what does it matter
Lea: appplies to any async api about layout
Peter: not sure the argument is complelling
Lea: so it's correct?
[scribe missed a bit]
Dan: let's ask Tab
Peter: I don't recall this being discussed recently in the CSS WG
Lea: it was discussed a lot a long time ago, but not so much sync vs async, but more about the name and what visible actually means etc.
Dan: who is vmpstr ?
Rossen: Vlad from Google.
Peter: it's part of a bigger problem that we have a number of sync methods that should have ben async from the get go and we're just adding more. We need a comprehensive look at all of these and move towards making them all async rather than arguing each one
Lea: we do have a design principle for that
Peter: the problem is.. I don't think we're going to get that end result of where's the effort to fix this problem as a whole as opposed to fighting it for each individual new feature. To me that feels like a Houdini problem. What's happening with that?
Rossen: every time I ask folks, they say "we should!" And that's where it ends.
[discussion about how CSS WG works]
Peter: propose opening Houdini issue...
Rossen: There's issue 5115 in css working group... if we have more specific guidance from CSS on this issue then we can apply it here [in #734]. This is how you avoid breaking the loop...
Peter: My concern: giving more APIs that are async ... it's related... we need a definition of what the loop is and what it does. Follow-on task : proposing async replacements...
Rossen: we can open a separate Houdini issue and have discussion there...
Rossen: Tess & I concluded (in review of event loop) that if we have this [5115] issue done and something we can point to then everything else becomes much easier.
Peter: I will file an issue in Houdini and reference these other issues and we go from there.
Comment by @LeaVerou Oct 24, 2022 (See Github)
The reason this pattern is useful is the way
content-visibility: hidden
, for example, works: if one just invokese.getBoundingClientRect()
ande
is in a subtree of acontent-visibility: hidden
element then that will cause an update in an otherwise skipped subtree. ThecheckVisibility
function, on the other hand, would only update the rendering while still respecting skipped elements, then can check thate
is indeed under acontent-visibility: hidden
and returnfalse
We had some trouble following this, how does making the API async complicate this use case?
Having this function be asynchronous puts a burden on the developer to ensure that the function is either called in an async function with an await, or to restructure the 'natural' flow to account for the fact that the visibility check will happen at a future time.
This applies to any async API, doesn’t it?
Comment by @vmpstr Oct 24, 2022 (See Github)
We had some trouble following this, how does making the API async complicate this use case?
Consider something like
function getSize(e) {
if (e.checkVisibility())
return e.getBoundingClientRect();
else
return new DOMRect();
}
function foo() {
/* gather some information about the current DOM state */
...
let size = getSize(e);
...
/* do something with the information and size */
...
}
Here, getSize
function has been updated to use checkVisibility
; it used to (for sake of argument) just return getBoundingClientRect
, which used to cause unnecessary rendering work. This now costs some necessary rendering work but no layout is needed for c-v hidden cases.
This is a correct update that affects one function because of synchronous nature of the API.
Now in order to update this same code but to use an asynchronous function, we have two options: await
and promise.then
. With await, this looks something like the following
async function getSize() {
let is_visible = await e.checkVisibility();
if (is_visible)
return e.getBoundingClientRect();
else
return new DOMRect();
}
async function foo() {
/* gather some information about the current DOM state */
...
let size = await getSize();
...
/* do something with the information and size,
however the information we got about the current DOM state may no longer be correct,
because getSize did asynchronous work which could have resulted in rAF callbacks, for example,
modifying DOM state. */
...
}
Here the information we gathered may be out of date, and we need to further restructure the code, if possible, to still be correct. We also need to mark foo
as async, which propagates further up the stack
The promise.then pattern definitely needs more refactoring here on top of getting new state.
Now, I'm not arguing that any of this is impossible to do. My only concern is that I don't see a compelling reason to force this complexity and async considerations on the developer when the synchronous version of the API seems to have no downsides, other than ensuring style is up to date (which is something that getBoundingClientRect would have done anyway)
This applies to any async API, doesn’t it?
Yes. I was highlighting that (IMHO) it seems unnecessary to add this requirement for this particular API.
FWIW, I am be missing some aspect that the asynchronous version provides, but if my understanding is correct, it seems like the synchronous version is more flexible here
Discussed
Nov 1, 2022 (See Github)
Peter: we opened up the issue in Houdini about sync/async as a bigger deal...
Dan: shall we put this on hold?
Rossen: it's not necessarily CSS only related.. We could spend some time on CSS call this week, some of the right people could be there.
bump to plenary for maybe an update from CSS
Discussed
Dec 1, 2022 (See Github)
Peter: one of thos sync / async / flush style... I don't think we can advance until Houdini work has been done.
Discussed
May 1, 2023 (See Github)
Dan: It is already shopped.
Peter: CSS wg has approved it...
Peter: it's on Element...
Dan: Other implementations?
Peter: from caniuse blink and firefox - so at least 2 implementers...
Rossen: so fairly down the path. What can we add?
Peter: I think it's fodder for sync vs/async discussion...
Rossen: I think we should close this one and have the discussion in the principles issues...
Peter: That's reasonable... I'm fine with closing this one... Not sure if we should do the same with the others yet.
<blockquote> Thanks for that @josepharhar. We discussed again in today's TAG plenary call. Regarding multi-stakeholder support, we also note from [caniuse](https://caniuse.com/?search=checkvisibility) data that there are multiple implementations, which is also not reflected in Chromestatus. Can you please update, or maybe start using the [BCD data](https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Page_structures/Compatibility_tables) which powers CanIUse to also power this indication? We still have some reservations regarding sync vs. async as [plinss highlighted](https://github.com/w3ctag/design-reviews/issues/734#issuecomment-1162190144) however we are going to move that discussion to our Design Principles work. </blockquote>Peter: fine closing it with concerns.
Dan: closed with "satisfied with concerns" label.
Discussed
May 1, 2023 (See Github)
Peter: one of the synchronous APIs we think should be async... Some work to make older APIs async?
Tess: all kinds of issues - opacitiy 5%... etc.. API will lie and say it's visible when humans can't see it. If it's being used for ad impression measurement use case then it's not resiliant to fraud.
Peter: the explainer talks about visibility, opacity, display:none,.. talks about closed shadow tree... talks about 'would it be visible in the viewport'... links to draft spec..
Dan: asks for an update
Comment by @torgo May 22, 2023 (See Github)
@vmpstr we're not clear on the current status of this and the info in Chromestatus is at odds with the info in the request. Also the explainer is in an archived repo. Is there current work still going on and are you still seeking TAG review? If so, can you give us an update and also update the issue as well please?
Comment by @josepharhar May 22, 2023 (See Github)
I updated the chromestatus, we shipped this in chrome 105.
Comment by @torgo May 24, 2023 (See Github)
Thanks for that @josepharhar. We discussed again in today's TAG plenary call. Regarding multi-stakeholder support, we also note from caniuse data that there are multiple implementations, which is also not reflected in Chromestatus. Can you please update, or maybe start using the BCD data which powers CanIUse to also power this indication? We still have some reservations regarding sync vs. async as plinss highlighted however we are going to move that discussion to our Design Principles work.
OpenedApr 27, 2022
Hello,
I'm requesting a TAG review of Element.isVisible.
Element.isVisible() returns true if the element is visible, and false if it is not. It checks a variety of factors that would make an element invisible, including visibility, content-visibility, and opacity.
Further details:
💬 leave review feedback as a comment in this issue and @-notify @vmpstr