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

Background image support: add background position controls #58592

Merged
merged 3 commits into from Feb 6, 2024
Merged

Background image support: add background position controls #58592

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c9bbce
Background image support: Try adding background position controls
andrewserong Feb 2, 2024
d9d87ba
Fix reset and hasValue logic, add tests, update label
andrewserong Feb 5, 2024
5af7895
Ensure default position of focal point picker control is centered
andrewserong Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 81 additions & 0 deletions packages/block-editor/src/hooks/background.js
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@ import {
__experimentalVStack as VStack,
DropZone,
FlexItem,
FocalPointPicker,
MenuItem,
VisuallyHidden,
__experimentalItemGroup as ItemGroup,
Expand Down Expand Up @@ -367,6 +368,64 @@ function backgroundSizeHelpText( value ) {
return __( 'Set a fixed width.' );
}

const coordsToBackgroundPosition = ( value ) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a quick pass at the mapping — if this PR looks okay, I'll fine-tune the logic a bit here and add some tests.

Copy link
Member

Choose a reason for hiding this comment

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

This particular bit LGTM, but just curious: why do we need to do this mapping? Aren't the percentage values enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question: we need to have at least some form of mapping so that we're not storing values like { x: 0.5, y: 0.5 } in the block attributes for style.background.backgroundPosition. For future proofing the feature, I think it's better if that value can be stored in the block delimiter as a real CSS value, rather than a value that's intended for the internal workings of the focal point picker. So at the very least, I think it's better if we can have the support map { x: 0.5, y: 0.5 } to 50% 50%.

In terms of mapping the natural language values like left, center, etc, that's mostly me thinking of what would be good to support for if/when we get to theme.json support where folks might be writing the JSON a bit by hand. It'd be nice to be able to support mapping potentially handwritten values so that they work naturally with this control.

For now, though, we only really need the generic decimal 0.5 > 50% mapping for this to work, so I'm happy to pare it back for this PR.

Copy link
Member

Choose a reason for hiding this comment

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

In terms of mapping the natural language values like left, center, etc, that's mostly me thinking of what would be good to support for if/when we get to theme.json support where folks might be writing the JSON a bit by hand.

Very good idea!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now I've just gone with converting between percentage and coordinate values. There's a bit more nuanced to the multi-value syntax for background position, so I think that'd be better to handle separately in a follow-up code quality PR. For now I've gone with the main use case logic which is background position values that were created using this particular UI control.

if ( ! value ) {
return undefined;
}

if ( value.x === 0 && value.y === 0 ) {
return 'left top';
}

if ( value.x === 0 && value.y === 1 ) {
return 'left bottom';
}

if ( value.x === 1 && value.y === 0 ) {
return 'right top';
}

if ( value.x === 1 && value.y === 1 ) {
return 'right bottom';
}

if ( value.x === 0.5 && value.y === 0.5 ) {
return 'center';
}

return `${ value.x * 100 }% ${ value.y * 100 }%`;
};

const backgroundPositionToCoords = ( value ) => {
if ( ! value ) {
return { x: 0.5, y: 0.5 };
}

if ( value === 'left top' ) {
return { x: 0, y: 0 };
}

if ( value === 'left bottom' ) {
return { x: 0, y: 1 };
}

if ( value === 'right top' ) {
return { x: 1, y: 0 };
}

if ( value === 'right bottom' ) {
return { x: 1, y: 1 };
}

if ( value === 'center' ) {
return { x: 0.5, y: 0.5 };
}

const [ x, y ] = value.split( ' ' ).map( ( v ) => parseFloat( v ) / 100 );

return { x, y };
};

function BackgroundSizePanelItem( {
clientId,
isShownByDefault,
Expand Down Expand Up @@ -446,6 +505,18 @@ function BackgroundSizePanelItem( {
} );
};

const updateBackgroundPosition = ( next ) => {
setAttributes( {
style: cleanEmptyObject( {
...style,
background: {
...style?.background,
backgroundPosition: coordsToBackgroundPosition( next ),
},
} ),
} );
};

const toggleIsRepeated = () => {
setAttributes( {
style: cleanEmptyObject( {
Expand All @@ -471,6 +542,16 @@ function BackgroundSizePanelItem( {
resetAllFilter={ resetAllFilter }
panelId={ clientId }
>
<FocalPointPicker
__nextHasNoMarginBottom
__next40pxDefaultSize
label={ __( 'Focal point' ) }
url={ style?.background?.backgroundImage?.url }
value={ backgroundPositionToCoords(
style?.background?.backgroundPosition
) }
onChange={ updateBackgroundPosition }
/>
<ToggleGroupControl
__nextHasNoMarginBottom
size={ '__unstable-large' }
Expand Down
19 changes: 18 additions & 1 deletion packages/style-engine/src/styles/background/index.ts
View file
Open in desktop
Expand Up @@ -52,6 +52,18 @@ const backgroundRepeat = {
},
};

const backgroundPosition = {
name: 'backgroundRepeat',
generate: ( style: Style, options: StyleOptions ) => {
return generateRule(
style,
options,
[ 'background', 'backgroundPosition' ],
andrewserong marked this conversation as resolved.
Show resolved Hide resolved
'backgroundPosition'
);
},
};

const backgroundSize = {
name: 'backgroundSize',
generate: ( style: Style, options: StyleOptions ) => {
Expand Down Expand Up @@ -89,4 +101,9 @@ const backgroundSize = {
},
};

export default [ backgroundImage, backgroundRepeat, backgroundSize ];
export default [
backgroundImage,
backgroundRepeat,
backgroundPosition,
backgroundSize,
];