-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Theme: Include density documentation in README.md #73976
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
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
In earlier iterations, this sentence was specifically talking about density, but was later generalized since all of the values inherit from the closest ThemeProvider.
|
Flaky tests detected in 3f3d40a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/20173375481
|
juanfra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ✅
packages/theme/README.md
Outdated
| - `'default'`: Standard spacing for general use | ||
| - `'compact'`: Reduced spacing for information-dense interfaces like data tables or dashboards | ||
| - `'comfortable'`: Increased spacing for focused experiences like modals, dialogs, or full-screen settings panels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: we might want to end these list items with a full stop, to stay consistent with the style used elsewhere.
| - `'default'`: Standard spacing for general use | |
| - `'compact'`: Reduced spacing for information-dense interfaces like data tables or dashboards | |
| - `'comfortable'`: Increased spacing for focused experiences like modals, dialogs, or full-screen settings panels | |
| - `'default'`: Standard spacing for general use. | |
| - `'compact'`: Reduced spacing for information-dense interfaces like data tables or dashboards. | |
| - `'comfortable'`: Increased spacing for focused experiences like modals, dialogs, or full-screen settings panels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify what you mean by "elsewhere" ? We're not especially consistent in this file 😅 The original proposed content follows from the color prop listing above, though there's also lists elsewhere in the file (e.g. "Building") that include periods.
I guess it's a broader question of whether we have content guidelines for this and whether they're discoverable enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's some guidance in the documentation copy guide for bulleted lists, though it's more about consistency and less about prescribing dots or not dots.
In any case, I pushed changes in b3bb942 to make it consistent across the files: Dots at the end of list items, and no dots within tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point 😅
Before leaving the nit I just checked a couple of other readme's as a reference, like dataviews, which felt fairly up to date and uses full stops in lists.
No strong feelings either way though. But indeed, maybe it’s a sign we could use a lightweight guideline at some point (updated after you shared the copy guide)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also have a preference for full stops at the end of bullet points 🙂
What?
Updates
@wordpress/theme'sREADME.mdto describe the behavior of thedensityprop.Why?
colorThemeProvidercomponent and its available featuresTesting Instructions
Review proposed documentation for accuracy.