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.

Sign up for GitHub

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

Jump to bottom

[selectors-4] Define a number of new pseudo-classes for matching the state of media elements. #6219

Merged
merged 3 commits into from Apr 22, 2021
Merged

[selectors-4] Define a number of new pseudo-classes for matching the state of media elements. #6219

merged 3 commits into from Apr 22, 2021

Conversation

hober
Copy link
Member

@hober hober commented Apr 16, 2021

We resolved to add these in Toronto in June 2019.

This is a first draft; I'm sure the text could use some work before merging.

Fixes #3821.
Fixes #3933.

@hober hober requested a review from tabatkins April 16, 2021 20:01
@hober hober self-assigned this Apr 16, 2021
@hober
Copy link
Member Author

hober commented Apr 16, 2021

cc @jernoble

tabatkins
tabatkins requested changes Apr 16, 2021
View reviewed changes
Copy link
Member

@tabatkins tabatkins left a comment

Choose a reason for hiding this comment

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

r+ after changes

selectors-4/Overview.bs Outdated
@@ -36,6 +36,14 @@ spec:css-pseudo-4; type:selector;
text: ::first-line
text: ::first-letter
</pre>
<pre class="anchors">
urlPrefix: https://html.spec.whatwg.org/multipage/; spec: HTML
Copy link
Member

Choose a reason for hiding this comment

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

All of these definitions already exist in HTML and are reasonably marked up, they're just not exported.

Please swap to specifying these in link-defaults, and then probably ask HTML to export them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 59f2bc1. Export of those terms requested in whatwg/html#6603.

selectors-4/Overview.bs Outdated
@@ -2403,23 +2411,63 @@ Resource State Pseudos</h2>
particularly images/videos,
and allow authors to select them based on some quality of their state.

<h3 id="video-state">
Video/Audio Play State: the '':playing'' and '':paused'' pseudo-classes</h3>
<h3 id="video-state"> <!-- keeping this ID because Cool URIs Don't Change -->
Copy link
Member

Choose a reason for hiding this comment

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

Good point, but no need to explain your ID choices. ^_^

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed in 6599d0b.

selectors-4/Overview.bs Outdated
(This includes both an explicit “paused” state,
and other non-playing states like “loaded, hasn't been activated yet”, etc.)

The <dfn>:seeking</dfn> pseudo-class represents an element
Copy link
Member

Choose a reason for hiding this comment

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

This spec is tab-indented, please stick with that consistently.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 6599d0b.

@hober hober mentioned this pull request Apr 21, 2021
Merged
@hober hober requested a review from tabatkins April 21, 2021 22:47
@tabatkins
Copy link
Member

Overall looks good.

Only comment left is that maybe we also want a generic :media pseudo-class that matches elements capable of matching any of these pseudos? I suggest this because of the annoyingness of things like :not(:muted) causing overselection of everything else on the page; instead, you could write :media:not(:muted) and get what you meant.

(I could see us instead recasting the whole thing into a :media() that accepts keywords for all the states, but we'd have to add negation to the states as well, and we already have :playing and :paused, so meh.)

@tabatkins
Copy link
Member

r+ for merging what's currently here; we can either address the :media question separately or fold it into this, whichever you prefer

@hober
Copy link
Member Author

hober commented Apr 22, 2021 •
edited

r+ for merging what's currently here; we can either address the :media question separately or fold it into this, whichever you prefer

Let's raise it as a separate / followup issue, since we don't have a WG resolution for it. (Not that I think it'll be hard to get one; you make a compelling argument.) Wanna file the followup issue?

Also, I'm not pressing the "squash and merge" button since I'm not an editor. Would you care to pull the trigger, @tabatkins?

@tabatkins tabatkins merged commit 95d3a2d into w3c:main Apr 22, 2021
@hober hober deleted the media-state-pseudos branch April 22, 2021 17:38
@tabatkins tabatkins mentioned this pull request Apr 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tabatkins tabatkins tabatkins approved these changes

Assignees

@hober hober

Labels
Needs Review of Proposed Text Needs Testcase (WPT) selectors-4 Current Work
Projects
None yet
Milestone
No milestone
2 participants
@hober @tabatkins