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

CSS Counters Module #29669

Merged
merged 27 commits into from
Oct 24, 2023
Merged

CSS Counters Module #29669

merged 27 commits into from
Oct 24, 2023

Conversation

estelle
Copy link
Member

@estelle estelle commented Oct 16, 2023

  • rewrite CSS counters module
  • Update system descriptor
    • remove the "if this works in your browser" as it's supported everywhere
    • simplified some explanations
    • expanded on some to make it more understandable
  • Updated @counter-styles to remove history
  • fixes CSS Modules: Counter Styles openwebdocs/project#181

@estelle estelle requested a review from a team as a code owner October 16, 2023 19:54
@estelle estelle requested review from dipikabh and removed request for a team October 16, 2023 19:54
@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Oct 16, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2023

Preview URLs

External URLs (1)

URL: /en-US/docs/Web/CSS/CSS_counter_styles
Title: CSS counter styles

(comment last updated: 2023-10-24 01:39:00)

@estelle estelle changed the title DRAFT: CSS Counters Module CSS Counters Module Oct 17, 2023
@estelle estelle marked this pull request as draft October 17, 2023 23:17
@estelle estelle changed the title CSS Counters Module DRAFT: CSS Counters Module Oct 17, 2023
@estelle estelle changed the title DRAFT: CSS Counters Module CSS Counters Module Oct 17, 2023
@estelle estelle marked this pull request as ready for review October 17, 2023 23:33
@dipikabh dipikabh self-assigned this Oct 18, 2023
Copy link
Contributor

@dipikabh dipikabh 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 updating these pages, @estelle! 🙌

I've added my edit suggestions as well as some questions.

files/en-us/web/css/css_counter_styles/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_counter_styles/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_counter_styles/index.md Outdated Show resolved Hide resolved
Comment on lines 26 to 27
- [`<counter-style>`](/en-US/docs/Web/CSS/@counter-style#counter-style-name)
- [`<counter-style-name>`](/en-US/docs/Web/CSS/@counter-style#counter-style-name)
Copy link
Contributor

Choose a reason for hiding this comment

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

  • These two types do not have corresponding pages yet, correct?
  • Also note that they both have the same link
  • The Formal syntax section and the spec only refer to <counter-style-name> out of the two. Is <counter-style> needed?
  • Do we need to add <declaration-list> since it is in the Formal syntax?

Copy link
Member Author

Choose a reason for hiding this comment

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

declaration list is not defined by this spec, but i did define it in one sentence on the page that uses it.
counter-style removed. good catch
counter style name does not have its own page.

files/en-us/web/css/css_counter_styles/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/@counter-style/system/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/@counter-style/system/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/@counter-style/system/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/@counter-style/system/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/@counter-style/system/index.md Outdated Show resolved Hide resolved
Co-authored-by: Dipika Bhattacharya <dipika@foss-community.org>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Dipika Bhattacharya <dipika@foss-community.org>
Co-authored-by: Dipika Bhattacharya <dipika@foss-community.org>
@estelle
Copy link
Member Author

estelle commented Oct 20, 2023

Thanks @dipikabh for the excellent review. I think I captured all the issues.

@estelle estelle requested a review from dipikabh October 20, 2023 00:10
Copy link
Contributor

@dipikabh dipikabh 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 updates and clarifying notes. I've pointed a few last nits.

files/en-us/web/css/css_counter_styles/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/css_counter_styles/index.md Outdated Show resolved Hide resolved
{{Compat}}
- [CSS generated content](/en-US/docs/Web/CSS/CSS_generated_content) module
- [CSS pseudo-elements](/en-US/docs/Web/CSS/CSS_pseudo-elements) module
- [Ready-made counter styles](https://w3c.github.io/predefined-counter-styles/) via W3C
Copy link
Contributor

Choose a reason for hiding this comment

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

The document, the Editor's Draft, has a date at the top. I think it might be good to note the year for this version of the document.

files/en-us/web/css/@counter-style/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/@counter-style/system/index.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

{{ EmbedLiveSample('Extends_example') }} will need to be updated to
{{ EmbedLiveSample('Extending_a_counter') }}

Co-authored-by: Dipika Bhattacharya <dipika@foss-community.org>
@estelle estelle requested a review from dipikabh October 23, 2023 22:04
@estelle
Copy link
Member Author

estelle commented Oct 23, 2023

@dipikabh Thanks for the review. I think we're good to go.

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

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

checking if this can resolve the lint error

files/en-us/web/css/css_counter_styles/index.md Outdated Show resolved Hide resolved
@dipikabh dipikabh merged commit 7297d9a into mdn:main Oct 24, 2023
6 checks passed
@estelle estelle deleted the counters branch December 5, 2023 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSS Modules: Counter Styles
2 participants