open-ui icon indicating copy to clipboard operation
open-ui copied to clipboard

[popup] Standardize the basic CSS for an element with the `popup` attribute

Open mfreed7 opened this issue 1 year ago • 43 comments

Akin to the <dialog> element's standardized styling, I think it'd be helpful to authors to get a very basic set of standardized styles that the UA applies to pop-ups by default. Otherwise, <div popup>Hello!</div> is just the word "Hello", and isn't set apart from the page at all, which can be a bit confusing.

The default styles can be almost identical, I think, to the ones for dialog, perhaps only changing position:absolute to position:fixed:

[popup="" i],
[popup=auto i],
[popup=hint i],
[popup=manual i] {
  position: fixed;
  inset-inline-start: 0;
  inset-inline-end: 0;
  width: fit-content;
  height: fit-content;
  margin: auto;
  border: solid;
  padding: 1em;
  background-color: Canvas;
  color: CanvasText;
}

It would seem to make sense for this UA rule to come before any rules that match specific elements, so that element-specific CSS overrides these.

Generally, thoughts? Objections?

mfreed7 avatar Jul 06 '22 18:07 mfreed7

Similarly, the fullscreen spec contains this set of standard styles that apply to the ::backdrop pseudo element:

::backdrop {
  position:fixed;
  top:0; right:0; bottom:0; left:0;
}

*|*:not(:root):fullscreen::backdrop {
  background:black;
} 

As written, and apparently as implemented in at least Chromium, the above CSS is used for both fullscreen elements and modal <dialog> elements.

For a pop-up element, I think the most common usage pattern would be to not style the ::backdrop, because the pop-up isn't modal and doesn't want to obscure the page. But it seems helpful to at least include the sizing rules, and perhaps manage pointer events more carefully? How about this?

[popup="" i]::backdrop,
[popup=auto i]::backdrop,
[popup=hint i]::backdrop,
[popup=manual i]::backdrop {
  position: fixed;
  top:0; right:0; bottom:0; left:0;
  background: transparent;
  pointer-events: none !important; /* So that clicks on the page can light-dismiss the pop-up */
}

mfreed7 avatar Jul 06 '22 19:07 mfreed7

I think the thing I'm most unsure of is the initial positioning. ideally these popups would be positioned based on their anchoring element, where applicable. They wouldn't necessarily be fixed all the time?

Other question, so long as the attribute can be used on any element, i would expect some of these UA styles to be rather weak - e.g., the hidden attribute's display: none, to ensure that the base UA styles of a specific element (let's say blockquote) or other author styles take priority. The latter not so much being a question as an expectation, but just trying to cover all the basis.

scottaohara avatar Jul 06 '22 20:07 scottaohara

I think the thing I'm most unsure of is the initial positioning. ideally these popups would be positioned based on their anchoring element, where applicable. They wouldn't necessarily be fixed all the time?

Hmm, interesting point. I think the problem is that technically you don't need to use the Anchor Positioning API to position a pop-up - you can put it top-left, or centered on the screen or whatever. I'm not sure what default style we'd apply that would "typically work" in the case that the pop-up has an anchor attribute. Perhaps a rule like:

[popup][anchor] {
  top: anchor(bottom);
  left: anchor(left);
}

That feels odd to add to the standardized CSS though.

Other question, so long as the attribute can be used on any element, i would expect some of these UA styles to be rather weak - e.g., the hidden attribute's display: none, to ensure that the base UA styles of a specific element (let's say blockquote) or other author styles take priority. The latter not so much being a question as an expectation, but just trying to cover all the basis.

Totally agree - I'm not exactly sure how to specify that though. But that's what I meant by this sentence:

It would seem to make sense for this UA rule to come before any rules that match specific elements, so that element-specific CSS overrides these.

mfreed7 avatar Jul 15 '22 00:07 mfreed7

yeh, thinking about it more, maybe if those anchoring styles were put in place (and if an rtl style where it was right: anchor(right) instead of left), then it would be on the author to do the rest. that seems like the best reasonable default.

and sorry, don't know how that last sentence you had in there about UA style positions didn't register for me.

scottaohara avatar Jul 15 '22 11:07 scottaohara

I think we likely also need to pull in a few more lines from the The rendering section of the spec for <dialog> under the "when its is modal flag is true" section, particularly these:

  inset-block-start: 0;
  inset-block-end: 0;
  overflow: auto;

The <dialog> rendering section also includes these two rules, which seem like they must have some history. Not sure whether they should be ported to <div popup> or not:

  max-width: calc(100% - 6px - 2em);
  max-height: calc(100% - 6px - 2em);

I'm sure @domenic knows where these come from and whether they are required here.

mfreed7 avatar Aug 01 '22 21:08 mfreed7

For simplicity, I would remove border and padding from the default stylings and set background color to transparent. I can imagine most people overriding this.

I agree that you'd only want anchor positioning when you have an anchor element, but I'm not sure how you would style that from the UA.

tbondwilkinson avatar Aug 12 '22 16:08 tbondwilkinson

For simplicity, I would remove border and padding from the default stylings and set background color to transparent. I can imagine most people overriding this.

I just copied those nearly verbatim from <dialog>'s styling. The rationale, I imagine, was to set the <dialog> off from the page at least a little, by default. If you remove the border and make the background transparent, when you first start playing with <div popup>Hello!</div>, it can be confusing, since you can barely find the word "Hello!" overlaying other page content somewhere. Adding the padding/border/background makes it very easy. And as you point out, it's very easy to override.

@domenic do you know where the conversation took place about this styling for <dialog>? It'd be interesting to see how we arrived here.

mfreed7 avatar Aug 12 '22 20:08 mfreed7

While likely to be overwritten, being that this content should work in situations where author styles could be removed or revised (e.g., browser reader mode), I don't understand how it would be ok for the UA default style for an element whose purpose is to overlay other content of the page could have a transparent background.

scottaohara avatar Aug 12 '22 21:08 scottaohara

@domenic do you know where the conversation took place about this styling for <dialog>? It'd be interesting to see how we arrived here.

I can't find anything. In https://github.com/whatwg/html/commit/2fb24fcf3f916236e8767e2cb72b23e5c75b77e9 they arrived in the spec with full default styling. Various Google searches with site:w3.org bugzilla or site:lists.whatwg.org don't give any leads.

I personally like the idea of following the weak precedent provided by <dialog>. At least the background and border. I could be swayed to omit the padding.

domenic avatar Aug 15 '22 02:08 domenic

Regarding ::backdrop styling, I'm not sure many additional styles are necessary? You already get position, top, right, bottom, and left from the fullscreen spec's general ::backdrop. background should default to transparent. So what's left is just adding pointer-events: none !important; for the popup cases.

domenic avatar Aug 15 '22 02:08 domenic

@domenic thanks for digging, and too bad there's not much context. Also thanks for pointing out that the fullscreen spec already mandates most of the ::backdrop styling. Chromium's implementation is individual per API, which is why I didn't even check.

If I keep the background/border because I heard several supportive comments, leave out the padding, and remove the overlap on ::backdrop styling, here's what's left:

[popup="" i],
[popup=auto i],
[popup=hint i],
[popup=manual i] {
  position: fixed;
  inset-inline-start: 0;
  inset-inline-end: 0;
  inset-block-start: 0;
  inset-block-end: 0;
  width: fit-content;
  height: fit-content;
  margin: auto;
  border: solid;
  overflow: auto;
  color: CanvasText;
  background-color: Canvas;
}

[popup="" i]::backdrop,
[popup=auto i]::backdrop,
[popup=hint i]::backdrop,
[popup=manual i]::backdrop {
    pointer-events: none !important;
}

[popup]:-internal-popup-hidden {
  display: none;
}

Objections?

mfreed7 avatar Aug 16 '22 18:08 mfreed7

[popup]:-internal-popup-hidden

This seems bad?

domenic avatar Aug 17 '22 00:08 domenic

[popup]:-internal-popup-hidden

This seems bad?

Sorry, in what sense? The behavior is bad? Or using an internal pseudo notation isn't the right way to express this in specs? (If the former, help me understand why it's bad? If the latter, what's the right way to specify this?)

mfreed7 avatar Aug 17 '22 23:08 mfreed7

I don't know what that's expressing, but I sure know that it's not something specs do. What is the goal of that Chromium-specific syntax? Then maybe we can talk about how to express it in specs.

domenic avatar Aug 18 '22 00:08 domenic

I don't know what that's expressing, but I sure know that it's not something specs do. What is the goal of that Chromium-specific syntax? Then maybe we can talk about how to express it in specs.

Ahh ok. So a <div popup> is display:none before it is shown. This rule makes that happen, via an internal CSS pseudo state (:-internal-popup-hidden) that matches when the pop-up shouldn't be showing. In this section of the explainer, step #1 of .showPopUp() is this:

  1. Move the pop-up to the top layer, and remove the UA display:none style.

The CSS rule in question is how that is implemented. Suggestions appreciated (either here or on the upcoming spec PR, whichever you prefer) for how to write that in specs.

mfreed7 avatar Aug 18 '22 15:08 mfreed7

The Open UI Community Group just discussed [popup] Standardize the basic CSS for an element with the popup attribute, and agreed to the following:

  • RESOLVED: The standard UA CSS for the pop-up API should be as written in https://github.com/openui/open-ui/issues/561#issuecomment-1216971826, but with simple [popup] selectors instead of [popup=value], etc. We still need to resolve the `display:none` selector.
The full IRC log of that discussion <masonf> Topic: [popup] Standardize the basic CSS for an element with the popup attribute
<masonf> github: https://github.com/openui/open-ui/issues/561
<bkardell_> masonf: this is about standardizing styles for the popup - there has been some good discussion on the issue. Mostly a copy/paste of the styles for dialog..
<bkardell_> masonf: if you don't have those things (border, background) it is very confusing
<masonf> q?
<bkardell_> masonf: the counterpoint is that at some point it gets opinionated and there is a lot you have to set. We did some investigation on why we got those particular styles, but nothing much to say here we think they are deccent
<scotto_> q+
<masonf> ack: scotto
<masonf> ack scotto_
<bkardell_> scotto_: I will generally +1 all the things you said. Stripping them away would be confusing - there has to be a reasonable UA default. The only one I am somewhat on the fence is backdrop styling - is it default transparent?
<emilio> q+
<bkardell_> masonf: it comes as part of fullscreen and it does have default styling.. we wouldn't have to add much here - the only item left to provide is pointer-events: none
<bkardell_> scotto_: I think most wouldn't need a backdrop, but makes sense
<una> q?
<bkardell_> emilio: I see you mentioned the rules are ..?... is that necessary? It seems better to have the styles be generic and applied to everything rather than every specific value? Does popup=true do something different? that would be unfortunate
<bkardell_> masonf: the answer to your last question is that that is an invalid value.
<bkardell_> masonf: any other value is not a valid popup and won't get any of the styling - that is why I put the explicit value in there
<bkardell_> masonf: I think it would be a mistake to make it look like it if it couldn't be it
<bkardell_> emilio: Is there a precedent for enumerated values where no value means a default like this
<bkardell_> emilio: you would need to write that 4 times, which is unfortunate
<bkardell_> masonf: there is nothing in there about the anchor attribute
<emilio> https://github.com/openui/open-ui/issues/561#issuecomment-1185047527
<bkardell_> emilio: maybe that comment is obsolete?
<bkardell_> masonf: ah... the standard styling is just fixed, centered on the screen. there was a question - there is anchor positioning that is a separate proposal but would be used here extensively. There was a question about whether we should rely on anchor positioning in the UA sheet by default
<bkardell_> masonf: that's the nature of the queastion here
<bkardell_> emilio: but my question is that it forces you to quadruplicate - at a minimum, more values in the future could make it more
<bkardell_> masonf: I'm not sure if there is a precent of exactly this where the missing value default is different - I do take your point about multiple selectors
<bkardell_> masonf: I think developers will do the simple thing
<bkardell_> q+
<masonf> ack emilio
<bkardell_> emilio: I'm looking at the enumerated attributes - most of them seem like if you specify an invalid attribute, there is still some reasonable default functionality
<bkardell_> emilio: if we have precedence -ok, but it feels a bit unfortunate.
<bkardell_> emilio: it's global and can apply to every element - maybe we could specify something simple... i don't know... in general I'm a bit sad about the complexity
<bkardell_> masonf: we should dig up the issue about why we decided this for context - the reason I recall was for forward compatibility. If we don't do it this way we can't really add new ones easily
<bkardell_> emilio: I don't think that has been a real issue for input type="bazinga"
<bkardell_> masonf: there's also some feature detection stuff where you can check whether it is supported
<masonf> q?
<masonf> the issue is this one: https://github.com/openui/open-ui/issues/533
<emilio> FWIW: let input = document.createElement("input"); input.type = "bazinga"; input.type // "text"
<emilio> bkardell_: there's a disconnect between the two statements
<emilio> ... I think the UA stylesheet needs to do something useful
<emilio> ... I'm not sure I'm making a point other than this is worth thinking about
<masonf> q?
<emilio> masonf: I think you almost convinced me, r/n you do an invalid attribute value you get a warning, so not sure we should bend over backwards
<emilio> scotto_: so does this mean that we should change what happens when you have an invalid popup?
<emilio> scotto_: because right now popup=something shows up on the screen and isn't a popup
<masonf> q?
<emilio> masonf: right, with the simplified UA style that'd be a fixed-pos element, but you'd get a console warning
<emilio> q+
<emilio> scotto_: I don't have a strong opinion, so long as it's broken equally for everyone it seems fine
<bkardell_> ack BoCupp
<bkardell_> ack bkardell_
<bkardell_> scribenick: bkardell_
<masonf> ack emilio
<bkardell_> emilio: I just wanted to express a slight pref toward making an invalid value still a popup somehow - like type on input a bit... if it has been discussed and there is a counter-example I'd be interested in looking at it
<masonf> q?
<emilio> emilio: slight preference for making popup="invalid" still be a popup, seems similar to <input type="invalid">
<bkardell_> masonf: I posted 533 above - give it a read it has that context, and add your comments - we can reconsider if there are new thoughts.
<masonf> q?
<masonf> Proposed resolution: The standard UA CSS for the pop-up API should be as written in https://github.com/openui/open-ui/issues/561#issuecomment-1216971826, but with simple [popup] selectors instead of [popup=value], etc.
<bkardell_> masonf: the big diff between those imo is input is an element, but popup is an attribute
<una> SGTM
<miriam> +1
<bkardell_> emilio: {something}
<bkardell_> masonf: yes
<masonf> Proposed resolution: The standard UA CSS for the pop-up API should be as written in https://github.com/openui/open-ui/issues/561#issuecomment-1216971826, but with simple [popup] selectors instead of [popup=value], etc. We still need to resolve the `display:none` selector.
<emilio> s/{something}/we can't standardize the :-internal- pseudo but yeah :)
<bkardell_> +1
<scotto_> +1
<masonf> RESOLVED: The standard UA CSS for the pop-up API should be as written in https://github.com/openui/open-ui/issues/561#issuecomment-1216971826, but with simple [popup] selectors instead of [popup=value], etc. We still need to resolve the `display:none` selector.
<bkardell_> q+

css-meeting-bot avatar Aug 18 '22 18:08 css-meeting-bot

@emilio mentioned on https://github.com/openui/open-ui/issues/533 that the lack of !important on the display:none rule means that this CSS breaks things:

[popup] {
  display: flex;
}

...because it overrides the UA rule that keeps non-showing pop-ups from being displayed.

The original purpose of not having !important on that rule was to make animations easier. But with https://github.com/openui/open-ui/issues/335 fixing those problems, I think we should probably add back !important.

mfreed7 avatar Aug 18 '22 23:08 mfreed7

Ok, with the resolution above plus the additional discussion, I think this is where we are:

[popup] {
  position: fixed;
  inset-inline-start: 0;
  inset-inline-end: 0;
  inset-block-start: 0;
  inset-block-end: 0;
  width: fit-content;
  height: fit-content;
  margin: auto;
  border: solid;
  overflow: auto;
  color: CanvasText;
  background-color: Canvas;
}

[popup]::backdrop {
    pointer-events: none !important;
}

[popup]:{something that denotes the pop-up is in the hidden state} {
  display: none !important;
}

mfreed7 avatar Aug 19 '22 17:08 mfreed7

In my CL to implement the above changes, the reviewer called out the lack of padding explicitly. I don't disagree - having a border without padding looks odd to me:

with padding with_padding

no padding no_padding

Also, to point it out, <dialog> has padding:

dialog dialog

Are we sure we explicitly don't want to include padding in the default popup styles?

mfreed7 avatar Aug 22 '22 20:08 mfreed7

For what it's worth, "the reviewer" was me.

I think the norm is that if a default style has border, then it should probably have padding as well, since a border with 0 padding looks pretty bad.

Given that popups are often smaller than dialogs, I'm not sure that the default for padding needs to be 1em, though. I think 0.25em or 2px would be plausible defaults as well. (Table cells default to 1px, even.)

dbaron avatar Aug 22 '22 20:08 dbaron

Yeah some padding makes sense to me. I don't have a strong preference for how much. Half or quarter of an em probably makes sense (and it's probably sensible to make it em-based to be consistent with dialog, but...)

emilio avatar Aug 22 '22 20:08 emilio

I think I suggested leaving out the padding, but I meant it in the context of also leaving out the border because most people would probably override that style. People pointed out that we need the dialog to appear normal even if CSS doesn't load though, so I agree that if you have a border, some padding probably makes sense.

tbondwilkinson avatar Aug 22 '22 20:08 tbondwilkinson

Thanks for the quick comments. Sounds like the start of consensus to make that change. While I'm here, I'll also point out this issue (https://github.com/whatwg/fullscreen/issues/201) because I think it makes sense to simplify the four inset-* properties into just inset:0. LMK if there are any disagreements with that.

Taken together, we're now here:

[popup] {
  position: fixed;
  inset: 0;      /* Simplify to this */
  width: fit-content;
  height: fit-content;
  margin: auto;
  border: solid;
  padding: 0.25em;  /* Since I heard two people include 0.25em in their comments */
  overflow: auto;
  color: CanvasText;
  background-color: Canvas;
}

[popup]::backdrop {
    pointer-events: none !important;
}

[popup]:{something that denotes the pop-up is in the hidden state} {
  display: none !important;
}

Also for reference:

Screen Shot 2022-08-22 at 2 53 42 PM Screen Shot 2022-08-22 at 2 54 05 PM

mfreed7 avatar Aug 22 '22 21:08 mfreed7

i personally like the .25em as a starting point. but if people prefer the .5em, then that's absolutely fine as well.

scottaohara avatar Aug 22 '22 23:08 scottaohara

Yay for default padding, 0.25em and 0.25em both look like a fine default to me.

hidde avatar Aug 23 '22 10:08 hidde

From someone who's spent a lot of time with popup recently, I don't think it makes sense to set display: none !important in the UA stylesheet 👎

It's kinda restricting for use cases and scenarios that might pop up (no pun intended 😅)

You can still set a layout style inside the pop-up when it is in the top layer with

[popup]:top-layer { display: flex; }

But, a wrapper element within the popup also does the trick. I can think of and have been playing with scenarios where you could have a popup shown in the DOM by default and then promote it to the top layer. For example, think of pop out media like a video player that's in the normal document flow that you promote to top layer when playing. Or how you might do certain picture in picture mode. Especially with shared element transitions.

Also, PWA and app like components like sliding drawers etc. You might have a tab that you press or tap to promote an action sheet or drawer to the top layer. You then get the backdrop and light dismiss which is handy. We were playing with a concept like this the other day for native feel UI. I've also built things like this in the past which can be tricky when building app like experiences on the web.

I'm not saying that having display: none will be a bad thing. But, blocking any sort of escape hatch for it wouldn't be ideal. Otherwise we potentially block creative and advanced scenarios we might not have even thought of yet.

The workaround as discussed with @mfreed7 would be to have some sort of JavaScript intervention that applies the popup attribute and calls showPopUp when necessary but seems rather OTT when before there was way out with only CSS and no JavaScript required.

jh3y avatar Aug 23 '22 17:08 jh3y

Thanks for the additional votes for 0.25em!

From someone who's spent a lot of time with popup recently, I don't think it makes sense to set display: none !important in the UA stylesheet 👎

I just wanted to point out also that the <dialog> element does not have a similar !important rule. E.g.:

<dialog>This will be visible!</dialog>
<style>dialog {display: flex;}</style>

Perhaps we should revisit that hasty decision on my part.

mfreed7 avatar Aug 23 '22 17:08 mfreed7

I think display: none; is definitely a reasonable UA default, but I agree that !important feels a bit much.

FWIW, pop-out media players to me usually look modal in that they make the rest of the page non-interactible, see e.g. https://www.google.com/search?q=how+to+fish click on the video there, though this one completely hides the background you could imagine it graying the background instead. But since popups are intended to be modeless, those types of use cases aren't I think supported.

But I do think that people will be very tempted to use popup for things that they want to light dismiss, which may include taking parts of the DOM and showing them more prominently in the top-layer, or combining it with anchor positioning to move them out of their flow and attach to some other element.

tbondwilkinson avatar Aug 23 '22 17:08 tbondwilkinson

I don't think it would be so bad if we were able to override the UA styles, but, we can't.

The pop-out media idea was just one of those that came to mind. FWIW, I've definitely seen them light dismissable in the past where dismissing can even pause the content. You could achieve that behavior described with popup=manual.

Yeah, for me, it's impossible to think of every edge case or use case that might pop up. But, allowing people to show us would be ideal 👍

jh3y avatar Aug 23 '22 17:08 jh3y

@mfreed7 I removed the agenda label for now since this was discussed in the past. Do you think you need to get another resolution given the changes?

gregwhitworth avatar Aug 23 '22 19:08 gregwhitworth