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

CSS module: Conditional rules #31387

Merged
merged 26 commits into from
Jan 9, 2024
Merged

CSS module: Conditional rules #31387

merged 26 commits into from
Jan 9, 2024

Conversation

estelle
Copy link
Member

@estelle estelle commented Dec 29, 2023

New (rewrite) module landing page.
Rewrite the Feature Queries guide.

@estelle estelle requested a review from a team as a code owner December 29, 2023 08:40
@estelle estelle requested review from chrisdavidmills and removed request for a team December 29, 2023 08:40
@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Dec 29, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 29, 2023 •
edited
Loading

Preview URLs

Flaws (2)

URL: /en-US/docs/Web/CSS/CSS_conditional_rules
Title: CSS conditional rules
Flaw count: 1

  • macros:
    • wrong xref macro used (consider changing which macro you use)

URL: /en-US/docs/Web/CSS/CSS_conditional_rules/Using_feature_queries
Title: Using feature queries
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/docs/Web/CSS/CSS_colors/Relative_colors

(comment last updated: 2024-01-09 09:16:22)

chrisdavidmills
chrisdavidmills requested changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

@estelle more great work, thanks for this. I've left you some comments and suggestions, but nothing too serious.

files/en-us/web/css/css_conditional_rules/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md Show resolved Hide resolved
@estelle estelle mentioned this pull request Jan 3, 2024
Merged
chrisdavidmills
chrisdavidmills requested changes Jan 3, 2024
View reviewed changes
Copy link
Contributor

@chrisdavidmills chrisdavidmills 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 all the additional work on this one, @estelle; this is now looking much better.

I had a few additional comments on the new text you added, and I think the examples still need some work. Much closer now, however.

files/en-us/web/css/css_conditional_rules/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md Outdated Show resolved Hide resolved
@estelle
Copy link
Member Author

estelle commented Jan 3, 2024

Thanks @chrisdavidmills !

chrisdavidmills and others added 2 commits January 4, 2024 09:02
@chrisdavidmills @github-actions
Update files/en-us/web/css/css_conditional_rules/using_feature_querie…
83edd3c 
…s/index.md

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@chrisdavidmills
Merge branch 'main' into conditional
c521fbd
chrisdavidmills
chrisdavidmills requested changes Jan 4, 2024
View reviewed changes
Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

@estelle IMO, the article wording is perfect now. I just made a small tweak to the AccentColor example as some of the wording didn't seem to reflect what the example was doing. You can check my change in df55654 to make sure you are happy with it.

I still have concerns over the embedded GH examples though — it looks like you've not addressed these comments:

@estelle
Copy link
Member Author

estelle commented Jan 4, 2024

It would help if i saved files before committing them ;)
Fixed in mdn/css-examples#168

@chrisdavidmills
Copy link
Contributor

@estelle inching closer. The example at https://mdn.github.io/css-examples/feature-queries/simple.html (the one embedded below the paragraph that starts "As another example, if you want to check if a browser supports...") is still using red to indicate support.

@estelle
Copy link
Member Author

estelle commented Jan 9, 2024

The files in other repos have been updated.

chrisdavidmills
chrisdavidmills approved these changes Jan 9, 2024
View reviewed changes
Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

@estelle perfect, thanks for all the great work here. I'll get this merged.

@chrisdavidmills chrisdavidmills merged commit f11dbc3 into mdn:main Jan 9, 2024
7 checks passed
dipikabh pushed a commit to dipikabh/content that referenced this pull request Jan 17, 2024
@estelle @github-actions
@chrisdavidmills @dipikabh
CSS module: Conditional rules (mdn#31387)
68331fe 
* First pass: rewrite mod landing page

* add note about non-implemented at-rules per generated content example

* functions, terms, guide outline

* files/en-us/web/css/css_conditional_rules/using_feature_queries

* spec urls, terms, guides, etc

* rewrite FQ guide

* Apply suggestions from code review

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

* lint kinda works

* last links for module page

* terms

* Apply suggestions from code review

Co-authored-by: Chris Mills <[email protected]>

* Update files/en-us/web/css/css_conditional_rules/index.md

* Update files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md

Co-authored-by: Chris Mills <[email protected]>

* fix typos

* comment before each code example explaining what version of a support query it is

* grammar

* add selector example

* Apply suggestions from code review

Co-authored-by: Chris Mills <[email protected]>

* missing period

* Small tweaks to AccentColor example

* Update files/en-us/web/css/css_conditional_rules/using_feature_queries/index.md

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: Chris Mills <[email protected]>
@estelle estelle deleted the conditional branch March 26, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@chrisdavidmills chrisdavidmills chrisdavidmills approved these changes

Assignees
No one assigned
Labels
Content:CSS Cascading Style Sheets docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@estelle @chrisdavidmills