-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Block Library: Add width attribute for resizable Column blocks #15499
Changes from 11 commits
0442276
67d2b75
609e787
6880cfa
5bb641c
4eb24e5
5836ad0
9f28c18
9334a4a
16b3ae0
a840fd9
1a9fa24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
<?php | ||
/** | ||
* Temporary compatibility shims for features present in Gutenberg, pending | ||
* upstream commit to the WordPress core source repository. Functions here | ||
* exist only as long as necessary for corresponding WordPress support, and | ||
* each should be associated with a Trac ticket. | ||
* | ||
* @package gutenberg | ||
*/ | ||
|
||
/** | ||
* Filters allowed CSS attributes to include `flex-basis`, included in saved | ||
* markup of the Column block. | ||
* | ||
* @since 5.7.0 | ||
* | ||
* @param string[] $attr Array of allowed CSS attributes. | ||
* | ||
* @return string[] Filtered array of allowed CSS attributes. | ||
*/ | ||
function gutenberg_safe_style_css_column_flex_basis( $attr ) { | ||
$attr[] = 'flex-basis'; | ||
|
||
return $attr; | ||
} | ||
add_filter( 'safe_style_css', 'gutenberg_safe_style_css_column_flex_basis' ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,55 +2,132 @@ | |
* External dependencies | ||
*/ | ||
import classnames from 'classnames'; | ||
import { forEach, find, difference } from 'lodash'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { InnerBlocks, BlockControls, BlockVerticalAlignmentToolbar } from '@wordpress/block-editor'; | ||
import { | ||
InnerBlocks, | ||
BlockControls, | ||
BlockVerticalAlignmentToolbar, | ||
InspectorControls, | ||
} from '@wordpress/block-editor'; | ||
import { PanelBody, RangeControl } from '@wordpress/components'; | ||
import { withDispatch, withSelect } from '@wordpress/data'; | ||
import { compose } from '@wordpress/compose'; | ||
import { __ } from '@wordpress/i18n'; | ||
|
||
const ColumnEdit = ( { attributes, updateAlignment } ) => { | ||
const { verticalAlignment } = attributes; | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { | ||
toWidthPrecision, | ||
getTotalColumnsWidth, | ||
getColumnWidths, | ||
getAdjacentBlocks, | ||
getRedistributedColumnWidths, | ||
} from '../columns/utils'; | ||
|
||
function ColumnEdit( { | ||
attributes, | ||
updateAlignment, | ||
updateWidth, | ||
hasChildBlocks, | ||
} ) { | ||
const { verticalAlignment, width } = attributes; | ||
|
||
const classes = classnames( 'block-core-columns', { | ||
[ `is-vertically-aligned-${ verticalAlignment }` ]: verticalAlignment, | ||
} ); | ||
|
||
const onChange = ( alignment ) => updateAlignment( alignment ); | ||
|
||
return ( | ||
<div className={ classes }> | ||
<BlockControls> | ||
<BlockVerticalAlignmentToolbar | ||
onChange={ onChange } | ||
onChange={ updateAlignment } | ||
value={ verticalAlignment } | ||
/> | ||
</BlockControls> | ||
<InnerBlocks templateLock={ false } /> | ||
<InspectorControls> | ||
<PanelBody title={ __( 'Column Settings' ) }> | ||
<RangeControl | ||
label={ __( 'Percentage width' ) } | ||
value={ width || '' } | ||
onChange={ updateWidth } | ||
min={ 0 } | ||
max={ 100 } | ||
required | ||
allowReset | ||
/> | ||
</PanelBody> | ||
</InspectorControls> | ||
<InnerBlocks | ||
templateLock={ false } | ||
renderAppender={ ( | ||
hasChildBlocks ? | ||
undefined : | ||
() => <InnerBlocks.ButtonBlockAppender /> | ||
) } | ||
/> | ||
</div> | ||
); | ||
}; | ||
} | ||
|
||
export default compose( | ||
withSelect( ( select, { clientId } ) => { | ||
const { getBlockRootClientId } = select( 'core/editor' ); | ||
withSelect( ( select, ownProps ) => { | ||
const { clientId } = ownProps; | ||
const { getBlockOrder } = select( 'core/block-editor' ); | ||
|
||
return { | ||
parentColumnsBlockClientId: getBlockRootClientId( clientId ), | ||
hasChildBlocks: getBlockOrder( clientId ).length > 0, | ||
}; | ||
} ), | ||
withDispatch( ( dispatch, { clientId, parentColumnsBlockClientId } ) => { | ||
withDispatch( ( dispatch, ownProps, registry ) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Any reason you didn't destructure registry There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm personally not a fan of destructuring in an arguments signature, since it loses context of what it is that is being destructured. To a lesser degree, I find it hard to visually distinguish between proper arguments, and properties destructured (attention to the |
||
return { | ||
updateAlignment( alignment ) { | ||
// Update self... | ||
dispatch( 'core/editor' ).updateBlockAttributes( clientId, { | ||
verticalAlignment: alignment, | ||
} ); | ||
updateAlignment( verticalAlignment ) { | ||
const { clientId, setAttributes } = ownProps; | ||
const { updateBlockAttributes } = dispatch( 'core/block-editor' ); | ||
const { getBlockRootClientId } = registry.select( 'core/block-editor' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice little perf tweak There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanks for reminding: I'd meant to reference some commentary at #13899 (comment) which had led to some of this general (but related/required) refactoring. |
||
|
||
// Update own alignment. | ||
setAttributes( { verticalAlignment } ); | ||
|
||
// Reset Parent Columns Block | ||
dispatch( 'core/editor' ).updateBlockAttributes( parentColumnsBlockClientId, { | ||
verticalAlignment: null, | ||
const rootClientId = getBlockRootClientId( clientId ); | ||
updateBlockAttributes( rootClientId, { verticalAlignment: null } ); | ||
}, | ||
updateWidth( width ) { | ||
const { clientId } = ownProps; | ||
const { updateBlockAttributes } = dispatch( 'core/block-editor' ); | ||
const { getBlockRootClientId, getBlocks } = registry.select( 'core/block-editor' ); | ||
|
||
// Constrain or expand siblings to account for gain or loss of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if the computations here could be extracted to a unit-testable pure function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, I expect there's a fair bit of clean-up which could be done here 😅 Perhaps even some reusability between redistribution logic between the individual Column and its parent Columns block. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Still generally more complex than I'd have liked, but improved refactoring and tests in 6880cfa. |
||
// total columns area. | ||
const columns = getBlocks( getBlockRootClientId( clientId ) ); | ||
const adjacentColumns = getAdjacentBlocks( columns, clientId ); | ||
|
||
// The occupied width is calculated as the sum of the new width | ||
// and the total width of blocks _not_ in the adjacent set. | ||
const occupiedWidth = width + getTotalColumnsWidth( | ||
difference( columns, [ | ||
find( columns, { clientId } ), | ||
...adjacentColumns, | ||
] ) | ||
); | ||
|
||
// Compute _all_ next column widths, in case the updated column | ||
// is in the middle of a set of columns which don't yet have | ||
// any explicit widths assigned (include updates to those not | ||
// part of the adjacent blocks). | ||
const nextColumnWidths = { | ||
...getColumnWidths( columns, columns.length ), | ||
[ clientId ]: toWidthPrecision( width ), | ||
...getRedistributedColumnWidths( adjacentColumns, 100 - occupiedWidth, columns.length ), | ||
}; | ||
|
||
forEach( nextColumnWidths, ( nextColumnWidth, columnClientId ) => { | ||
updateBlockAttributes( columnClientId, { width: nextColumnWidth } ); | ||
} ); | ||
}, | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,13 +9,19 @@ import classnames from 'classnames'; | |
import { InnerBlocks } from '@wordpress/block-editor'; | ||
|
||
export default function save( { attributes } ) { | ||
const { verticalAlignment } = attributes; | ||
const { verticalAlignment, width } = attributes; | ||
|
||
const wrapperClasses = classnames( { | ||
[ `is-vertically-aligned-${ verticalAlignment }` ]: verticalAlignment, | ||
} ); | ||
|
||
let style; | ||
if ( Number.isFinite( width ) ) { | ||
style = { flexBasis: width + '%' }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this kses allowed (just to make sure we check as all inline style properties are not allowed)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good catch! It is not. I will need to filter this set, and follow-up with a Trac ticket. https://core.trac.wordpress.org/browser/trunk/src/wp-includes/kses.php?rev=45242#L2057 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Added in 5bb641c |
||
} | ||
|
||
return ( | ||
<div className={ wrapperClasses }> | ||
<div className={ wrapperClasses } style={ style }> | ||
<InnerBlocks.Content /> | ||
</div> | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this change :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you why the sibling inserter is not shown when there's a single column with a single block? It feels like a separate issue but it's made more obvious with this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's the same as the fact that there is no sibling inserter shown at the end of the block list, only the "default appender". The column is effectively the same as the top-level block list.
However, I don't necessary agree that the sibling inserter shouldn't be shown after the last block in a block list. I think there may have been an issue about this at some point?