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.

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

Add min-height to media & text Block #32325

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions packages/block-library/src/media-text/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@
},
"focalPoint": {
"type": "object"
},
"minHeight": {
"type": "string"
}
},
"supports": {
Expand Down
3 changes: 3 additions & 0 deletions packages/block-library/src/media-text/constants.js
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
/**
* WordPress dependencies
*/
export const DEFAULT_MEDIA_SIZE_SLUG = 'full';
55 changes: 54 additions & 1 deletion packages/block-library/src/media-text/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import { map, filter } from 'lodash';
*/
import { __, _x } from '@wordpress/i18n';
import { useSelect } from '@wordpress/data';
import { useInstanceId } from '@wordpress/compose';
import { useState, useRef } from '@wordpress/element';
import {
BlockControls,
BlockVerticalAlignmentControl,
__experimentalUseInnerBlocksProps as useInnerBlocksProps,
InspectorControls,
useBlockProps,
useSetting,
__experimentalImageURLInputUI as ImageURLInputUI,
__experimentalImageSizeControl as ImageSizeControl,
store as blockEditorStore,
Expand All @@ -28,6 +30,9 @@ import {
ToolbarButton,
ExternalLink,
FocalPointPicker,
__experimentalUnitControl as UnitControl,
__experimentalUseCustomUnits as useCustomUnits,
BaseControl,
} from '@wordpress/components';
import { pullLeft, pullRight } from '@wordpress/icons';
import { store as coreStore } from '@wordpress/core-data';
Expand Down Expand Up @@ -137,9 +142,13 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
mediaWidth,
rel,
verticalAlignment,
minHeight,
} = attributes;
const mediaSizeSlug = attributes.mediaSizeSlug || DEFAULT_MEDIA_SIZE_SLUG;

const instanceId = useInstanceId( UnitControl );
const inputId = `block-media-text-height-input-${ instanceId }`;

const image = useSelect(
( select ) =>
mediaId && isSelected
Expand All @@ -148,6 +157,24 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
[ isSelected, mediaId ]
);

const CSS_UNITS = useCustomUnits( {
availableUnits: useSetting( 'spacing.units' ) || [
'px',
'em',
'rem',
'vw',
'vh',
],
defaultValues: {
px: '430',
'%': '50',
em: '20',
rem: '20',
vw: '20',
vh: '50',
},
} );

const refMediaContainer = useRef();
const imperativeFocalPointPreview = ( value ) => {
const { style } = refMediaContainer.current.resizable;
Expand Down Expand Up @@ -188,6 +215,7 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
const style = {
gridTemplateColumns,
msGridColumns: gridTemplateColumns,
minHeight,
};
const onMediaAltChange = ( newMediaAlt ) => {
setAttributes( { mediaAlt: newMediaAlt } );
Expand Down Expand Up @@ -290,6 +318,28 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {
</PanelBody>
);

const dimensions = (
<PanelBody title={ __( 'Dimensions' ) }>
Copy link
Contributor

Choose a reason for hiding this comment

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

The addition of the “Dimensions” panel could be problematic if padding or margin block support is adopted for this block as it also adds “Dimensions”. The cover block actually has this issue at the moment and there are a couple of PRs open addressing it.

There is also the proposed min-height block support that might reduce the changes needed here to toggling a support flag in the block.json plus the native changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The best idea would then probably be to wait until these things are done?

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be easiest to hold fire until a decision is made on which direction we all wish to take at least.

Whether we choose to inject the ad hoc control within the proposed dimensions panel slot (as per the Cover block) or go with the min-height block support, some changes will be needed.

If there's no rush we can wait and only have to do the job once 🙂

<BaseControl
id={ inputId }
label={ __( 'Minimum height of Media & Text' ) }
>
<UnitControl
id={ inputId }
style={ { maxWidth: 80, marginTop: 8 } }
units={ CSS_UNITS }
value={ minHeight }
step="1"
isResetValueOnUnitChange
min={ 0 }
onChange={ ( value ) => {
setAttributes( { minHeight: value } );
} }
></UnitControl>
</BaseControl>
</PanelBody>
);

const blockProps = useBlockProps( {
className: classNames,
style,
Expand All @@ -302,7 +352,10 @@ function MediaTextEdit( { attributes, isSelected, setAttributes } ) {

return (
<>
<InspectorControls>{ mediaTextGeneralSettings }</InspectorControls>
<InspectorControls>
{ mediaTextGeneralSettings }
{ dimensions }
</InspectorControls>
<BlockControls group="block">
<BlockVerticalAlignmentControl
onChange={ onVerticalAlignmentChange }
Expand Down
3 changes: 2 additions & 1 deletion packages/block-library/src/media-text/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ class MediaTextEdit extends Component {
mediaWidth,
mediaType,
verticalAlignment,
minHeight,
} = attributes;
const { containerWidth, isMediaSelected } = this.state;

Expand Down Expand Up @@ -365,7 +366,7 @@ class MediaTextEdit extends Component {
) }
</BlockControls>
<View
style={ containerStyles }
style={ ( containerStyles, minHeight ) }
onLayout={ this.onLayoutChange }
>
<View
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/media-text/save.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export default function save( { attributes } ) {
href,
linkTarget,
rel,
minHeight,
} = attributes;
const mediaSizeSlug = attributes.mediaSizeSlug || DEFAULT_MEDIA_SIZE_SLUG;
const newRel = isEmpty( rel ) ? undefined : rel;
Expand Down Expand Up @@ -86,6 +87,7 @@ export default function save( { attributes } ) {
}
const style = {
gridTemplateColumns,
minHeight,
};
return (
<div { ...useBlockProps.save( { className, style } ) }>
Expand Down