ToolsPanel: Standardize input control label margin

[aaronrobertshaw]

Related: #36334

Description

This PR standardizes the line-height and margin of InputControl labels within the ToolsPanel to improve consistency.

See: #36334 (comment)

How has this been tested?

1. Open editor and add text to a paragraph
2. With paragraph block selected, toggle on display of both line height and letter spacing controls from the typography panel
3. Ensure the labels and spacing looks consistent

Screenshots

Before	After

Types of changes

Enhancement.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

[aaronrobertshaw]

I ended up using LabelWrapper from the input control styles here as I couldn't get BaseLabel to work for me. Open to suggestions to improve this.

[ramonjd]

Very nice. Looks 👍 to me. Thanks for the elegant solution.

Before	After

[ciampo]

I inspected the control components and the corresponding labels:

• The "Appearance" control is based on the CustomSelectControl component, which internally uses a "vanilla" <label>
• The "Letter spacing" control is based on the UnitControl component, which builds on top of the InputControl component (including its label)

Instead of solving this problem specifically to ToolsPanel, I think that a better approach here would be to align the label inCustomSelectControl to have the same styling (including line height, margins and paddings) to the one of InputControl.

Not sure if it's the best solution here, but maybe CustomSelectControl could use the BaseLabel directly from InputControl? (Or, in general, have CustomSelectControl be based off InputControl as much as possible) ?

---

Also a side note — should we update this Storybook example to allow single-columns controls? It would be another good way to test for the changes in this PR in an isolated environment

[aaronrobertshaw]

I inspected the control components and the corresponding labels:

I think there is another type of control missed here, the BaseControl.

Just within the Typography panel alone we have three four sources for different labels:

• CustomSelectControl --> HTML label
• InputControl --> BaseLabel
• BaseControl --> StyledLabel
• Text Decoration/Transform controls --> HTML legend

I'd be inclined to believe the BaseControl should be the starting point. Plus it actually provides a margin so the label isn't cramped against the input.

Instead of solving this problem specifically to ToolsPanel, I think that a better approach here would be to align the label inCustomSelectControl to have the same styling (including line height, margins and paddings) to the one of InputControl.

The InputControl appears to be the odd one out here in terms of styling.

I wanted to avoid making any breaking changes (even if only styling) outside of the ToolsPanel given these are all likely used elsewhere. It feels like it would be better to bring all of these inline in a separate PR once all the required components and those built off them are switch to Emotion and the context system.

Also a side note — should we update this Storybook example to allow single-columns controls? It would be another good way to test for the changes in this PR in an isolated environment

My understanding was that Storybook example was a playground for developing the new controls for the Typography panel. I'm not sure it makes sense to try and use it to text/develop the current controls at the same time. A small problem with relying on the Storybook and an isolated environment is we still have to make it line up neatly outside that sterile environment and in the real editors.

[ciampo]

I think there is another type of control missed here, the BaseControl [...] Just within the Typography panel alone we have three four sources for different labels

Thank you for the sleuthing and the detailed explanation!

I'd be inclined to believe the BaseControl should be the starting point

Refactoring these components to use the same base control/label sounds definitely like the sensible thing to do

I wanted to avoid making any breaking changes (even if only styling) outside of the ToolsPanel given these are all likely used elsewhere. It feels like it would be better to bring all of these inline in a separate PR once all the required components and those built off them are switch to Emotion and the context system.

This is definitely a sensible point — I'd be ok with this PR focusing on a "hotfix" specific for ToolsPanel.

But at the same time it'd be great if we could follow up and look into the discrepancies across "control" components, come up with a good understanding/dependency graph, and work towards a normalization (in the direction that you suggested above) soon.

My understanding was that Storybook example was a playground for developing the new controls for the Typography panel. I'm not sure it makes sense to try and use it to text/develop the current controls at the same time. A small problem with relying on the Storybook and an isolated environment is we still have to make it line up neatly outside that sterile environment and in the real editors.

I agree that relying on Storybook alone is not a good idea, since it is a "sterile" environment, and that ultimately things need to look good in the real editors. At the same time, I also believe that it's important to use Storybook as the first "test stage" for our components.

As an example, I tried to to update Storybook so that the Line Height and Letter Spacing controls are next to each other, and noticed a misalignment

Code used to update Storybook

diff --git a/packages/components/src/tools-panel/stories/typography-panel.js b/packages/components/src/tools-panel/stories/typography-panel.js
index c57ac4c19e..afd8646788 100644
--- a/packages/components/src/tools-panel/stories/typography-panel.js
+++ b/packages/components/src/tools-panel/stories/typography-panel.js
@@ -1,3 +1,8 @@
+/**
+ * External dependencies
+ */
+import { css } from '@emotion/react';
+
 /**
  * WordPress dependencies
  */
@@ -12,6 +17,7 @@ import { useState } from '@wordpress/element';
 /**
  * Internal dependencies
  */
+import { useCx } from '../../utils/hooks/use-cx';
 import FontSizePicker from '../../font-size-picker';
 import Panel from '../../panel';
 import {
@@ -97,6 +103,10 @@ export const TypographyPanel = () => {
 	const [ letterSpacing, setLetterSpacing ] = useState();
 	const [ textTransform, setTextTransform ] = useState();
 
+	const cx = useCx();
+
+	const singleColumnClassName = cx( css( { gridColumn: 'span 1' } ) );
+
 	return (
 		<>
 			<div style={ { maxWidth: '280px' } }>
@@ -157,6 +167,7 @@ export const TypographyPanel = () => {
 							label="Line Height"
 							onDeselect={ () => setLineHeight( undefined ) }
 							isShownByDefault={ true }
+							className={ singleColumnClassName }
 						>
 							<LineHeightControl
 								value={ lineHeight }
@@ -169,6 +180,7 @@ export const TypographyPanel = () => {
 							label="Letter Spacing"
 							onDeselect={ () => setLetterSpacing( undefined ) }
 							isShownByDefault={ true }
+							className={ singleColumnClassName }
 						>
 							<LetterSpacingControl
 								value={ letterSpacing }

What I'm suggesting is a component-first approach (as opposed to an editor-first approach). In order to do that:

1. We should make sure that a component is displayed correctly in Storybook first. This would guarantee that the component works as expected "out o the box"
2. We should then test it in the real code editor, and see if there are any discrepancies. In case there are, we should investigate the issue, and always try to fix it at the source (the component), and restart the process from point 1. When that's not possible, we would need to resort to applying a fix in the editor code

[aaronrobertshaw]

As an example, I tried to to update Storybook so that the Line Height and Letter Spacing controls are next to each other, and noticed a misalignment

This appears to be due to the lack of a standard line-height applied to labels in the Storybook.

I've added a new rule that ensures that standardized line-height for BaseControl labels within the tool panel and Storybook.

You'll notice that:

• Line height and letter spacing labels align
• Font size component effectively has it's label heightened due to the toggle icon
• The proposed letter-case control uses ToggleGroupControl which would need updating to style the label effectively

[ciampo]

This appears to be due to the lack of a standard line-height applied to labels in the Storybook.

Yes, my main point is to say that these fixes to the labels:

• should be applied to the individual components (and not to the ToolsPanel)
• should be tested in Storybook first, and in the editor after

I understand the need to apply this fix to the ToolsPanel temporarily with this PR, but we should follow-up with an investigation and a refactor of the control components. Ideally, they should all inherit from the same BaseControl and share the same label styles.

Finally, before merging, could you also add a quick note to the CHANGELOG? Thank you!

🚀