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-3] Proposal: Add an event to fire when content-visibility: auto state changes #7384

Closed
vmpstr opened this issue Jun 17, 2022 · 20 comments

Comments

@vmpstr
Copy link
Member

vmpstr commented Jun 17, 2022

Right now we specify that content-visibility: auto skips its content when the element isn't relevant to the user. This means that the contents of the element will not be rendered by the UA. This is also a great opportunity for frameworks (e.g React) or general script (e.g. canvas updates) to stop, since the effect of these updates would not be rendered anyway.

However, to coordinate the starting/stopping updates, the UA should give the developer an signal (this proposed event) which would inform script that the rendering state changed.

The name is hard to come up with but I have a couple of proposals in the explainer:
https://github.com/vmpstr/web-proposals/blob/main/explainers/cv-auto-event.md

Happy to hear other suggestions or thoughts about this in general.

@tabatkins
Copy link
Member

I'm in favor of just being big and verbose - this is something very specific and I don't think we should try to claim a generic name for it. So +1 for ContentVisibilityAutoStateChanged

@vmpstr vmpstr added the Agenda+ label Jun 28, 2022
@smfr
Copy link
Contributor

smfr commented Jul 5, 2022

Can you describe the exact timing of when this event is fired? We don't want it to reveal exactly when the UA does style updates or layout, so I would prefer that it fire during the "update the rendering steps" somewhere.

@FremyCompany
Copy link
Contributor

  1. Are we sure this is different from an IntersectionObserver?
  2. How about ContentVisibilityStateChanged? This might apply to values other than auto in the future.

@chrishtr
Copy link
Contributor

chrishtr commented Jul 6, 2022

Can you describe the exact timing of when this event is fired? We don't want it to reveal exactly when the UA does style updates or layout, so I would prefer that it fire during the "update the rendering steps" somewhere.

It should fire during the same timing as IntersectionObserver callbacks, i.e. in a posted event loop task after the observer fires. (content-visibility:auto is already defined in terms of IntersectionObserver timings.)

Therefore the feature would not expose the exact timing of style or layout.

  • Are we sure this is different from an IntersectionObserver?

It's not different. But this event allows a web app to subscribe to the implicit UA-internal IntersectionObserver without having to (a) add more javascript that duplicates it and slows down the page, and (b) reverse-engineer the (UA-dependent) margin chosen by the UA.

  • How about ContentVisibilityStateChanged? This might apply to values other than auto in the future.

This name SGTM.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-contain-3] Proposal: Add an event to fire when content-visibility: auto state changes, and agreed to the following:

  • RESOLVED: Add the new event with the name ContentVisibilityAutoStateChanged
  • RESOLVED: Add a note about potential issues for a11y
The full IRC log of that discussion <dael> Topic: [css-contain-3] Proposal: Add an event to fire when content-visibility: auto state changes
<dael> github: https://github.com//issues/7384
<dael> vmpstr: This is a prop for a new event for content-visibility: auto
<dael> vmpstr: content-visibility: auto right now skips rendering when element is offscreen or not relevant to use
<dael> vmpstr: Prop is expose and event that fires when this state changes
<dael> vmpstr: Motivation is some of the partner feedback. In particular they want to skip additional work when element is offscreen such as react update. Event is convenient way to do so
<dael> Rossen_: Catching up, did we establish event name?
<dael> vmpstr: We can talk about name. I have proposals and TabAtkins commented more explicit and verbose is better which I'm fine with
<dael> Rossen_: Backing up, can you remind me for the content-visibility: auto elements nested inside do they participate in a11y tree?
<dael> vmpstr: content-visibility: auto elements do participate in a11y. They're exposed as visible.
<dael> Rossen_: From PoV of a11y these events and content-visibility: auto has no meaningful meaning?
<chrishtr> whether the content is skipped affects layout
<dael> vmpstr: The event itself doesn't. The only potential difference is if script does something in response tot he event such as skipping react updates that eventually cause a different a11y tree.
<dael> Rossen_: Thanks. That's what I wanted to decipher here. If, as an author, I'm pausing a bunch of work on the event and I'm aware what impact pausing would have on a11y that could degrade the experience of assistive tech
<Rossen_> q?
<chrishtr> q+
<heycam> q+
<dael> vmpstr: It is a possibility. Currently the solution is to add own intersection observer and do the same thing but impossible to guess the margin content-visibility: auto has internally. but this could be done with intersection observer
<dael> chrishtr: Clarifying your point, Rossen_. I think you were saying exposing the event makes it easier for author to remove offscreen content from DOM?
<dael> Rossen_: That would be one more kind of aggressive change, yes
<dael> chrishtr: Yeah. Then I would repeat that it is already possible and virtual scrollers do it. Main value is it allow them to play with perf benefits. But if a site unmounts DOM that's far offscreen it would be less info for a11y tech
<Rossen_> ack chrishtr
<Rossen_> ack heycam
<dael> heycam: By the sounds of it this can be achieved using intersection observer already by watching for the same changes, maybe with some additional checks for focus. Wondering about firing of events and which lements fired to. One difference with intersection observer is you register on an element and sounds like this event should be dispatched to every element
<dael> heycam: Could be like the focus state of an element changes, would that mean have to dispatch a separate event to each ancestor?
<dael> vmpstr: I think I would like to dispatch this event to an element that has content-visibility: auto style on it when relevance of that element changes. Spec is element is relevent if in flat tree has an element that has focus. It's if the content-visibility: auto element with the element in the subtree has focus
<dael> heycam: I see. Okay
<Rossen_> q?
<dael> Rossen_: Additional feedback?
<dael> Rossen_: I guess you're looking for resolution to add this event, with name and shape tbd?
<dael> vmpstr: If no obj to content-visibility-auto-state-change I'd like to resolve on that. But also happy to resolve to add event
<dael> Rossen_: If no other feedback or additional comments to vmpstr then are there any objections to exposing such event?
<dael> Rossen_: Objections to ContentVisibilityAutoStateChanged as the name
<dael> heycam: Is there a way to query the content-visibility auto state?
<dael> vmpstr: No so the event would have that information on it
<dael> Rossen_: Hearing no objection on event or name
<dael> RESOLVED: Add the new event with the name ContentVisibilityAutoStateChanged
<dael> Rossen_: Still a bit concerned about if there is a way to add author guidance about potential degredation of a11y experience. I don't think that's the event itself, but the event might make it more confortable
<dael> vmpstr: Can add a note to the spec next to the event
<dael> Rossen_: Would love that
<dael> Rossen_: Thanks
<dael> RESOLVED: Add a note about potential issues for a11y

@chrishtr
Copy link
Contributor

chrishtr commented Jul 6, 2022

As discussed on the call, the event will need a parameter indicating whether the element is now in a skipped or not-skipped state. I suggest we call the event parameter skipped, and its type a boolean.

@noahlemen
Copy link

This is definitely crucial for being able to leverage content-visibility:auto to replace userland implementions of virtual scrollers.

For example, in our JS-based virtual scroller on Facebook, we use intersection observers to determine visibility – we then use this to control both rendering of the elements, as well as to flag these React components for deprioritized updates.

When attempting to use content-visibility:auto to replace this implementation, we currently lose out on the latter. We certainly could continue to use an intersection observer for this aspect of the functionality, but I think that is undesirable for the reasons @chrishtr mentioned.

@emilio
Copy link
Collaborator

emilio commented Jul 13, 2022

@vmpstr there's other things that affect c-v: auto state, like focus and selection. Does the event still fire at IntersectionObserver time even when e.g., a descendant gets focus?

@bgrins
Copy link
Member

bgrins commented Jul 13, 2022

@noahlemen I'm curious if the additional "relevant to the user" states are desirable for your virtual scroller use case - specifically focused and selected elements. Or do you really only want elements in/around the viewport, and would do work to filter out elements that are focused/selected and not in viewport?

@noahlemen
Copy link

that's a good question! for the specific virtual scroller i have in mind currently, we are just interested in the "on-screen" case, at least in regards to maintaining parity with the JS-based virtual scroller. i don't think it would be problematic to begin included focused/selected elements, but its not something we currently do as far as i know.

to be clear, would this ContentVisibilityAutoStateChanged event respond to changes to an elements "relevant to the user" state based on any criteria, not just the "on-screen" condition?

@bgrins
Copy link
Member

bgrins commented Jul 13, 2022

to be clear, would this ContentVisibilityAutoStateChanged event respond to changes to an elements "relevant to the user" state based on any criteria, not just the "on-screen" condition?

Yes, I believe so based on the summary in the explainer

This proposal is to add an event that would fire on a content-visibility: auto element when the rendering state of the element changes due to any of the attributes that make the element relevant to the user.

Though further down it talks about polyfilling without including selectionchange and focus events which I think would be necessary to have feature parity, so it'd be good to confirm with @vmpstr:

Note that even without this event, it is still possible to polyfill the behavior with a combination of IntersectionObserver and MutationObserver. From this perspective, the event does not add a capability to observe new behaviors, but rather an easier way to discover events that are should already be discoverable.

@bgrins
Copy link
Member

bgrins commented Jul 13, 2022

I don't have any particular insights about this, but I do wonder if there's a set of use cases for this to replace Intersection Observer on paper but then in practice it would either not solve the use case, or would require additional hoops due to:

  1. wanting to customize threshold options in a way that don't match UA default (is there good data for how frequently these are customized in the wild?)
  2. not wanting focused-or-selected-out-of-viewport elements included

@vmpstr
Copy link
Member Author

vmpstr commented Jul 13, 2022

The intent of the event is to respond to any changes that take the element from being relevant to not being relevant (and vice versa). The polyfill would need to consider selectionchange and possibly other events as well, which I realize I haven't mentioned in the explainer.

I'm curious to hear whether the proximity to the viewport is the interesting thing to observe, or it's strictly required that we don't include things like selection (and other relevant-to-the-user aspects) here. As an example, if there is an off-screen selection, then I can see an argument for why one would want that content to be updated since it can always make it into the copy buffer via ctrl/cmd-c or something similar. Similarly some focused elements (text input) support things like ctrl-a ctrl-c to select the text in there regardless of whether or not the element is on-screen.

@chrishtr
Copy link
Contributor

I think all of the use cases that make the element relevant to the user should fire the event. This makes sense, since these are also all of the cases where libraries like React need to know so that they should update the DOM to match the latest state of the application. (Otherwise the user will see a stale state.)

@bgrins
Copy link
Member

bgrins commented Jul 13, 2022

Agreed that the event should map to the content-visibility semantics of relevance, and I can see where it would be beneficial for i.e. focused offscreen elements to be included. Though I don't know if that's what developers actually want for certain use cases like virtual scrollers.

For example if a virtual scrolled element is big enough for 10 rows, and a user focuses a text field in a row 1, then scrolls down to row 20 - would that mean that the DOM now has row 1 followed by rows 20-29? If so, is that desireable? I don't know enough about how these are implemented to know.

@chrishtr
Copy link
Contributor

chrishtr commented Jul 13, 2022

For example if a virtual scrolled element is big enough for 10 rows, and a user focuses a text field in a row 1, then scrolls down to row 20 - would that mean that the DOM now has row 1 followed by rows 20-29? If so, is that desireable?

I think it's desirable, because if it was done by the user, they indicated via focus that they care about the thing being focused. However, if the virtual list was virtualized in chunks, other chunks between the focused one and the near-screen would not be relevant to the user, and so could be deprioritized by the virtual list script-side implementation (as well as the browser).

Keeping focused elements, even if far off-screen, up-to-date is also important for accessibility APIs.

Virtual scroller implementations can also if they so desire unfocus elements by checking document.activeElement and calling blur on them. This could make sense if the element is far off-screen and in the web page UI it doesn't make sense to focus it.

@bgrins
Copy link
Member

bgrins commented Jul 14, 2022

I had a question about the second use case in the explainer:

Similarly, the developer may want to stop any other script updates (e.g. canvas updates) when the user-agent is not rendering the element.

I may be missing something, but isn't that better served by using an IntersectionObserver? In the sense that you presumably don't want to continue making visual-only updates for elements that are offscreen but focused or highlighted. Which is the behavior with IntersectionObserver already - and in order to achieve this with the new event you'd need to track those states and exempt those elements.

@chrishtr
Copy link
Contributor

I may be missing something, but isn't that better served by using an IntersectionObserver? In the sense that you presumably don't want to continue making visual-only updates for elements that are offscreen but focused or highlighted. Which is the behavior with IntersectionObserver already - and in order to achieve this with the new event you'd need to track those states and exempt those elements.

A focused or highlighted element that is offscreen is much more likely to be brought onscreen sometime soon, and therefore it's a good idea for the developer to keep its canvases up to date.

@jcsteh
Copy link

jcsteh commented Jul 20, 2022

I have some concerns regarding accessibility here.

I understand that this event doesn't facilitate anything that isn't already possible with other functionality (e.g. IntersectionObserver and MutationObserver) and that isn't already being done (e.g. by virtual scrollers). However, there's a difference between "authors can already do this" vs "the spec includes something which is intended to be used in a certain way that will fundamentally cause this problem". In the former case, it's reasonable to argue that if an author is doing something a little obscure, they must understand the consequences across the board. In the latter case, the web platform is providing a feature which authors can reasonably expect to "just work".

The primary use case for this feature - stopping DOM updates to a subtree that is not rendered (as outlined in the explainer) - will cause pretty severe problems for a11y. It's not a technical problem with the event itself, but it is a problem with the way that it is intended to be used. The decision was made to expose content-visibility: auto in the a11y tree on the basis that the content was still semantically relevant; it just wasn't visually relevant right now. The intent behind this event fundamentally changes that logic.

I don't think adding a note is sufficient to resolve this. If the primary use case causes problems for a11y, either that use case needs to be normatively forbidden by the spec or the technique needs to be fixed somehow to avoid this problem.

There are likely use cases that don't cause problems for a11y; e.g. stopping computation that wouldn't affect the DOM, stopping canvas updates, etc. Those use cases are fine. While i realise there is often a preference to avoid codifying specific use cases into specs, I think this is potentially warranted when primary use cases might cause significant harm.

@vmpstr
Copy link
Member Author

vmpstr commented Jul 28, 2022

Hey, thanks for your comments and for taking the time to discuss this outside of this thread.

The conclusion we seemed to have reached is that we can proceed with speccing this event, but we should add normative developer guidance in the spec (and mdn) to avoid cases that break the semantic meaning of elements affected by c-v auto. IOW, since currently the content is semantically relevant regardless of whether it's skipped by c-v auto, this event should not be used to break that (e.g. by stopping all updates indefinitely, although deprioritizing the updates is a reasonable use case).

@vmpstr vmpstr closed this as completed in 763d618 Aug 2, 2022
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

10 participants