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

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

Closed
mfreed7 opened this issue Jul 6, 2022 · 44 comments
Closed
Labels
popover The Popover API

Comments

@mfreed7
Copy link
Collaborator

mfreed7 commented Jul 6, 2022

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 mfreed7 added the popover The Popover API label Jul 6, 2022
@mfreed7
Copy link
Collaborator Author

mfreed7 commented Jul 6, 2022

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 */
}

@scottaohara
Copy link
Collaborator

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.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 6, 2022
When [1] landed, it forgot to change the default CSS for `async`
to `manual`. Clearly that indicates a lack of appearance testing,
so this CL adds a WPT for the appearance of pop-ups. I also opened
this issue to discuss the standard styling:

  openui/open-ui#561

This CL also had to add code to the <selectmenu> positioning algorithm
to remove any margin. I think that's the right way to handle this,
but that does mean developer `margin` settings on a slotted listbox
won't have any effect. That seems right, but I don't know for sure.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3722345

Bug: 1307772
Change-Id: I4ecf7bee947e90e2abdcedb47b3d38c958abb2f5
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 6, 2022
When [1] landed, it forgot to change the default CSS for `async`
to `manual`. Clearly that indicates a lack of appearance testing,
so this CL adds a WPT for the appearance of pop-ups. I also opened
this issue to discuss the standard styling:

  openui/open-ui#561

In the meantime, this CL also refactors all of the test-assumed default
styles for [popup] into popup-styles.css, so that if we change these
in the future it won't be so painful.

This CL also had to add code to the <selectmenu> positioning algorithm
to remove any margin. I think that's the right way to handle this,
but that does mean developer `margin` settings on a slotted listbox
won't have any effect. That seems right, but I don't know for sure.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3722345

Bug: 1307772
Change-Id: I4ecf7bee947e90e2abdcedb47b3d38c958abb2f5
aarongable pushed a commit to chromium/chromium that referenced this issue Jul 6, 2022
When [1] landed, it forgot to change the default CSS for `async`
to `manual`. Clearly that indicates a lack of appearance testing,
so this CL adds a WPT for the appearance of pop-ups. I also opened
this issue to discuss the standard styling:

  openui/open-ui#561

In the meantime, this CL also refactors all of the test-assumed default
styles for [popup] into popup-styles.css, so that if we change these
in the future it won't be so painful.

This CL also had to add code to the <selectmenu> positioning algorithm
to remove any margin. I think that's the right way to handle this,
but that does mean developer `margin` settings on a slotted listbox
won't have any effect. That seems right, but I don't know for sure.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3722345

Bug: 1307772
Change-Id: I4ecf7bee947e90e2abdcedb47b3d38c958abb2f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3749343
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1021422}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 6, 2022
When [1] landed, it forgot to change the default CSS for `async`
to `manual`. Clearly that indicates a lack of appearance testing,
so this CL adds a WPT for the appearance of pop-ups. I also opened
this issue to discuss the standard styling:

  openui/open-ui#561

In the meantime, this CL also refactors all of the test-assumed default
styles for [popup] into popup-styles.css, so that if we change these
in the future it won't be so painful.

This CL also had to add code to the <selectmenu> positioning algorithm
to remove any margin. I think that's the right way to handle this,
but that does mean developer `margin` settings on a slotted listbox
won't have any effect. That seems right, but I don't know for sure.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3722345

Bug: 1307772
Change-Id: I4ecf7bee947e90e2abdcedb47b3d38c958abb2f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3749343
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1021422}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 6, 2022
When [1] landed, it forgot to change the default CSS for `async`
to `manual`. Clearly that indicates a lack of appearance testing,
so this CL adds a WPT for the appearance of pop-ups. I also opened
this issue to discuss the standard styling:

  openui/open-ui#561

In the meantime, this CL also refactors all of the test-assumed default
styles for [popup] into popup-styles.css, so that if we change these
in the future it won't be so painful.

This CL also had to add code to the <selectmenu> positioning algorithm
to remove any margin. I think that's the right way to handle this,
but that does mean developer `margin` settings on a slotted listbox
won't have any effect. That seems right, but I don't know for sure.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3722345

Bug: 1307772
Change-Id: I4ecf7bee947e90e2abdcedb47b3d38c958abb2f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3749343
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1021422}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 14, 2022
…d more testing, a=testonly

Automatic update from web-platform-tests
Fix async->manual rename in CSS, plus add more testing

When [1] landed, it forgot to change the default CSS for `async`
to `manual`. Clearly that indicates a lack of appearance testing,
so this CL adds a WPT for the appearance of pop-ups. I also opened
this issue to discuss the standard styling:

  openui/open-ui#561

In the meantime, this CL also refactors all of the test-assumed default
styles for [popup] into popup-styles.css, so that if we change these
in the future it won't be so painful.

This CL also had to add code to the <selectmenu> positioning algorithm
to remove any margin. I think that's the right way to handle this,
but that does mean developer `margin` settings on a slotted listbox
won't have any effect. That seems right, but I don't know for sure.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3722345

Bug: 1307772
Change-Id: I4ecf7bee947e90e2abdcedb47b3d38c958abb2f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3749343
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1021422}

--

wpt-commits: fe9314db9649f54fbbe71c911af71c0cc2ac06ae
wpt-pr: 34729
@mfreed7
Copy link
Collaborator Author

mfreed7 commented Jul 15, 2022

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.

@scottaohara
Copy link
Collaborator

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.

aosmond pushed a commit to aosmond/gecko that referenced this issue Jul 15, 2022
…d more testing, a=testonly

Automatic update from web-platform-tests
Fix async->manual rename in CSS, plus add more testing

When [1] landed, it forgot to change the default CSS for `async`
to `manual`. Clearly that indicates a lack of appearance testing,
so this CL adds a WPT for the appearance of pop-ups. I also opened
this issue to discuss the standard styling:

  openui/open-ui#561

In the meantime, this CL also refactors all of the test-assumed default
styles for [popup] into popup-styles.css, so that if we change these
in the future it won't be so painful.

This CL also had to add code to the <selectmenu> positioning algorithm
to remove any margin. I think that's the right way to handle this,
but that does mean developer `margin` settings on a slotted listbox
won't have any effect. That seems right, but I don't know for sure.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3722345

Bug: 1307772
Change-Id: I4ecf7bee947e90e2abdcedb47b3d38c958abb2f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3749343
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1021422}

--

wpt-commits: fe9314db9649f54fbbe71c911af71c0cc2ac06ae
wpt-pr: 34729
@mfreed7
Copy link
Collaborator Author

mfreed7 commented Aug 1, 2022

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.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 2, 2022
These were missing - see [1]. Without at least the block start
and end positions, pop-ups could easily be shown off screen below
the viewport.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I006c664f89e8996d75c74130e4510967acfd486b
aarongable pushed a commit to chromium/chromium that referenced this issue Aug 2, 2022
These were missing - see [1]. Without at least the block start
and end positions, pop-ups could easily be shown off screen below
the viewport.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I006c664f89e8996d75c74130e4510967acfd486b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3803128
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030616}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 2, 2022
These were missing - see [1]. Without at least the block start
and end positions, pop-ups could easily be shown off screen below
the viewport.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I006c664f89e8996d75c74130e4510967acfd486b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3803128
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030616}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 2, 2022
These were missing - see [1]. Without at least the block start
and end positions, pop-ups could easily be shown off screen below
the viewport.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I006c664f89e8996d75c74130e4510967acfd486b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3803128
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030616}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 4, 2022
…ies to `popup`, a=testonly

Automatic update from web-platform-tests
Add block-start/end and overflow properties to `popup`

These were missing - see [1]. Without at least the block start
and end positions, pop-ups could easily be shown off screen below
the viewport.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I006c664f89e8996d75c74130e4510967acfd486b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3803128
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030616}

--

wpt-commits: 84cf9a63a6d8d96c2965ea018c0b170699e2c08d
wpt-pr: 35308
@tbondwilkinson
Copy link

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.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Aug 12, 2022

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.

@scottaohara
Copy link
Collaborator

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.

@domenic
Copy link
Contributor

domenic commented Aug 15, 2022

@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 whatwg/html@2fb24fc 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
Copy link
Contributor

domenic commented Aug 15, 2022

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.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Aug 16, 2022

@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 mfreed7 added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Aug 16, 2022
@domenic
Copy link
Contributor

domenic commented Aug 17, 2022

[popup]:-internal-popup-hidden

This seems bad?

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Aug 17, 2022

[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?)

@tbondwilkinson
Copy link

I think an :open pseudoclass kind of implies the possibility for a :closed? That feels like what Domenic might be asking. I think :opening and :closing are useful additional states, where something could be still marked as :closed but also as :opening for styling transitions, perhaps.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Aug 29, 2022

I think an :open pseudoclass kind of implies the possibility for a :closed? That feels like what Domenic might be asking. I think :opening and :closing are useful additional states, where something could be still marked as :closed but also as :opening for styling transitions, perhaps.

Ahh, you might be right that :closed was what @domenic was requesting? I always wonder whether we really should add negated pseudo classes for everything, when we have :not(). But here, this is interesting, because the "open" state for a pop-up is actually tri-state (hidden, transitioning, and showing). So there is an unobservable state unless we add a specific pseudo for it. Hence the :opening (or :opening-or-open?) suggestion.

@tbondwilkinson
Copy link

True, :not(:open) could be the thing that you specify here for display: none; for now?

I hadn't considered the option of a tri-state... or would it be a quad state so you could style transitioning to open and transitioning to close separately? I think people could probably argue for a while about whether to considering something that is "opening" "open" or "closed" so it might be good at least to consider side-stepping that argument by considering that a separate state. I do think people will fall into various categories:

  1. Consider open/close to be a binary. Either a popup is open or its not.
  2. Consider open/close to have additional edge states for opening and closing. And whether they considering opening/closing to fall under open or close may vary.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Aug 31, 2022

True, :not(:open) could be the thing that you specify here for display: none; for now?

Can't do that - :open means the pop-up is in the "showing" state. So :not(:open) would mean either "hidden" or "transitioning", and we definitely only want to apply display:none when "hidden". Else transitions are broken.

I hadn't considered the option of a tri-state... or would it be a quad state so you could style transitioning to open and transitioning to close separately? I think people could probably argue for a while about whether to considering something that is "opening" "open" or "closed" so it might be good at least to consider side-stepping that argument by considering that a separate state. I do think people will fall into various categories:

  1. Consider open/close to be a binary. Either a popup is open or its not.
  2. Consider open/close to have additional edge states for opening and closing. And whether they considering opening/closing to fall under open or close may vary.

Ugh. This is why I hesitated to specify an explicit pseudo state for this. Perhaps this part of the feature needs more discussion before we introduce it. Certainly I don't think it is needed for "v1".


Last call for objections to this being the proposed standard CSS for popup:

[popup] {
  position: fixed;
  inset: 0;
  width: fit-content;
  height: fit-content;
  margin: auto;
  border: solid;
  padding: 0.25em;
  overflow: auto;
  color: CanvasText;
  background-color: Canvas;
}

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

[popup]:{spec language for "is hidden"} {
  display: none;
}

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Sep 2, 2022

I'm going to close this issue as agreed, for now. I'm very happy to re-open it if something comes up.

Spec-wise, I think the {spec language for "is hidden"} portion is easier to write in spec prose than CSS, so the details we've been discussing above are likely implementation details. @josepharhar FYI, and hopefully I'm correct that there's an easy sentence to describe visibility. Could you please incorporate the updated CSS above into the spec PR?

@josepharhar
Copy link
Collaborator

Spec-wise, I think the {spec language for "is hidden"} portion is easier to write in spec prose than CSS, so the details we've been discussing above are likely implementation details

Yes, since the secret pseudo selector we're using in chromium should not be in the spec, I crafted the spec to have the same behavior without any selectors.

Could you please incorporate the updated CSS above into the spec PR?

Done: https://github.com/whatwg/html/pull/8221/files/9e0421c76e82132537c90e7da6e04885689c79de..a2cd6f0457d5d959916e1c01ba5c6a14a72160ee

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 5, 2022
…rtant to display:none, a=testonly

Automatic update from web-platform-tests
Simplify pop-up CSS rules, and add !important to display:none

Per the resolution and discussion at [1], we've decided to simplify
the UA rules for pop-up attribute selectors, to capture *any*
`popup` attribute value. Additionally (and this part is tentative),
it looks like we'll likely resolve to put back `!important` on
display:none, so that `[popup] {display:flex}` can work.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: If3d52cce0c9cbd3c6134ff57836d65c93eb12c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3840811
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1037921}

--

wpt-commits: a0ece3bbdbfdf1bd0f66f841b06d54c1d927af0a
wpt-pr: 35535
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Sep 5, 2022
…eet, a=testonly

Automatic update from web-platform-tests
Incorporate updates to pop-up UA stylesheet

There's been pushback [1] to the !important rule on display:none,
and more conversation about padding. This CL updates the behavior
to keep up with the conversation. It also un-breaks several demos
that were relying on the non-!important display:none rule.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: Ia267bf54546e433436963ae4a89d7828181c7015
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3851364
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1038529}

--

wpt-commits: b1b8cdddc3fe9cbb5fe13fc05cbe25f6d574b6dd
wpt-pr: 35586
@scottaohara
Copy link
Collaborator

scottaohara commented Sep 8, 2022

@mfreed7 was playing around with popup today and noticed that the default style for the backdrop was updated so that is has a background of rgba(0, 0, 0, 0.1). That doesn't jive with what's been outlined here though, as the backdrop shouldn't have any background color to it by default.

edit:

i realized this happened because i was using a dialog element that i was using popup=manual on. So, even though it was non-modal, the dialog's backdrop was being made visible.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Sep 8, 2022

Re-opening this issue to discuss what we need to add to the UA stylesheet to avoid (maybe?) the <dialog popup> getting that background color. Then again, maybe that's the correct background color since it's a <dialog>?

@mfreed7 mfreed7 reopened this Sep 8, 2022
@scottaohara
Copy link
Collaborator

thanks @mfreed7. Definitely think this is something that would need to be avoided since a dialog opened via .show() doesn't have a backdrop - which makes sense because its modeless. So a dialog opened via showPopUp - also being modeless - should also not have a backdrop by default.

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Sep 9, 2022

I suppose the right thing to do here would be to modify the UA stylesheet to make sure rgba(0,0,0,0.1) only gets applied to dialog:modal and not dialog:open.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 4, 2022
When a `<dialog>` is opened via `.showPopUp()`, it should have
a completely transparent ::backdrop. This CL fixes that by adding
a special case for dialog::open.

See also:
openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I444730c57c699f4af4a708cc316dd81aa7563669
aarongable pushed a commit to chromium/chromium that referenced this issue Oct 4, 2022
When a `<dialog>` is opened via `.showPopUp()`, it should have
a completely transparent ::backdrop. This CL fixes that by adding
a special case for dialog::open.

See also:
openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I444730c57c699f4af4a708cc316dd81aa7563669
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3933277
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1054947}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 4, 2022
When a `<dialog>` is opened via `.showPopUp()`, it should have
a completely transparent ::backdrop. This CL fixes that by adding
a special case for dialog::open.

See also:
openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I444730c57c699f4af4a708cc316dd81aa7563669
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3933277
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1054947}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 4, 2022
When a `<dialog>` is opened via `.showPopUp()`, it should have
a completely transparent ::backdrop. This CL fixes that by adding
a special case for dialog::open.

See also:
openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I444730c57c699f4af4a708cc316dd81aa7563669
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3933277
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1054947}
@mfreed7
Copy link
Collaborator Author

mfreed7 commented Oct 5, 2022

I suppose the right thing to do here would be to modify the UA stylesheet to make sure rgba(0,0,0,0.1) only gets applied to dialog:modal and not dialog:open.

Ok, the Chromium implementation has been modified to do this, by making the UA selector

[popup]:open::backdrop {

and adding

background-color: transparent;

@josepharhar is adding the same to the spec. I'm going to close this as re-resolved.

@mfreed7 mfreed7 closed this as completed Oct 5, 2022
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
When [1] landed, it forgot to change the default CSS for `async`
to `manual`. Clearly that indicates a lack of appearance testing,
so this CL adds a WPT for the appearance of pop-ups. I also opened
this issue to discuss the standard styling:

  openui/open-ui#561

In the meantime, this CL also refactors all of the test-assumed default
styles for [popup] into popup-styles.css, so that if we change these
in the future it won't be so painful.

This CL also had to add code to the <selectmenu> positioning algorithm
to remove any margin. I think that's the right way to handle this,
but that does mean developer `margin` settings on a slotted listbox
won't have any effect. That seems right, but I don't know for sure.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3722345

Bug: 1307772
Change-Id: I4ecf7bee947e90e2abdcedb47b3d38c958abb2f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3749343
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1021422}
NOKEYCHECK=True
GitOrigin-RevId: 2ed54d704dae56efa5e85f5ddbcf9bb6317c9faf
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
These were missing - see [1]. Without at least the block start
and end positions, pop-ups could easily be shown off screen below
the viewport.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I006c664f89e8996d75c74130e4510967acfd486b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3803128
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030616}
NOKEYCHECK=True
GitOrigin-RevId: bb5ff0d7122f94d110fa1010dab3a4394e9b7cda
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Per the resolution and discussion at [1], we've decided to simplify
the UA rules for pop-up attribute selectors, to capture *any*
`popup` attribute value. Additionally (and this part is tentative),
it looks like we'll likely resolve to put back `!important` on
display:none, so that `[popup] {display:flex}` can work.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: If3d52cce0c9cbd3c6134ff57836d65c93eb12c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3840811
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1037921}
NOKEYCHECK=True
GitOrigin-RevId: 3f17bf65ec57702a1ec74c58d03a8dd09f08f68b
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
There's been pushback [1] to the !important rule on display:none,
and more conversation about padding. This CL updates the behavior
to keep up with the conversation. It also un-breaks several demos
that were relying on the non-!important display:none rule.

[1] openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: Ia267bf54546e433436963ae4a89d7828181c7015
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3851364
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Reviewed-by: David Baron <dbaron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1038529}
NOKEYCHECK=True
GitOrigin-RevId: 9fa262ecbbae1b0adc8b0ee40915218a406da949
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
When a `<dialog>` is opened via `.showPopUp()`, it should have
a completely transparent ::backdrop. This CL fixes that by adding
a special case for dialog::open.

See also:
openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I444730c57c699f4af4a708cc316dd81aa7563669
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3933277
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1054947}
NOKEYCHECK=True
GitOrigin-RevId: 4c1740d3237061fd2c84bad3fe9179d3fa345e9d
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 21, 2022
…=testonly

Automatic update from web-platform-tests
Fix backdrop color for <dialog popup>

When a `<dialog>` is opened via `.showPopUp()`, it should have
a completely transparent ::backdrop. This CL fixes that by adding
a special case for dialog::open.

See also:
openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I444730c57c699f4af4a708cc316dd81aa7563669
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3933277
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1054947}

--

wpt-commits: 76ccc49778212f00dd246191bc2427e7ce33d0fe
wpt-pr: 36260
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Oct 26, 2022
…=testonly

Automatic update from web-platform-tests
Fix backdrop color for <dialog popup>

When a `<dialog>` is opened via `.showPopUp()`, it should have
a completely transparent ::backdrop. This CL fixes that by adding
a special case for dialog::open.

See also:
openui/open-ui#561 (comment)

Bug: 1307772
Change-Id: I444730c57c699f4af4a708cc316dd81aa7563669
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3933277
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1054947}

--

wpt-commits: 76ccc49778212f00dd246191bc2427e7ce33d0fe
wpt-pr: 36260
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
popover The Popover API
Projects
None yet
Development

No branches or pull requests