-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 some documentation about the Styles UI components #49720
Conversation
c8e596d
to
c46616a
Compare
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!
✅ New UI components section reads well and makes sense
Tiny nit: The indentation for the value
prop example contains a mixture of spaces and tabs.
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.
This all reads nicely to me, too, LGTM aside from the whitespace issue that Aaron raised. Thanks for updating the docs!
Just added a comment to connect the discussion you started on the child layout controls about the default controls values.
- `defaultControls`: The default controls are the controls that are used by default to render the UI. They are used to provide the UI components with the necessary information to render the UI. An example value for the default controls for the `ColorPanel` component is: | ||
|
||
```js | ||
{ | ||
background: true, | ||
text: true, | ||
link: true, | ||
}, | ||
``` |
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.
Thanks for starting the discussion about what the defaults for each of the panels should be over in #49389 (comment).
Something I was wondering about from that comment — should the defaults across the components be defaulting to false
to follow the behaviour in block.json
where we explicitly flag which ones we reveal? I know that in the site editor they're all displayed by default, but at the component level, I wasn't sure which idea would be preferable. Some example ideas:
- The component as it lives in the block editor package specifies the defaults across the board (e.g. most are hidden by default, except for child layout controls). It is then up to other use cases (e.g. global styles) to specify if they'd like to reveal more controls by default. Or:
- At the component level, we default to displaying everything by default. It is then up to the consumer to be more restrictive if it so wishes (e.g. the
block.json
behaviour).
What do you think?
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.
My strong opinion is that the behavior should be the same.
My other opinion that is a bit less strong is that there's also a third option which is for us to be opinionated about what the defaults should be, that way consumers of the component (think of a component as its own unit, independent of the existing usage) itself shouldn't have to provide that prop or a big object to actually choose what to show, they should have sane defaults. And since we control what is shown or hidden on Global Styles (as opposed to the blocks), I don't see why global styles shouldn't be using these sane defaults. (at least at the root level, as there's an argument that block level should also follow the block.json config)
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.
Anyway, let's continue this discussion with Isabel in the other thread and update the documentation when we find a solution.
Co-authored-by: Aaron Robertshaw <[email protected]>
Flaky tests detected in f026f53. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4676749929
|
WHAT
🤖 Generated by Copilot at c8e596d
This pull request improves the documentation of the global styles system in the block editor in
./packages/block-editor/src/components/global-styles/README.md
.🤖 Generated by Copilot at c8e596d
WHY
Hopefully this sets some guidelines for folks as they touch and evolve these components and also helps people understand what they should do build new similar components.
Todo: As a follow-up, I think we should consider adding Storybook stories for all of these components.