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

Add note about parens in matchMedia #37504

Merged
merged 11 commits into from
Jan 23, 2025
Merged

Conversation

chriskirknielsen
Copy link
Contributor

Description

Provides a note about explicit requirements for a matchMedia condition to be wrapped in parentheses.

Motivation

I was bitten by this and didn't find an explicit callout to help developers who might have been in my situation and could not understand why their code wouldn't work.

I was bitten by this and didn't find an explicit callout to help developers who might have been in my situation and could not understand why their code wouldn't work.
@chriskirknielsen chriskirknielsen requested a review from a team as a code owner January 5, 2025 18:18
@chriskirknielsen chriskirknielsen requested review from wbamberg and removed request for a team January 5, 2025 18:18
@github-actions github-actions bot added Content:WebAPI Web API docs size/xs [PR only] 0-5 LoC changed labels Jan 5, 2025
Copy link
Contributor

github-actions bot commented Jan 5, 2025

Preview URLs

(comment last updated: 2025-01-23 16:30:40)

@Josh-Cena
Copy link
Member

Josh-Cena commented Jan 5, 2025

This is misleading though. Anything that needs parentheses in <media-query-list> needs parentheses here; anything that doesn't, doesn't. For example print is just print not (print).

@chriskirknielsen
Copy link
Contributor Author

chriskirknielsen commented Jan 5, 2025

@Josh-Cena That's a good point! Do you know what would be a better verbiage to communicate this?

@chriskirknielsen
Copy link
Contributor Author

Taking some notes from the CSS media page:

Note

Just like in CSS, any media feature must be wrapped in parentheses inside the expression. For example: matchMedia('(max-width: 600px)') will work, whereas matchMedia('max-width: 600px') will not. Keywords for media types (all, print, screen) and logical operators (and, or, not, only) do not need to be wrapped in parentheses.

chriskirknielsen and others added 4 commits January 22, 2025 00:48
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

Thanks for the update @chriskirknielsen ! I think the wording here is fine but it doesn't need to be a note.

files/en-us/web/api/window/matchmedia/index.md Outdated Show resolved Hide resolved
@chriskirknielsen
Copy link
Contributor Author

Thank you @wbamberg, much appreciated!

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Member

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

This LGTM. Thanks!

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@wbamberg wbamberg merged commit 7eb271b into mdn:main Jan 23, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/xs [PR only] 0-5 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants