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 content-visibility property #306

Closed
1 of 3 tasks
vmpstr opened this issue Sep 10, 2018 · 25 comments
Closed
1 of 3 tasks

CSS content-visibility property #306

vmpstr opened this issue Sep 10, 2018 · 25 comments
Assignees
Labels
Progress: propose closing we think it should be closed but are waiting on some feedback or consensus Topic: CSS Topic: graphics

Comments

@vmpstr
Copy link

vmpstr commented Sep 10, 2018

Bonjour TAG,

I'm requesting a TAG review of:

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our Github repo for each point of feedback
  • open a single issue in our Github repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]
@plinss plinss added this to the 2018-10-09-telcon milestone Sep 18, 2018
@travisleithead travisleithead self-assigned this Oct 9, 2018
@torgo torgo changed the title TAG Review Request: Display Locking Display Locking Oct 30, 2018
@kenchris
Copy link

kenchris commented Oct 31, 2018

At TPAC, it sounded as there is some overlap between this and the Invisible Searchable DOM proposals and that you needed to work out what solution to promote or whether you could come up with a new joined proposal.

In the light of that, are you still looking for feedback on this proposal at this time?

@rakina

@kenchris
Copy link

Given the opposition from Apple and Mozilla (see WICG/virtual-scroller#164) is this moving forward, and worth out time to review in details?

@kenchris
Copy link

kenchris commented Mar 3, 2020

The URL (and hashchange) could change/fire with the find-in-page especially with the new scroll to text fragment feature https://github.com/WICG/ScrollToTextFragment/

@vmpstr
Copy link
Author

vmpstr commented Mar 3, 2020

Just to clarify your last point here, do you mean that it already should fire on find-in-page and scroll-to-text, or that we should update hashchange spec so that it fires on find-in-page and scroll-to-text?

When considering find-in-page specifically, which is a primary use case for us, I think it would be somewhat awkward to have hashchange fire on a find-in-page match (mostly since the hash part of the URL doesn't change). In order to satisfy our use case, it would also have to fire on the element that contains the find-in-page match text (as opposed to on the window), which adds to the awkwardness in my opinion.

I think we can safely drop fragment-link navigation (ie #id navigation) from beforematch since that can be handled by hashchange for sure. Although in my mind it still feels like it is useful to include it in beforematch.

@kenchris
Copy link

kenchris commented Mar 3, 2020

Yes, it would probably have to be a fragmentchange event or similar as we are changing the url fragments. I don't know if there are any plans to change the fragments when selecting a match using find-in-page but it could make sense for copying the URL instead of having to select the text and then generate a url with the right fragment

@domenic
Copy link
Member

domenic commented Mar 3, 2020

Although in my mind it still feels like it is useful to include it in beforematch.

Strong +1. It's better to leave hashchange for being dedicated to handling hash changes, and not force developers to structure their code like

document.addEventListener("beforematch", handleBeforeMatch);
document.addEventListener("hashchange", () => {
  handleHashChange();
  handleBeforeMatch();
});

Letting "beforematch" be the one-stop-shop for this sort of handling is a much more elegant and decoupled design, instead of taking a dependency on the incidental fact that we already have an event for 1 of 3 cases.

@kenchris
Copy link

kenchris commented Mar 3, 2020

Maybe we can find a better name than beforematch - it wasn't really clear to me when it would fire. Like if I do find-in-page for "firefox" and it finds 3 matches, will it fire for the second one just as I click on "move to next"?

It is also not clear that match relates to find-in-page etc

@vmpstr
Copy link
Author

vmpstr commented Mar 3, 2020

Like if I do find-in-page for "firefox" and it finds 3 matches, will it fire for the second one just as I click on "move to next"?

Yes, that's the intended behavior

Maybe we can find a better name than beforematch

That's fair, we're discussing it here: WICG/display-locking#115

@dbaron
Copy link
Member

dbaron commented Mar 3, 2020

Strong +1. It's better to leave hashchange for being dedicated to handling hash changes, and not force developers to structure their code like

Yeah, I think that's fine. But I think it's worth mentioning the existence of hashchange in the explainer where you mention that use case.

@vmpstr
Copy link
Author

vmpstr commented Mar 6, 2020

After further testing (with @josepharhar's help), I'm no longer sure that my position of keeping fragment navigation is correct.

Specifically, script can change the hash and expect that the scroll be changed immediately. The way we've been thinking about beforematch is that its handler can and sometimes will change something about the rendering of the page that may shift elements around. It might be desirable to delay scroll until the event handler ran, but we can't do that for fragment link navigation without breaking the web.

I've filed WICG/display-locking#135 to discuss this further.

@vmpstr
Copy link
Author

vmpstr commented May 11, 2020

Hello,

We'd like to focus only on the CSS content-visibility property in this issue (contain-intrinsic-size and beforematch events should be discussed separately if needed). The current version of the spec draft can be found here:
https://drafts.csswg.org/css-contain-2/#content-visibility

We would appreciate another pass at this review!

Note: If possible please file an issue in https://github.com/w3c/csswg-drafts/issues/ with your feedback

@dbaron dbaron changed the title Display Locking CSS content-visibility property May 11, 2020
@dbaron dbaron added this to the 2020-05-21-f2f-seoul milestone May 11, 2020
@kenchris
Copy link

@vmpstr the spec doesn't have the hidden-matchable which the explainer says will solve use-case 2. Was that dropped and should the explainer be updated?

@kenchris kenchris added Progress: propose closing we think it should be closed but are waiting on some feedback or consensus and removed Progress: pending editor update TAG is waiting for a spec/explainer update Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review Missing: spec labels May 28, 2020
@atanassov atanassov added Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review Progress: pending editor update TAG is waiting for a spec/explainer update and removed Provenance: Fugu Topic: HTML labels May 28, 2020
@kenchris kenchris added Provenance: Fugu Topic: CSS and removed Progress: pending editor update TAG is waiting for a spec/explainer update Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review Provenance: Fugu labels May 28, 2020
@atanassov
Copy link

@dbaron @kenchris and myself took another look this issue, now scoped to content-visibility only. It would be good to update the explainer to match the CSS Containment module, ex. the value hidden-matchable is no longer part of the spec.

We advise you to continue working with the CSSWG in completing the rest of the spec.

@vmpstr
Copy link
Author

vmpstr commented May 28, 2020

The hidden-matchable value is something that we would still like to pursue in the future but it would require a separate discussion / design review, as well as require additional features like #511 to be useful.

For now, we're focussing on the values outlined in the spec draft. I will update the explainer to separate the proposed values here from the possible new values that we are thinking of proposing.

Thanks!

@vmpstr
Copy link
Author

vmpstr commented May 28, 2020

I've updated this in the repo, hopefully the separation of the features is more clear now. https://github.com/WICG/display-locking/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Progress: propose closing we think it should be closed but are waiting on some feedback or consensus Topic: CSS Topic: graphics
Projects
None yet
Development

No branches or pull requests