Skip to content

Commit

Permalink
Change DimensionsTool to not render If resizing is not possible
Browse files Browse the repository at this point in the history
  • Loading branch information
t-hamano committed Aug 4, 2023
1 parent 3df376b commit 8f72a03
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 93 deletions.
92 changes: 43 additions & 49 deletions packages/block-editor/src/components/dimensions-tool/index.js
Expand Up @@ -58,7 +58,6 @@ function DimensionsTool( {
scaleOptions, // Default options handled by ScaleTool.
defaultScale = 'fill', // Match CSS default value for object-fit.
unitsOptions, // Default options handled by UnitControl.
isResizable = true, // Whether to allow width or height changes.
} ) {
// Coerce undefined and CSS default values to be null.
const width =
Expand Down Expand Up @@ -157,60 +156,55 @@ function DimensionsTool( {
} }
/>
) }
{ isResizable && (
<WidthHeightTool
panelId={ panelId }
units={ unitsOptions }
value={ { width, height } }
onChange={ ( { width: nextWidth, height: nextHeight } ) => {
const nextValue = { ...value };
<WidthHeightTool
panelId={ panelId }
units={ unitsOptions }
value={ { width, height } }
onChange={ ( { width: nextWidth, height: nextHeight } ) => {
const nextValue = { ...value };

// 'auto' is CSS default, so it gets treated as null.
nextWidth = nextWidth === 'auto' ? null : nextWidth;
nextHeight = nextHeight === 'auto' ? null : nextHeight;
// 'auto' is CSS default, so it gets treated as null.
nextWidth = nextWidth === 'auto' ? null : nextWidth;
nextHeight = nextHeight === 'auto' ? null : nextHeight;

// Update width.
if ( ! nextWidth ) {
delete nextValue.width;
} else {
nextValue.width = nextWidth;
}
// Update width.
if ( ! nextWidth ) {
delete nextValue.width;
} else {
nextValue.width = nextWidth;
}

// Update height.
if ( ! nextHeight ) {
delete nextValue.height;
} else {
nextValue.height = nextHeight;
}
// Update height.
if ( ! nextHeight ) {
delete nextValue.height;
} else {
nextValue.height = nextHeight;
}

// Auto-update aspectRatio.
if ( nextWidth && nextHeight ) {
delete nextValue.aspectRatio;
} else if ( lastAspectRatio ) {
nextValue.aspectRatio = lastAspectRatio;
} else {
// No setting defaultAspectRatio here, because
// aspectRatio is optional in this scenario,
// unlike scale.
}
// Auto-update aspectRatio.
if ( nextWidth && nextHeight ) {
delete nextValue.aspectRatio;
} else if ( lastAspectRatio ) {
nextValue.aspectRatio = lastAspectRatio;
} else {
// No setting defaultAspectRatio here, because
// aspectRatio is optional in this scenario,
// unlike scale.
}

// Auto-update scale.
if (
! lastAspectRatio &&
!! nextWidth !== !! nextHeight
) {
delete nextValue.scale;
} else if ( lastScale ) {
nextValue.scale = lastScale;
} else {
nextValue.scale = defaultScale;
setLastScale( defaultScale );
}
// Auto-update scale.
if ( ! lastAspectRatio && !! nextWidth !== !! nextHeight ) {
delete nextValue.scale;
} else if ( lastScale ) {
nextValue.scale = lastScale;
} else {
nextValue.scale = defaultScale;
setLastScale( defaultScale );
}

onChange( nextValue );
} }
/>
) }
onChange( nextValue );
} }
/>
</>
);
}
Expand Down
25 changes: 0 additions & 25 deletions packages/block-editor/src/components/dimensions-tool/test/index.js
Expand Up @@ -637,30 +637,5 @@ describe( 'DimensionsTool', () => {
],
] );
} );

it( 'when isResizable is false then the WidthHeightTool component should not be rendered', async () => {
const onChange = jest.fn();

const value = {};

render(
<Example
initialValue={ value }
onChange={ onChange }
isResizable={ false }
/>
);

const widthInput = screen.queryByRole( 'spinbutton', {
name: 'Width',
} );

const heightInput = screen.queryByRole( 'spinbutton', {
name: 'Height',
} );

expect( widthInput ).not.toBeInTheDocument();
expect( heightInput ).not.toBeInTheDocument();
} );
} );
} );
39 changes: 20 additions & 19 deletions packages/block-library/src/image/image.js
Expand Up @@ -477,25 +477,26 @@ export default function Image( {
/>
</ToolsPanelItem>
) }
<DimensionsTool
value={ { width, height, scale, aspectRatio } }
onChange={ ( newValue ) => {
// Rebuilding the object forces setting `undefined`
// for values that are removed since setAttributes
// doesn't do anything with keys that aren't set.
setAttributes( {
width: newValue.width,
height: newValue.height,
scale: newValue.scale,
aspectRatio: newValue.aspectRatio,
} );
} }
isResizable={ isResizable }
defaultScale="cover"
defaultAspectRatio="auto"
scaleOptions={ scaleOptions }
unitsOptions={ dimensionsUnitsOptions }
/>
{ isResizable && (
<DimensionsTool
value={ { width, height, scale, aspectRatio } }
onChange={ ( newValue ) => {
// Rebuilding the object forces setting `undefined`
// for values that are removed since setAttributes
// doesn't do anything with keys that aren't set.
setAttributes( {
width: newValue.width,
height: newValue.height,
scale: newValue.scale,
aspectRatio: newValue.aspectRatio,
} );
} }
defaultScale="cover"
defaultAspectRatio="auto"
scaleOptions={ scaleOptions }
unitsOptions={ dimensionsUnitsOptions }
/>
) }
<ResolutionTool
value={ sizeSlug }
onChange={ updateImage }
Expand Down

0 comments on commit 8f72a03

Please sign in to comment.