WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.37k stars 4.15k forks source link

Typography panel: Component upsizing #36230

Closed mirka closed 2 years ago

mirka commented 2 years ago

On hold until #39397 is addressed.


Tasks required to cleanly migrate the Typography panel components to the new 40px sizes.

Prep work (merge at will)

Final steps to enable changes (merge all at once)

Component Enabling PR
FontAppearanceControl 🧪 #36162
FontFamilyControl 🧪 #TBD1
FontSizePicker #36162
LetterSpacingControl 🧪 #TBD1
LineHeightControl #36196
TextDecorationControl 🧪 #TBD2
TextTransformControl 🧪 #TBD2

(We can merge these into a temporary branch for testing before the final merge)

aaronrobertshaw commented 2 years ago

TBD2: Replace TextTransformControl 🧪 and TextDecorationControl 🧪 implementation with ToggleGroupControl 🧪

I took a brief look at switching these typography controls to use ToggleGroupControl. The switch itself is easy enough assuming that we are ok replacing the current icons with simple text labels. There is a bit of a problem though in that the ToggleGroupControl is based on a RadioGroup so once you select a value you can't "toggle it off".

In the context of block supports, the ability to toggle it off was required to allow for any styles applied by the theme or global styles to take effect.

If it interests, here is a quick patch to see this in action ```diff diff --git a/packages/block-editor/src/components/text-transform-control/index.js b/packages/block-editor/src/components/text-transform-control/index.js index 262484d843..ede7f11571 100644 --- a/packages/block-editor/src/components/text-transform-control/index.js +++ b/packages/block-editor/src/components/text-transform-control/index.js @@ -1,29 +1,27 @@ /** * WordPress dependencies */ -import { Button } from '@wordpress/components'; -import { __ } from '@wordpress/i18n'; import { - formatCapitalize, - formatLowercase, - formatUppercase, -} from '@wordpress/icons'; + __experimentalToggleGroupControl as ToggleGroupControl, + __experimentalToggleGroupControlOption as ToggleGroupControlOption, +} from '@wordpress/components'; +import { __ } from '@wordpress/i18n'; const TEXT_TRANSFORMS = [ { name: __( 'Uppercase' ), + label: 'AB', value: 'uppercase', - icon: formatUppercase, }, { name: __( 'Lowercase' ), + label: 'ab', value: 'lowercase', - icon: formatLowercase, }, { name: __( 'Capitalize' ), + label: 'Ab', value: 'capitalize', - icon: formatCapitalize, }, ]; @@ -38,28 +36,22 @@ const TEXT_TRANSFORMS = [ */ export default function TextTransformControl( { value, onChange } ) { return ( -
- { __( 'Letter case' ) } -
- { TEXT_TRANSFORMS.map( ( textTransform ) => { - return ( -
-
+ + { TEXT_TRANSFORMS.map( ( textTransform ) => { + return ( + + ); + } ) } + ); } ```
mirka commented 2 years ago

once you select a value you can't "toggle it off"

Mmm, good point. I'm going to break this one out into a discrete issue (#36735). If there aren't any clear solutions, it might be better to keep the existing implementation for now.

ciampo commented 2 years ago

Thank you for looking into this, @mirka !

The changes that you're proposing definitely go in a good direction — standardizing our UI!

I have a few considerations:

mirka commented 2 years ago
  • consistency across components: we should be careful about the "meaning" of a size prop

I think a manageable scope to start with is to care about consistency within "form" elements only (e.g. buttons, inputs, selects, etc).

As for the actual size scale naming, my assessment of the current landscape and designer sentiment is that we're not at all ready to commit to anything 😅 (I attempted to establish a size scale beforehand but it turned out to be unfeasible.) I'm so hesitant to commit to size names at this stage, even the large identifier is prefixed with __unstable. There's a substantial chance that this size will end up being called something else. I think we'll have to manage with temporary size names until we figure out a unified size scale that works across contexts (block placeholders vs. sidebar).

Regarding using experimental components inside stable components, it is something that should be discussed more in details. In general, if the experimental component remains an implementation detail (or is easy enough to replace later on), I don't see it as a problem.

You're right, I guess that will be true in most cases, and like you said before it's pretty much unavoidable to use experimental components inside mature ones.

ciampo commented 2 years ago

I think a manageable scope to start with is to care about consistency within "form" elements only (e.g. buttons, inputs, selects, etc).

Makes sense

As for the actual size scale naming, my assessment of the current landscape and designer sentiment is that we're not at all ready to commit to anything 😅 (I attempted to establish a size scale beforehand but it turned out to be unfeasible.) I'm so hesitant to commit to size names at this stage, even the large identifier is prefixed with __unstable. There's a substantial chance that this size will end up being called something else. I think we'll have to manage with temporary size names until we figure out a unified size scale that works across contexts (block placeholders vs. sidebar).

That's my fear too. Let's see how things evolve here, and let's consider other potential conventions too in case we can think about any alternative.

You're right, I guess that will be true in most cases, and like you said before it's pretty much unavoidable to use experimental components inside mature ones.

Agreed! Of course that doesn't mean that we shouldn't be mindful when using experimental components

mirka commented 2 years ago

Closing in favor of #41973

The Typography panel in Global Styles have been upsized in #42718. The ones in the post editor can also be upsized after TextTransformControl and TextDecorationControl are migrated to the component being prepared in #43060.