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

[selectors-4] Introduce :popover-open pseudo class #8637

Open
mfreed7 opened this issue Mar 23, 2023 · 20 comments
Open

[selectors-4] Introduce :popover-open pseudo class #8637

mfreed7 opened this issue Mar 23, 2023 · 20 comments

Comments

@mfreed7
Copy link
Contributor

mfreed7 commented Mar 23, 2023

We recently resolved to add general-purpose :open and :closed pseudo classes, which apply to "things that can open and close". While not defined in CSSWG, the stated intention was to include things like <details>, <select>, <dialog>, and Popover.

@jakearchibald recently raised the issue that in some cases, this is ambiguous, because there are multiple "ways" that something can be open or closed, and both might apply at the same time. Take, as an example, <details popover>, which can be "open" as a popover, but "closed" as a details, or vice versa. There's no way for these to be disambiguated on the basis of :open or :closed alone.

It is a bit unclear whether fullscreen elements should be included as "something that can open and close", but it's possible. That allows the possibility of a fullscreen <details> element which can be both open and closed. But fullscreen elements have a dedicated pseudo, :fullscreen, which allows one to disambiguate that state of openness vs. others.

It would seem that we need another such pseudo for popovers, because any element can also be a popover, and can therefore run into this dual-states problem. We discussed this in OpenUI, and proposed :popover as a pseudo class that applies only to popovers that are open (as a popover). That would allow developers to do:

details[popover]:closed:popover {
  /* styles for a non-expanded details that is open as a popover */
}
@dbaron
Copy link
Member

dbaron commented Mar 23, 2023

One thing I suggested in the OpenUI discussion is that :open and :closed could be for anything where the open/closed state is related to the element semantics, but that things like popover and fullscreen (which can be applied to any element) should have separate pseudo-classes to distinguish them and avoid this problem of unexpected intersections.

@mfreed7
Copy link
Contributor Author

mfreed7 commented Mar 23, 2023

One thing I suggested in the OpenUI discussion is that :open and :closed could be for anything where the open/closed state is related to the element semantics, but that things like popover and fullscreen (which can be applied to any element) should have separate pseudo-classes to distinguish them and avoid this problem of unexpected intersections.

This also seems like a reasonable suggestion. @jh3y do you know of any popover use cases that would be harmed by not having access to a :closed pseudo class? It seems like :not(:popover) would generally work instead, but just checking.

@jh3y
Copy link
Contributor

jh3y commented Mar 23, 2023

Generally up until now you'd likely do something like [popover]:not(:open) so don't see an issue. I remember a while back it was not(:top-layer) in a demo 👀

@tabatkins
Copy link
Member

:open and :closed could be for anything where the open/closed state is related to the element semantics, but that things like popover and fullscreen (which can be applied to any element) should have separate pseudo-classes to distinguish them

That distinction seems pretty reasonable to me, yeah.

@jakearchibald
Copy link
Contributor

jakearchibald commented Mar 27, 2023

@dbaron @tabatkins should :open apply to popovers as well as :popover, or is :popover instead of :open?

I think I'm leaning towards :open not applying to popover, as it also avoids difficult questions around :open and :closed applying at the same time, due to multiple definitions of open/closed.

So if an element type can open/close, then it uses :open and :closed, whereas concepts that apply to multiple element types, such as popovers and fullscreen, get their own pseudoclass.

@jh3y
Copy link
Contributor

jh3y commented Mar 27, 2023

My understanding was that :popover replaces :open to detect whether a popover is in the top layer.

The only issue I see is "Does :popover communicate that state well?". But, then I think it's likely fine as it kinda falls in with :modal/:fullscreen. Hopefully, it won't cause confusion.

@keithamus
Copy link
Member

Could all elements that are in the top-layer have a :top-layer pseudo? That would be applicable to dialogs, modals, and popovers, right? I suppose one counterpoint is that it does not clear up the ambiguity between an open dialog and an open dialog popover?

@jh3y
Copy link
Contributor

jh3y commented Mar 27, 2023

That's what it was at one point last year. There was a :top-layer pseudo for detecting when a popover was "popped" 😅

But, I think it was removed for naming reasons.

@mfreed7
Copy link
Contributor Author

mfreed7 commented Mar 27, 2023

So we might consider :overlay as a pseudo class for things in the top layer, given that we decided to call the CSS property overlay. But I think that's a question for a different issue.

I'm in favor of a) adding :popover for (only) popovers in the open state, and b) removing support for :open and :closed for popovers entirely. It feels like the definition for :open and :closed really refers specifically to specific elements that can open and closed, and not any element that can be made to be open or closed through a separate attribute or Javascript method. So that rules out popover and fullscreen.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 27, 2023
See [1] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover` which only applies to popovers in the open
state, but it does not yet remove `:open` and `:closed` support for
popovers. However, it does convert all of the popover WPTs to use
`:popover` instead of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637

Bug: 1307772
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
@jh3y
Copy link
Contributor

jh3y commented Mar 27, 2023

I still find :overlay to be an interesting choice that potentially gets muddled up with ::backdrop.

Sometimes I wonder if the appropriate pseudo is one we discussed before with :show/n/ing. Something like details is opened/closed. But, we have popovers that can be shown or not shown (:shown, :not(:shown)).

@mfreed7
Copy link
Contributor Author

mfreed7 commented Mar 27, 2023

Sometimes I wonder if the appropriate pseudo is one we discussed before with :show/n/ing. Something like details is opened/closed. But, we have popovers that can be shown or not shown (:shown, :not(:shown)).

I think the problem is that, due to the issues we've discussed above, it needs to be specific to the API. So it should have "popover" in the name. And akin to :fullscreen for the "fullscreen" API, it seems most parallel to use :popover. Another alternative is something like :popover-open but that's verbose and seemingly unnecessary. If the popover is "closed" but made visible via display:block or something, I'd say it really isn't a popover any longer, it's just an element, so it still makes sense for that not to match :popover.

josepharhar added a commit to josepharhar/html that referenced this issue Mar 27, 2023
This is currently under discussion here:
w3c/csswg-drafts#8637
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 27, 2023
See [1] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover` which only applies to popovers in the open
state, but it does not yet remove `:open` and `:closed` support for
popovers. However, it does convert all of the popover WPTs to use
`:popover` instead of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637

Bug: 1307772
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 27, 2023
See [1] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover` which only applies to popovers in the open
state, but it does not yet remove `:open` and `:closed` support for
popovers. However, it does convert all of the popover WPTs to use
`:popover` instead of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637

Bug: 1307772
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
@nt1m
Copy link
Member

nt1m commented Mar 29, 2023

I'm not convinced with the parallel with :fullscreen, an element with the popover attribute is always a popover even when closed, whereas it's not the case for fullscreen. Though you could arguably adjust the definition of what a popover is to exclude closed popovers.

Although given the alternatives would be:

  • :popover-open/closed - a bit verbose
  • don't allow details/select/ or whatever element which has an open state to match popover - a bit arbitrary
  • status quo - a bit confusing in those edge cases

:popover is probably fine.

@nt1m
Copy link
Member

nt1m commented Mar 29, 2023

If the popover is "closed" but made visible via display:block or something, I'd say it really isn't a popover any longer, it's just an element, so it still makes sense for that not to match :popover.

That's a good point.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [selectors-4] Introduce `:popover` pseudo class, and agreed to the following:

  • RESOLVED: Add :popover-open pseudoclass, undefined popoverness from :open
The full IRC log of that discussion <TabAtkins> chrishtr: we previous resolved to add :open/:closed to apply when popover is opened or closed.
<TabAtkins> chrishtr: <details> has an open/closed concept independent of in the top layer, but it can also be a popover.
<TabAtkins> chrishtr: So we need two axises..
<TabAtkins> chrishtr: Proposal is we scope :open/:closed to an element's own notion of being open or closed. New pseudo :popover that's whether it's in the top layer or not.
<lea> q+
<TabAtkins> plinss: Nit: whether it's "popped over", not necessarily "in the top layer"
<TabAtkins> chrishtr: agreed
<TabAtkins> lea: What does the popover pseudo match, exactly?
<TabAtkins> ???: matches popovers in the open state
<TabAtkins> lea: So what would :popover:closed ever match?
<Rossen_> ack lea
<TabAtkins> chrishtr: A closed <details> element that's popover
<TabAtkins> dbaron: Idea is that it matches popovers who are currently "popover open"
<TabAtkins> dbaron: Open and close are reserved for elements that have their own state.
<fantasai> :pop-open / :pop-closed ?
<Rossen_> ack dbaron
<TabAtkins> lea: I think open/closed is fine, but :popover sounds like it matches any popover element, aka with a popover attribute regardless of whether it's popping or not
<lea> q+
<TabAtkins> Rossen_: So :popover-open sounds like a refinement?
<TabAtkins> chrishtr: Yes, I think :popover-open is descriptive, a little longer but useful
<Rossen_> ack lea
<TabAtkins> lea: I think we had some issues about defining top layer with CSS, want to make sure this doesn't become an obstacle for that
<TabAtkins> plinss: Yes, this just reflects the popover state, not what layer it's in.
<flackr> +1
<chrishtr> +1
<TabAtkins> Rossen_: Seeing +1s in the chat. Can we reoslve to add, with the changed name :popover-open?
<TabAtkins> plinss: Slight hesitancy due to name getting kinda long.
<TabAtkins> plinss: Maybe worth bikeshedding the whole feature name? "popover" is a little weird.
<dbaron> popover is a weird name because calling it popup turned out to not be web-compatible
<TabAtkins> fantasai: Suggest :pop-open
<TabAtkins> dbaron: [reads comment from irc]
<TabAtkins> Rossen_: So :popover-open is proposed resolution
<TabAtkins> RESOLVED: Add :popover-open pseudoclass, undefined popoverness from :open

@nt1m
Copy link
Member

nt1m commented Mar 29, 2023

Given verbosity was mentioned in the meeting, another possibility is :showing since the name of the state is "popover showing" in the HTML spec. Though the overly generic name does scare me a bit.

@chrishtr
Copy link
Contributor

Given verbosity was mentioned in the meeting, another possibility is :showing since the name of the state is "popover showing" in the HTML spec. Though the overly generic name does scare me a bit.

To repeat something I mentioned during the meeting, I think :popover-open is good because it's very clear to developers what it means and refers directly to the name of the attribute and the fact that the popover is open. You can literally read it and see what it means. I also like that better than changing the form of the word to popped or similar. It is "popped" of course, but that's a bit too English-language-native-speaker ambiguous to me.

@dbaron
Copy link
Member

dbaron commented Mar 29, 2023

A slight correction to what I said in the telecon: we ended up with the name popover after a series of two renaming issues (following the earlier change from element to attribute), but it was the first of them (popup to popUp) that was about web-compatibility. The second (to popover) was about confusion with other platform features that use the name popup to refer to things that open separate OS-level windows (or at least browser-level tabs). See openui/open-ui#546 and openui/open-ui#627 .

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 29, 2023
See [1] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover` which only applies to popovers in the open
state, but it does not yet remove `:open` and `:closed` support for
popovers. However, it does convert all of the popover WPTs to use
`:popover` instead of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637

Bug: 1307772
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 31, 2023
See [1]/[2] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover-open` which only applies to popovers in the open
state, and it removes `:open` and `:closed` support for popovers. It also converts all of the popover WPTs to use `:popover-open` instead
of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637
[2] whatwg/html#9077

Bug: 1307772
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 31, 2023
See [1]/[2] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover-open` which only applies to popovers in the open
state, and it removes `:open` and `:closed` support for popovers. It also converts all of the popover WPTs to use `:popover-open` instead
of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637
[2] whatwg/html#9077

Bug: 1307772
Fixed: 1429670
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 31, 2023
See [1]/[2] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover-open` which only applies to popovers in the open
state, and it removes `:open` and `:closed` support for popovers. It also converts all of the popover WPTs to use `:popover-open` instead
of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637
[2] whatwg/html#9077

Bug: 1307772
Fixed: 1429670
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4373888
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1124869}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 31, 2023
See [1]/[2] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover-open` which only applies to popovers in the open
state, and it removes `:open` and `:closed` support for popovers. It also converts all of the popover WPTs to use `:popover-open` instead
of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637
[2] whatwg/html#9077

Bug: 1307772
Fixed: 1429670
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4373888
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1124869}
annevk pushed a commit to whatwg/html that referenced this issue Apr 3, 2023
@annevk
Copy link
Member

annevk commented Apr 3, 2023

HTML side of this landed: whatwg/html#9077.

cookiecrook pushed a commit to cookiecrook/wpt that referenced this issue Apr 8, 2023
See [1]/[2] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover-open` which only applies to popovers in the open
state, and it removes `:open` and `:closed` support for popovers. It also converts all of the popover WPTs to use `:popover-open` instead
of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637
[2] whatwg/html#9077

Bug: 1307772
Fixed: 1429670
Change-Id: I8d840512166ccdb5d5c8abbb7192bbce7177ee88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4373888
Reviewed-by: Joey Arhar <jarhar@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1124869}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 15, 2023
…opovers, a=testonly

Automatic update from web-platform-tests
Convert `:open` to `:popover-open` for popovers

See [1]/[2] for more context, but there are cases where `:open` is
ambiguous for popovers. If multiple elements support `:open/:closed`,
and [popover] can be applied to any of them, there are situations
where an element is both open and closed. For example,
`<details popover>` can be closed as a details element and open as
a popover, which makes it match both `:open` and `:closed`. It seems
that really `:open` and `:closed` should match *elements* that can
open and close, and not things that can be made to open or close
via an attribute or other mechanism such as JS.

This CL adds `:popover-open` which only applies to popovers in the open
state, and it removes `:open` and `:closed` support for popovers. It also converts all of the popover WPTs to use `:popover-open` instead
of either `:open` or `:closed`.

[1] w3c/csswg-drafts#8637
[2] whatwg/html#9077

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

--

wpt-commits: e68cb913b7cd3002609729bd2bde85b24ecaff39
wpt-pr: 39222
@destradaxcheckapp
Copy link

Hi @mfreed7

I have a working scenario where I need this pseudo class and is also related to your comment about the toggle events not being cancelable when hiding popovers.

I created a React component to encapsulate a popover and I need it to be descriptively controlled instead of imperatively.

My Popover component receives an open property, renders a

<dialog popover="auto" ref={dialogRef}>{children}</dialog>

and shows (dialogRef.showPopover()) or hides (dialogRef.hidePopover()) the popover based on the open property.

  1. I cannot intercept and cancel (event.preventDefault()) the toggle event to rely on the consumer of my component changing the open property; the most I can do is let the consumer know that the popover was closed.

  2. Popovers can be light-dismissed by a user action (clicking outside or pressing the Escape key), so I need to first check if the popover is still visible before attempting to call .hidePopover();, because trying to hide an already-hidden popover generates a warning.

I can easily achieve this with const isVisible = dialogRef.matches(':popover-open');, and it works like a charm, but testing environments still don't recognize this :popover-open pseudo class so I'm forced to check the display value instead:

const isVisible = getComputedStyle(dialogRef).getPropertyValue('display') === 'block'

I see that, as @annevk mentioned, this was already merged to the HTML side, but I can't find if there's been any update on this side

@mfreed7
Copy link
Contributor Author

mfreed7 commented Apr 5, 2024

  1. I cannot intercept and cancel (event.preventDefault()) the toggle event to rely on the consumer of my component changing the open property; the most I can do is let the consumer know that the popover was closed.

Right, this is by design, to avoid a slew of footguns. Sounds like you have a way to make your application work, which is good.

  1. Popovers can be light-dismissed by a user action (clicking outside or pressing the Escape key), so I need to first check if the popover is still visible before attempting to call .hidePopover();, because trying to hide an already-hidden popover generates a warning.

You could also use the toggle event to keep track of the state, if you like.

I can easily achieve this with const isVisible = dialogRef.matches(':popover-open');, and it works like a charm, but testing environments still don't recognize this :popover-open pseudo class

It's landed in browsers, so I'm not sure why testing environments wouldn't have it.

const isVisible = getComputedStyle(dialogRef).getPropertyValue('display') === 'block'

Note that this is a bit brittle, since you can set display:whatever on the dialog manually.

I see that, as @annevk mentioned, this was already merged to the HTML side, but I can't find if there's been any update on this side

Right, this is part of the HTML spec, but doesn't appear yet in the selectors spec. @tabatkins @fantasai is it easy (and appropriate) to get :popover-open added to the CSS spec?

@mfreed7 mfreed7 changed the title [selectors-4] Introduce :popover pseudo class [selectors-4] Introduce :popover-open pseudo class Apr 5, 2024
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