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

Add layout content size controls to global styles #42309

Merged
merged 7 commits into from
Jul 20, 2022
Merged

Add layout content size controls to global styles #42309

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cdae78c
Add layout content size controls to global styles
tellthemachines Jul 11, 2022
bd6e4af
Align flex items
tellthemachines Jul 12, 2022
84eb319
Render flex wrappers conditionally
tellthemachines Jul 19, 2022
e604bcf
Fix reset behaviour
tellthemachines Jul 19, 2022
7affab7
Optimise markup
tellthemachines Jul 19, 2022
f22d79e
Tweak how controls are laid out within panel
aaronrobertshaw Jul 20, 2022
8cb01e5
Fix text i18n
aaronrobertshaw Jul 20, 2022
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
99 changes: 98 additions & 1 deletion packages/edit-site/src/components/global-styles/dimensions-panel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ import {
__experimentalBoxControl as BoxControl,
__experimentalUnitControl as UnitControl,
__experimentalUseCustomUnits as useCustomUnits,
Flex,
FlexItem,
} from '@wordpress/components';
import { __experimentalUseCustomSides as useCustomSides } from '@wordpress/block-editor';
import { Icon, positionCenter, stretchWide } from '@wordpress/icons';

/**
* Internal dependencies
Expand All @@ -19,11 +22,27 @@ import { getSupportedGlobalStylesPanels, useSetting, useStyle } from './hooks';
const AXIAL_SIDES = [ 'horizontal', 'vertical' ];

export function useHasDimensionsPanel( name ) {
const hasContentSize = useHasContentSize( name );
const hasWideSize = useHasWideSize( name );
const hasPadding = useHasPadding( name );
const hasMargin = useHasMargin( name );
const hasGap = useHasGap( name );

return hasPadding || hasMargin || hasGap;
return hasContentSize || hasWideSize || hasPadding || hasMargin || hasGap;
}

function useHasContentSize( name ) {
const supports = getSupportedGlobalStylesPanels( name );
const [ settings ] = useSetting( 'layout.contentSize', name );

return settings && supports.includes( 'contentSize' );
}

function useHasWideSize( name ) {
const supports = getSupportedGlobalStylesPanels( name );
const [ settings ] = useSetting( 'layout.wideSize', name );

return settings && supports.includes( 'wideSize' );
}

function useHasPadding( name ) {
Expand Down Expand Up @@ -86,6 +105,8 @@ function splitStyleValue( value ) {
}

export default function DimensionsPanel( { name } ) {
const showContentSizeControl = useHasContentSize( name );
const showWideSizeControl = useHasWideSize( name );
const showPaddingControl = useHasPadding( name );
const showMarginControl = useHasMargin( name );
const showGapControl = useHasGap( name );
Expand All @@ -99,6 +120,22 @@ export default function DimensionsPanel( { name } ) {
],
} );

const [ contentSizeValue, setContentSizeValue ] = useSetting(
'layout.contentSize',
name
);

const hasContentSizeValue = () => !! contentSizeValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Taking another look at this check, I'm wondering if part of the issue with the ToolsPanel menu state is that if the contentSizeValue matches the value after reset, then this will return true. So if my content width is set to 840px and I change it to 900px and then reset it, the value here is 840px, and so the menu item thinks there's still a custom value?

That said, there seems to be something else funny going on, because when I first load the site editor, the ToolsPanel seems to be showing padding as customised, too 🤔

image

The weird behaviour does appear to be on trunk, too, so not sure if it's worth attempting to fix now? I think on trunk it's a little less visible because it seems to be working fine at the block level, and we currently only have the Padding dimensions control made visible at the root level 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

So if my content width is set to 840px and I change it to 900px and then reset it, the value here is 840px, and so the menu item thinks there's still a custom value?

It appears that the menu is reflecting exactly what the hasContentSizeValue callback is telling it. Any truthy value for contentSizeValue is going to state that the ToolsPanelItem has a customized value.

If I understand the situation correctly, there's always a root value so the logic here needs to be more nuanced. It should be checking if the user has set a value i.e. check the style value from the user origin rather than the merged styles aka all origin.

The border panel checks the user origin border styles to reflect the whether the user has customized any values e.g. const [ userBorderStyles ] = useStyle( 'border', name, 'user' );

It would be a worthwhile follow up to give these global styles panels an audit to check they all behave appropriately. I know things have evolved rapidly here on multiple fronts so its entirely possible a few gremlins crept in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checking for user styles makes sense! Updated.

const resetContentSizeValue = () => setContentSizeValue( '' );

const [ wideSizeValue, setWideSizeValue ] = useSetting(
'layout.wideSize',
name
);

const hasWideSizeValue = () => !! wideSizeValue;
const resetWideSizeValue = () => setWideSizeValue( '' );

const [ rawPadding, setRawPadding ] = useStyle( 'spacing.padding', name );
const paddingValues = splitStyleValue( rawPadding );
const paddingSides = useCustomSides( name, 'padding' );
Expand Down Expand Up @@ -141,6 +178,66 @@ export default function DimensionsPanel( { name } ) {

return (
<ToolsPanel label={ __( 'Dimensions' ) } resetAll={ resetAll }>
<Flex>
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this row might need to be behind a check for showContentSizeControl || showSideSizeControl, otherwise in the Layout panel at the block level, there is a larger space due to the gap surrounding the empty Flex component:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes, definitely!

<FlexItem>
{ showContentSizeControl && (
<ToolsPanelItem
label={ __( 'Content size' ) }
hasValue={ hasContentSizeValue }
onDeselect={ resetContentSizeValue }
isShownByDefault={ true }
>
<Flex align="flex-end">
<FlexItem>
<UnitControl
label={ __( 'Content' ) }
labelPosition="top"
__unstableInputWidth="80px"
value={ contentSizeValue || '' }
onChange={ ( nextContentSize ) => {
setContentSizeValue(
nextContentSize
);
} }
units={ units }
/>
</FlexItem>
<FlexItem>
<Icon icon={ positionCenter } />
</FlexItem>
</Flex>
</ToolsPanelItem>
) }
</FlexItem>
<FlexItem>
{ showWideSizeControl && (
<ToolsPanelItem
label={ __( 'Wide size' ) }
hasValue={ hasWideSizeValue }
onDeselect={ resetWideSizeValue }
isShownByDefault={ true }
>
<Flex align="flex-end">
<FlexItem>
<UnitControl
label={ __( 'Wide' ) }
labelPosition="top"
__unstableInputWidth="80px"
value={ wideSizeValue || '' }
onChange={ ( nextWideSize ) => {
setWideSizeValue( nextWideSize );
} }
units={ units }
/>
</FlexItem>
<FlexItem>
<Icon icon={ stretchWide } />
</FlexItem>
</Flex>
</ToolsPanelItem>
) }
</FlexItem>
</Flex>
{ showPaddingControl && (
<ToolsPanelItem
hasValue={ hasPaddingValue }
Expand Down
2 changes: 2 additions & 0 deletions packages/edit-site/src/components/global-styles/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ const ROOT_BLOCK_SUPPORTS = [
'textDecoration',
'textTransform',
'padding',
'contentSize',
'wideSize',
];

export function getSupportedGlobalStylesPanels( name ) {
Expand Down