-
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
Block Support: Add text decoration block support using CSS variables #26059
Block Support: Add text decoration block support using CSS variables #26059
Conversation
@noahshrader or @ItsJonQ It would be great if we could get a design review for the Text Decoration controls UI when you get the chance. My apologies if there is someone else I should have pinged for this, you drew the short straw as you were mentioned on #25641 |
I think we'll want to match icons up here, but will defer to others on thoughts. @aaronrobertshaw I did notice if another setting like line-height is present it bumps up against this: |
I've tidied up the controls and switched the buttons to use icons which avoids i18n of the preset names. |
The direction seems good and I can help review/move this forward today. I'd like a confirmation from folks @mtias @youknowriad as if we want this for 5.6 (merge before Monday) or can land after. |
It was suggested in the PR adding block support for font styles that the current use of a segmented control here ( I'm open to suggestions here as well. Should this be changed to a cc @ItsJonQ |
@jasmussen Would you have a few moments to share any design thoughts on this one and #26060 ? |
Thanks for the ping. High level, we need a good design and pattern for such controls. I think icons in a segmented control is a good general approach, both for text decoration and text transform. It also needs to default to being unset, and for there to be an easy way to unset it. I would note that I see both text decoration and text transform controls as primarily global styles, and while you should be able to override them on a per paragraph basis, you should not see them by default. I'd love to get you a design that accomplishes this, but unless this can be limited to just global styles, I'd think it won't be a good experience to ship in 5.6. |
As part of discussions around the UI for this PR's text decoration controls and the text transform controls from #26060 it is likely they both will be displayed on a single row within the Typography panel. I've updated this PR with a new component that will cater to achieving such a layout. The changes also include simple icon only styling. |
Awesome. Can you update the underline icon to use this SVG?
|
Looking good! Let's make sure we only enable these new tools in dynamic blocks at first (navigation, site title, etc) and not on headings and paragraphs just yet. |
Thanks for the continued feedback.
|
77f4b5f
to
28b520d
Compare
I've rebased this to pull in the latest changes in approach to registering/applying block support. I think this just needs a review now to move forward. |
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.
28b520d
to
1460a7c
Compare
I've rebased and resolved conflicts for this after merge of text transform support #26060 |
906077c
to
9300e35
Compare
The direction the design is headed it to display text decoration and text transform controls side-by-side. This commit provides a new component to handle grouping these together and arranging them within a flex container. It also makes minor tweaks like labelling the controls "Decoration" instead of "Text Decoration".
* This updates the typography block support to check for text decoration support and generate an appropriate inline-style adding it to the attributes. * Removes opt-in for text decoration support from heading block * Opts-in for text decoration for navigation block and updates its CSS to leverage it
9300e35
to
4b1adb9
Compare
@aaronrobertshaw @apeatling This PR landed without documentation. I've prepared a fix for it at #26891 |
Description
This adds block support for text decoration styles and is a continuation from discussion around #25641 around adding font style block support.
Changes Included
Notes
How has this been tested?
Manually tested using heading block.
Test Instructions
var()
and an appropriate CSS variable.Screenshots
Types of changes
Enhancement
Next Steps
class-block-supported-styles-test.php
if needed after approach confirmed.Checklist: