Global Styles: Heading element UI controls #42176
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +2.09 kB (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
|
I'd like to know if this design is acceptable, and then I can continue with the typography selection screen. |
|
Thanks for getting started on this @carolinan. The design looks good to me. We can always iterate on it later if we want to, so I wouldn't let that stop you proceeding. |
aristath
force-pushed
the
try/heading-color-ui
branch
from
5a88076 to
84c118e
Compare
|
Should size and line height be available for the "all headings" option? Heading levels are intended to have different sizes. |
carolinan
changed the title
|
I don't think this is correct:
What is the expected result? |
carolinan
added
the
Global Styles
Line height doesn't seem to be a problem, but I don't think size should be there. |
I think this is correct - it should be possible to set the heading element globally, and for individual blocks. This PR is looking great. Is there much more to do? |
|
I hid the font size option for the "all headings" option. I wonder if it should also be hidden for links? |
scruffian
reviewed
Aug 2, 2022
| const colorsPerOrigin = useColorsPerOrigin( name ); | ||
| const gradientsPerOrigin = useGradientsPerOrigin( name ); | ||
|
|
||
| const hasTextColor = |
There was a problem hiding this comment.
All of these supports (hasTextColor, hasBackgroundColor, hasGradientColor) are quite verbose, and we probably have this same logic elsewhere. Can we extract this to some kind of hook or shared function? That could be a follow-up PR....
There was a problem hiding this comment.
Yes I fully agree.
scruffian
reviewed
Aug 2, 2022
| > | ||
| <ToggleGroupControlOption | ||
| value="heading" | ||
| label={ __( 'All' ) } |
There was a problem hiding this comment.
Does this need a translators comment?
scruffian
approved these changes
Aug 2, 2022
57 tasks
|
Just stumbled upon this PR. How do we decide which context: (global, block, element...) supports "heading colors"? It seems that right now this button randomly shows up for most blocks that support colors? Do we want opt-in/opt-out... |
|
We don't decide, that's what I was trying to get to with this comment, but I was too vague:
I was first thinking it could be opt-in or an allowed list, but would that not affect third party blocks negatively? For example, the verse block has a heading color option but it can't have headings. |
|
Edit: Actually the verse can have a heading, the block has the "edit as HTML" option. |
|
I think it should probably follow the same approach as other elements like "button". I think it relies on |
Agreed. There is no Heading (element) block support equivalent to |
This was referenced Mar 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Add heading elements to the list of elements that can be customized in the Global Styles UI.
Fixes #41974
Why?
To allow customizing heading elements and not only heading blocks.
How?
Adds a new heading color screen to the global styles sidebar.
Testing Instructions
Screenshots or screencast
Updated August 1:
The label above the buttons with the heading level selection now says "Select heading level".
The heading above the text color and background color options indicates which heading the style will be applied to.
With H2 selected:
Screen.Recording.2022-08-02.at.06.40.01.mov