Global Styles: Caption element UI controls for color and typography #49141
Conversation
MaggieCabrera
added
the
Global Styles
MaggieCabrera
requested review from
scruffian,
richtabor,
talldan,
getdave,
glendaviesnz,
tellthemachines,
jeryj,
carolinan,
aaronrobertshaw,
ajlende,
andrewserong and
draganescu
|
Size Change: +817 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 637af78. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4467213059
|
|
if I understood this comment correctly, #42176 (comment) |
Good point! thanks for the links and the example for the headings. I followed your lead but it's looking like the blocks are not supporting the element. Maybe they can only be changed globally and not on a block by block basis, so I'm going to investigate that. If that's the case then we should follow up adding support on the specific blocks, but now the controls should not be showing for blocks that definitely don't have captions. |
There was a problem hiding this comment.
what happened to this file?
There was a problem hiding this comment.
the two files are incredibly similar and can be refactored into one, so that's what I did in packages/edit-site/src/components/global-styles/screen-color-element.js. The rest of the elements were a bit more different, but in the future they could probably be abstracted too.
There was a problem hiding this comment.
Could we do this in another PR? 🙏🏻 Just moving the code around?
There was a problem hiding this comment.
that's what I did on the original PR and was encouraged to abstract it like this! :D
| @@ -211,7 +216,11 @@ function ContextScreens( { name, parentMenu = '', variation = '' } ) { | |||
| </GlobalStylesNavigationScreen> | |||
|
|
|||
| <GlobalStylesNavigationScreen path={ parentMenu + '/colors/text' }> | |||
| <ScreenTextColor name={ name } variation={ variation } /> | |||
There was a problem hiding this comment.
@draganescu here is where I stopped using screen-text-color.js in favor of screen-color-element.js instead
can you try again? I failed at copy pasting |
|
Looks good. One thought though—do we need Background Color for the caption? I'm not sure it adds value, and doesn't look good unless the image fills the available space. What do you think?
|
Would it be useful to have it for captions in tables? that would make it a candidate for a block specific element |
|
Yes, I think we can skip background color initially. |
I don't think so. Let's remove the background color to keep this a bit lighter. Then we're good to go here. |
|
Omg it's a bit tiring how this project still hasn't learnt what happens when they create a nonsense exception and hide hide hide hide more and more... Excuse me, Background-color is indeed important for captions... This discussion regarding "I think is not so used" always is flawed by essence. Gutenberg, maybe, may not offer all CSS options to be available. But they should be easily added by developers , inan extensible system, and related to this issue, those options that are already available to be used, should not be "hindered in the closet until mama and papa allows me to be who I am". And in this case, NO, Background-color ARE used on captions. Never on your official designs, true.. but outside yes... Just some examples: Sometimes a caption is positioned above the image, with a negative margin... Therefore it needs to be accompanied by text-shadow or background-color, to differentiate the caption text from the image behind it... The same logic suggests what happens when the body tag has an image... And lately, clients request these gaussian backdrop blur effect on everything... Captions as bubbles floating on the page. In this case, the background color is also needed... The Background-Color is used on High-Kontrast Style variations... Please please, in doubt, keep it. Please add background color to captions in global styles. Thank you of course |
|
Thanks for your comment. Please be assured that contributors above always try to focus on adding the most value for the time they have available. I'm pleased you raised the request for background colors on captions because the beauty of this being an open source project is that everyone is free to voice their opinion and advocate for certain features. That doesn't mean everything gets implemented immediately, but it does mean that features that are in high demand get visibility and everyone has a voice. As for this specific feature - please bear in mind that it might be that contributors need to prioritise other features. That said, pull requests are always welcomed if you feel able. I'm sure folks here would be happy to support with that. Thanks again for your feedback. |
What?
This PR replaces #44094 which had turned stale.
Add caption elements to the list of elements that can be customized in the Global Styles UI.
Closes #44071
Why?
So far, this could only be achieved via theme.json, now we can control these elements with Global Styles too.
How?
Adding captions to the list of elements in the color and typography sections of Global Styles. This PR also refactors the heading and button color screens to share the same code with the caption one, since the code was really similar between them. In the future we should try and adapt it to the other elements as well.
Testing Instructions
I'm using
emptythemewith the following in theme.json:Go to global styles and change typography and color settings for the captions. Check that the frontend matches what you see on the editor.
Screenshots or screencast
Screen.Capture.on.2022-09-15.at.16-27-26.mp4