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

Rename :open to :popover-open and remove :closed #9077

Merged
merged 2 commits into from
Apr 3, 2023

Conversation

josepharhar
Copy link
Contributor

@josepharhar josepharhar commented Mar 27, 2023

This is currently under discussion here:
w3c/csswg-drafts#8637
@annevk annevk added needs implementer interest Moving the issue forward requires implementers to express interest topic: selectors topic: popover The popover attribute and friends labels Mar 28, 2023
@annevk
Copy link
Member

annevk commented Mar 29, 2023

I guess we're largely waiting for a CSS WG resolution here?

cc @whatwg/css @nt1m @jakearchibald

@jakearchibald
Copy link
Contributor

That's my understanding, yeah.

@josepharhar
Copy link
Contributor Author

The CSSWG just resolved to make it :popover-open: w3c/csswg-drafts#8637 (comment)
I updated this PR

@josepharhar josepharhar changed the title Rename :open to :popover and remove :closed Rename :open to :popover-open and remove :closed Mar 29, 2023
@annevk annevk requested a review from nt1m March 30, 2023 06:32
@nt1m
Copy link
Member

nt1m commented Mar 30, 2023

Looks good to me.

@annevk annevk removed the needs implementer interest Moving the issue forward requires implementers to express interest label Mar 31, 2023
@annevk
Copy link
Member

annevk commented Mar 31, 2023

@josepharhar can you complete the checklist in OP? You can consider WebKit to be interested. Thanks!

@zcorpan
Copy link
Member

zcorpan commented Mar 31, 2023

cc @emilio @cathiechen

@josepharhar
Copy link
Contributor Author

@josepharhar can you complete the checklist in OP? You can consider WebKit to be interested. Thanks!

Done

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also file an MDN issue. I'll merge this once the tests have landed.

@josepharhar
Copy link
Contributor Author

Please also file an MDN issue. I'll merge this once the tests have landed.

There isn't an MDN page for popover yet though, is there? Is there an existing issue for popover or a draft of an MDN article...?

@nt1m
Copy link
Member

nt1m commented Mar 31, 2023

(Small nit: can you rename the Chromium PR & WPT PR to use :popover-open ?)

@josepharhar
Copy link
Contributor Author

(Small nit: can you rename the Chromium PR & WPT PR to use :popover-open ?)

Mason is on it as we speak

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request 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 pull request 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 pull request 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1124869}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1124869}
@mfreed7
Copy link
Contributor

mfreed7 commented Mar 31, 2023

(Small nit: can you rename the Chromium PR & WPT PR to use :popover-open ?)

Done, and landed. 😄

cookiecrook pushed a commit to cookiecrook/wpt that referenced this pull request 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1124869}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 13, 2023
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request 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 <[email protected]>
Auto-Submit: Mason Freed <[email protected]>
Commit-Queue: Mason Freed <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1124869}

--

wpt-commits: e68cb913b7cd3002609729bd2bde85b24ecaff39
wpt-pr: 39222
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 18, 2023
luwes added a commit to luwes/selectlist-polyfill that referenced this pull request Jun 17, 2023
Loirooriol pushed a commit to Loirooriol/servo that referenced this pull request Nov 17, 2023
Loirooriol pushed a commit to Loirooriol/servo that referenced this pull request Nov 17, 2023
Loirooriol pushed a commit to Loirooriol/servo that referenced this pull request Nov 18, 2023
Loirooriol pushed a commit to Loirooriol/servo that referenced this pull request Nov 18, 2023
Loirooriol pushed a commit to Loirooriol/servo that referenced this pull request Nov 18, 2023
Loirooriol pushed a commit to Loirooriol/servo that referenced this pull request Nov 20, 2023
Loirooriol pushed a commit to Loirooriol/servo that referenced this pull request Nov 20, 2023
mrobinson pushed a commit to servo/servo that referenced this pull request Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: popover The popover attribute and friends topic: selectors
Development

Successfully merging this pull request may close these issues.

6 participants