-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 transform block support using CSS variables #26060
Block Support: Add text transform block support using CSS variables #26060
Conversation
@noahshrader or @ItsJonQ It would be great if we could get a design review for the Text Transform 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 |
This works well. It has the same UI issue as #26059 though: |
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 |
After further discussions around the UI for this PR's text transform controls and the text decoration controls from #26059 it is likely they should be displayed on a single row within the Typography panel. I've updated this PR with a new component that will cater to achieving this layout. The changes also include simple icon-only styling and using plain text to more closely reflect the icons that will eventually be used. |
Nice work. Here's a snippet of a design that tentatively takes a stab at an organization of related typography tools: I think it's fine to use that dark toggle, even if at a glance it appears able to toggle multiple at once, you only need to toggle either once to learn how it works, and it comes with an intuitive "untoggle" benefit. Worth noting, though, that these two controls shown above, should not show up in the typography for the Paragraph panel yet. We need a good system to progressively show such controls in that block, because it's not something you should be using all the time. But until we have such a progressively unlocking system, I'd recommend adding this only to the Navigation block as an experimental place for it to be a the beginning. I think we should use icons for both underline, strikethrough and the various capitalizations. Do you need SVGs for those? I can provide. Also, the toggle should be square. Since it doesn't feel like we should be replacing the entire segmented control with this design (there are still cases where we need the classic segmented control), if you need to change the CSS appearance to accommodate this use case, I wonder if a prop is a good way forward? |
Thanks 🙂
I agree.
At this stage, this feature is opt-in only and was only turned on in the heading block for testing purposes. This is easy to change.
Definitely, icons would be the way to go. If you can provide SVGs for the various capitalizations that would be great! I only used plain text as a placeholder due to not finding SVG icons within
When I add the SVG icons, I'll update the CSS to ensure each toggle is square.
Just so I'm clear, your suggestion is that I continue to use a I appreciate the feedback on this. |
Awesome, I'll get you SVGs in a moment, feel free to update any existing icons in the package directly, or add new ones for ones that are missing.
I did not dive deep into the component structure here, so I'll defer entirely to you on which components to use. I just want to make sure that for this particular type of "toggle", we use the same toggle in other places going forward. |
SVGs. UPPERCASE:
lowercase:
Capitalize:
|
Thank you for the icons, looks much better now 👌 I've also removed the opt-in to this feature from the heading block and added it to the navigation block instead. |
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 "Letter case" instead of "Text Transform" and using plain text to provide something like the icons will be.
cb69e2a
to
c35f246
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.
Failing e2e tests are timeouts and not related to this PR, so moving ahead with merge. |
Congratulations on your first merged pull request, @aaronrobertshaw! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts: https://profiles.wordpress.org/me/profile/edit/ And if you don't have a WordPress.org account, you can create one on this page: https://login.wordpress.org/register Kudos! |
@aaronrobertshaw @apeatling This PR landed without documentation. I've prepared a fix for it at #26891 |
Description
This adds block support for text transform 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: