Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ToolsPanel: Update typography panel layout #35911

Merged
merged 3 commits into from
Oct 29, 2021
Merged

ToolsPanel: Update typography panel layout #35911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6bf5425
Re-establish single column span for appropriate controls
aaronrobertshaw Oct 25, 2021
b1baae8
Update letter-spacing control layout
aaronrobertshaw Oct 25, 2021
9fd5385
Pass through __unstableInputWidth to UnitControl
aaronrobertshaw Oct 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions packages/block-editor/src/components/letter-spacing-control/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,18 @@ import useSetting from '../../components/use-setting';
/**
* Control for letter-spacing.
*
* @param {Object} props Component props.
* @param {string} props.value Currently selected letter-spacing.
* @param {Function} props.onChange Handles change in letter-spacing selection.
* @return {WPElement} Letter-spacing control.
* @param {Object} props Component props.
* @param {string} props.value Currently selected letter-spacing.
* @param {Function} props.onChange Handles change in letter-spacing selection.
* @param {boolean} props.__unstableInputWidth Input width to pass through to inner UnitControl.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By looking at https://github.com/WordPress/gutenberg/blob/0160b7f1ccdc806a64eb111b36b4fd9c328ad49a/packages/components/src/input-control/types.ts, it seems like this prop should be of type string | number | undefined (and not boolean) — is this on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created follow-up PR to address this: #36367.

To correct the boolean type here but still override the default I needed to use null. For backwards compatibility, we need to provide a default width so setting the __unstableWidth prop to undefined doesn't work when wishing to restore full width to the control under the Typography ToolsPanel.

*
* @return {WPElement} Letter-spacing control.
*/
export default function LetterSpacingControl( { value, onChange } ) {
export default function LetterSpacingControl( {
value,
onChange,
__unstableInputWidth = '60px',
} ) {
const units = useCustomUnits( {
availableUnits: useSetting( 'spacing.units' ) || [ 'px', 'em', 'rem' ],
defaultValues: { px: '2', em: '.2', rem: '.2' },
Expand All @@ -29,7 +35,7 @@ export default function LetterSpacingControl( { value, onChange } ) {
<UnitControl
label={ __( 'Letter-spacing' ) }
value={ value }
__unstableInputWidth="60px"
__unstableInputWidth={ __unstableInputWidth }
units={ units }
onChange={ onChange }
/>
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/letter-spacing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export function LetterSpacingEdit( props ) {
<LetterSpacingControl
value={ style?.typography?.letterSpacing }
onChange={ onChange }
__unstableInputWidth={ false }
Copy link
Contributor

@ciampo ciampo Nov 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, I'm not sure if passing false is correct here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've switched this to null instead to override the default width required for backwards compatibility purposes.

/>
);
}
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/hooks/typography.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ export function TypographyPanel( props ) {
) }
{ ! isLetterSpacingDisabled && (
<ToolsPanelItem
className="single-column"
hasValue={ () => hasLetterSpacingValue( props ) }
label={ __( 'Letter-spacing' ) }
onDeselect={ () => resetLetterSpacing( props ) }
Expand Down
4 changes: 4 additions & 0 deletions packages/block-editor/src/hooks/typography.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,8 @@
.block-editor-line-height-control input {
max-width: 100%;
}

.single-column {
grid-column: span 1;
}
}