-
Notifications
You must be signed in to change notification settings - Fork 682
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-position-4] Is the overlay property ready to ship? #8730
Comments
"I think it would be useful for the Blink process to inform us, at least as a courtesy, when these intents are proposed for CSS features." What would be the easiest/best/clearest/least disruptive way to do that? I want to make sure that we are not requiring something that is going to turn into a firehose the WG doesn't want. How can we turn this into a well-defined gate? (And "does the CSS WG know about this feature" obviously is too loosely interpretable. :) ) |
Good question :) The least disruptive way (which has been used quite often) is to make it part of the discussion in the last substantive issue. Either a comment or in minuted discussion just say something like, “We think this is the last issue that needs resolution before we announce an intent to ship” gives everyone a heads-up that (if they are interested) they should make a last pass over the spec and get any concerns they have raised quickly. Another way (that also gets used from time to time) is to open an issue like this (or #7533 as another example). If the spec itself is not in a state where the whole thing can progress, we can make a determination that the group agrees a particular feature is ready. Even if that doesn’t happen, the issue might spur someone to give the spec another review. The intents you all post are useful inputs to our process, and notifying us of relevant ones is welcome. The whole blink-dev list is a firehose I do not want to require everyone in the group to monitor, but I do want everyone in the group to be aware of your intents for the specifications we are working on. |
No, of course not, I wasn't suggesting "add css WG to the blink-dev list". :) And @fantasai pointed me at a couple of other examples: It seems like perhaps something like "if this is still considered an unstable feature according to CSS wg, you should file an 'is this feature shippable' issue and link here" would be a good addition to the blink intent process? |
Yes, if an upcoming intent to ship has not already been communicated in regular issue discussion. I think it would also be good to wait on a ship intent until we have had the editor’s draft published (which is another one of our review checkpoints that we just got to today for this feature). Blink’s participants in the group can request publication at any time (we prioritize these requests over everything else). |
I want to raise the concern that a bunch of spec text landed without any prior review, notably the model described in this comment: #8189 (comment) No resolution was made for this particular model, and it was never discussed in any CSSWG call. Yet it ended up in the CSS positioning spec, which is fairly disappointing, and a bunch of follow up PRs in fullscreen/HTML specs were opened as well. The change from 1 to 2 element collections is a significant one. I also asked for clarification on why the new model is linked to rendering updates without any clear answer. |
Hi @nt1m,
It was discussed in a CSSWG calll, see here.
Those were added to resolve the longstanding desire to move the definition of the top layer to CSS, via css-position-4, which was also discussed in the CSSWG as linked above, and also an earlier one a year or so ago.
I don't know what you mean by this, can you clarify? Is it the pending top layer removals list? There's relevant discussion on the various PRs about how this is a concept related to the algorithms that use the top layer (fullscreen, popover, etc), I can link you to Tab for more details on why it's written that way right now if you need it.
Could you clarify which question didn't receive an answer? I and others responded to all of your concerns and questions in #8189, as far as I can tell. |
Yes, as far as I can tell I or Chris answered all the questions you raised, @nt1m (in #8189 (comment) and #8189 (comment)), and you didn't respond any further . If we skipped something, please restate, with our apologies. |
From the notes, the model wasn't really covered in detail.
Ah I missed the last comment of #8189, sorry. I did ask for a new issue to be filed, since it takes time to review specs on our side in general, and having a proper place to do it without things fall in the void is generally good. Anyway, I guess we can re-use this issue.
This doesn't quite explain how this works relative to transitions, afaik, removing from the top layer kills the transition, so this means update the rendering will kill all pending removal transitions for top layer rendering at least, so this makes me question whether this model actually works. This is the main part I wanted clarification about. In general the spec could use more precision around the role the From reading the spec right now, adding to the top layer will trigger right away "rendered in the top layer" style adjustments, so this makes me question what the
I do think this should be defined, it is quite an important part of the feature design. |
Actually, I think we've usually had them as their own issue, although often the discussion groups several issues including whether we think it's OK to ship. And I think that's better (as its own issue, as in the examples Chris points out), both because the discussion can be more focused and less confused; and also because it requires its own set of edits (to the snapshot). |
Yes, actually removing an element from the top layer bypasses anything that
Hm, what else do you want detail on in there? Happy to expand on this as necessary.
Correct, as soon as it's in the top layer list, it renders as part of the top layer (and stops doing so as soon as it's not in the list). The 'overlay' property exists to allow authors to signal they want to delay removal from the top layer for some amount of time, by adding a transition. That's all it does. |
We often leave anti-abuse mechanisms undefined to allow for UAs to tweak them over time as proves necessary/prudent. I wouldn't be opposed to adding a liberal limit that you UAs must at least apply (but still allowing them to do shorter, or kill a transition for any other reason). However, the question is just whether it's actually something worthwhile to make testable. |
This makes sense, thanks.
OK that clears up a lot of things, thanks. I assumed there was also a way to delay the entry based on the name of the other issue (entry/exit transitions for top layer), but thinking about it, I guess that use case isn't terribly useful. Thanks for the clarifications! Makes it easier to review the proposal. |
…er exactly what its purpose is. #8730
I've done a bit of rewriting in the 'overlay' section to hopefully make it clearer exactly what the role of the property is, too. |
… as well, to allow for entry animations as well. #8730
I've made a minor change - "rendered in the top layer" now checks both that it's in the top layer list and has overlay:auto. The only effect of this is that entry animations are now possible, as you can delay the none->auto transition to keep the element displaying in the page normally for a little bit. I've adjusted the language in the This brings up a new possibility, tho. If authors can delay the element both leaving and entering the top layer, I'm not sure the "only set by the UA" is important any longer. At this point, the spec's model is essentially "the UA and the author can both signal whether they want the element in the top layer or not, and it only changes state when both agree". (The UA signals by putting something in the top layer list or the pending top layer removals list; the author signals by controlling 'overlay'.) As currently written, tho, the " The model is robust enough now that I don't think we're not protecting the author from anything important by not letting them set So do we want to remove that restriction, and make the UA-set |
I think it's positive that this looking more similar to a normal CSS property but @smfr pointed out this part is actually potentially a footgun:
If someone sets |
Hm, could you elaborate on what you think is problematic about that case? If they have both entry and exit animations, and set overlay in both, this'll work as expected. If they only have one or the other, it'll be broken anyway, locked permanently in the "displayed as top layer" or "displayed as normal" styles. The worst case I can see is them setting overlay in one of the animations but not the other, but they can do the same thing with any other property as well, breaking their page without our help. I don't think this is unique or particularly dangerous. |
I see an intent to ship for this property has been posted:
https://groups.google.com/a/chromium.org/g/blink-dev/c/YIgIas5_Q8s/m/okhgfSdlCgAJ
I think it would be useful for the Blink process to inform us, at least as a courtesy, when these intents are proposed for CSS features.
I have concerns that there has not yet been enough review on this very-recently-updated editor’s draft, so I am filing this issue to let everyone know they might want to spend some time looking at these changes. The intent to ship gets the details of the property’s values wrong, so the Blink contacts for this intent may also want to review the draft again.
There is still an active discussion in #8189, so the assertion that “the spec is done” in the related webkit/gecko standards positions issues is likely premature.
Please open any new issues on the draft that are needed (and link them here).
The text was updated successfully, but these errors were encountered: