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 aspect ratio to image placeholder #54216

Merged
merged 5 commits into from
Sep 11, 2023
Merged

Add aspect ratio to image placeholder #54216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
42f7889
adds aspect ratio control and behavior to image block placeholder
draganescu Sep 6, 2023
0c35e2e
Allow the image placeholder to shrink to small sizes and then allow i…
scruffian Sep 8, 2023
98294e6
Align the placeholder to the top
scruffian Sep 8, 2023
59f03ae
remove resolution tool
scruffian Sep 8, 2023
1e964bf
refactor duplicate code
scruffian Sep 8, 2023
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
45 changes: 27 additions & 18 deletions packages/block-library/src/image/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ export function ImageEdit( {
width,
height,
sizeSlug,
aspectRatio,
scale,
} = attributes;
const [ temporaryURL, setTemporaryURL ] = useState();

Expand Down Expand Up @@ -335,7 +337,16 @@ export function ImageEdit( {
instructions={ __(
'Upload an image file, pick one from your media library, or add one with a URL.'
) }
style={ isSelected ? undefined : borderProps.style }
style={ {
aspectRatio:
! ( width && height ) && aspectRatio
? aspectRatio
: undefined,
width: height && aspectRatio ? '100%' : width,
height: width && aspectRatio ? '100%' : height,
objectFit: scale,
...borderProps.style,
} }
>
{ content }
</Placeholder>
Expand All @@ -344,23 +355,21 @@ export function ImageEdit( {

return (
<figure { ...blockProps }>
{ ( temporaryURL || url ) && (
<Image
temporaryURL={ temporaryURL }
attributes={ attributes }
setAttributes={ setAttributes }
isSelected={ isSelected }
insertBlocksAfter={ insertBlocksAfter }
onReplace={ onReplace }
onSelectImage={ onSelectImage }
onSelectURL={ onSelectURL }
onUploadError={ onUploadError }
containerRef={ ref }
context={ context }
clientId={ clientId }
blockEditingMode={ blockEditingMode }
/>
) }
<Image
temporaryURL={ temporaryURL }
attributes={ attributes }
setAttributes={ setAttributes }
isSelected={ isSelected }
insertBlocksAfter={ insertBlocksAfter }
onReplace={ onReplace }
onSelectImage={ onSelectImage }
onSelectURL={ onSelectURL }
onUploadError={ onUploadError }
containerRef={ ref }
context={ context }
clientId={ clientId }
blockEditingMode={ blockEditingMode }
/>
{ ! url && blockEditingMode === 'default' && (
<BlockControls group="block">
<BlockAlignmentControl
Expand Down
96 changes: 53 additions & 43 deletions packages/block-library/src/image/image.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,53 @@ export default function Image( {
availableUnits: [ 'px' ],
} );

const dimensionsControl = (
<DimensionsTool
value={ { width, height, scale, aspectRatio } }
onChange={ ( {
width: newWidth,
height: newHeight,
scale: newScale,
aspectRatio: newAspectRatio,
} ) => {
// Rebuilding the object forces setting `undefined`
// for values that are removed since setAttributes
// doesn't do anything with keys that aren't set.
setAttributes( {
// CSS includes `height: auto`, but we need
// `width: auto` to fix the aspect ratio when
// only height is set due to the width and
// height attributes set via the server.
width: ! newWidth && newHeight ? 'auto' : newWidth,
height: newHeight,
scale: newScale,
aspectRatio: newAspectRatio,
} );
} }
defaultScale="cover"
defaultAspectRatio="auto"
scaleOptions={ scaleOptions }
unitsOptions={ dimensionsUnitsOptions }
/>
);

const resetAll = () => {
setAttributes( {
width: undefined,
height: undefined,
scale: undefined,
aspectRatio: undefined,
} );
};

const sizeControls = (
<InspectorControls>
<ToolsPanel label={ __( 'Settings' ) } resetAll={ resetAll }>
{ isResizable && dimensionsControl }
</ToolsPanel>
</InspectorControls>
);

const controls = (
<>
<BlockControls group="block">
Expand Down Expand Up @@ -443,17 +490,7 @@ export default function Image( {
</BlockControls>
) }
<InspectorControls>
<ToolsPanel
label={ __( 'Settings' ) }
resetAll={ () =>
setAttributes( {
width: undefined,
height: undefined,
scale: undefined,
aspectRatio: undefined,
} )
}
>
<ToolsPanel label={ __( 'Settings' ) } resetAll={ resetAll }>
{ ! multiImageSelection && (
<ToolsPanelItem
label={ __( 'Alternative text' ) }
Expand Down Expand Up @@ -482,38 +519,7 @@ export default function Image( {
/>
</ToolsPanelItem>
) }
{ isResizable && (
<DimensionsTool
value={ { width, height, scale, aspectRatio } }
onChange={ ( {
width: newWidth,
height: newHeight,
scale: newScale,
aspectRatio: newAspectRatio,
} ) => {
// Rebuilding the object forces setting `undefined`
// for values that are removed since setAttributes
// doesn't do anything with keys that aren't set.
setAttributes( {
// CSS includes `height: auto`, but we need
// `width: auto` to fix the aspect ratio when
// only height is set due to the width and
// height attributes set via the server.
width:
! newWidth && newHeight
? 'auto'
: newWidth,
height: newHeight,
scale: newScale,
aspectRatio: newAspectRatio,
} );
} }
defaultScale="cover"
defaultAspectRatio="auto"
scaleOptions={ scaleOptions }
unitsOptions={ dimensionsUnitsOptions }
/>
) }
{ isResizable && dimensionsControl }
<ResolutionTool
value={ sizeSlug }
onChange={ updateImage }
Expand Down Expand Up @@ -726,6 +732,10 @@ export default function Image( {
);
}

if ( ! url && ! temporaryURL ) {
return sizeControls;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a new sizeControls const or can we reuse controls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

controls includes caption and I think setting caption on placeholder is not expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we update controls to extend sizeControls?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure we could add some internal getter for controls that supports an include caption arg but why tho 😁 - it's not something really reused, it's a one time need in terms of the block's codebase.

}
ajlende marked this conversation as resolved.
Show resolved Hide resolved

return (
<>
{ /* Hide controls during upload to avoid component remount,
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/placeholder/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export function Placeholder(
const fieldsetClasses = classnames( 'components-placeholder__fieldset', {
'is-column-layout': isColumnLayout,
} );

return (
<div { ...additionalProps } className={ classes }>
{ withIllustration ? PlaceholderIllustration : null }
Expand Down
6 changes: 2 additions & 4 deletions packages/components/src/placeholder/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
box-sizing: border-box;
position: relative;
padding: 1em;
min-height: 200px;
width: 100%;
text-align: left;
margin: 0;
Expand All @@ -17,7 +16,7 @@
@supports (position: sticky) {
display: flex;
flex-direction: column;
justify-content: center;
justify-content: top;
align-items: flex-start;
}

Expand Down Expand Up @@ -180,7 +179,6 @@
color: inherit;
display: flex;
box-shadow: none;
min-width: 100px;

// Blur the background so layered dashed placeholders are still visually separate.
// Make the background transparent to not interfere with the background overlay in placeholder-style() pseudo element
Expand Down Expand Up @@ -225,7 +223,7 @@

// By painting the borders here, we enable them to be replaced by the Border control.
@include placeholder-style();
overflow: hidden;
overflow: auto;
}

// Position the spinner.
Expand Down
Loading