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

Use UnitControl instead of RangeControl for column width #24711

Merged
merged 25 commits into from
Sep 29, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
770886d
allow using custom units in columns
aristath Aug 20, 2020
0e5c3b0
tweak the variations
aristath Aug 20, 2020
8d61839
Fix deprecation
aristath Aug 21, 2020
dc3b8b7
Add rudimentary units validation
aristath Aug 24, 2020
79d9d25
add extra check for css-var
aristath Aug 24, 2020
770f0c6
shorter is better here
aristath Aug 24, 2020
47552bb
Merge branch 'master' into aristath/try-column-width
aristath Aug 26, 2020
4f89115
Merge branch 'master' into aristath/try-column-width
aristath Aug 27, 2020
d89af25
Switch to using the new UnitControl
aristath Aug 27, 2020
cdafd79
Merge branch 'master' into aristath/try-column-width
aristath Sep 21, 2020
7968226
Merge branch 'master' into aristath/try-column-width
aristath Sep 22, 2020
f40bdbd
Update packages/block-library/src/column/deprecated.js
aristath Sep 22, 2020
c387ce7
Merge remote-tracking branch 'aristath/aristath/try-column-width' int…
aristath Sep 22, 2020
e7050cb
add whitespace
aristath Sep 22, 2020
c33b23c
tweak migration according to the review
aristath Sep 22, 2020
0c852a2
simplify width check
aristath Sep 22, 2020
3e4a3a8
Merge branch 'master' into aristath/try-column-width
aristath Sep 23, 2020
6f00325
Don't allow negative numbers
aristath Sep 23, 2020
7b3c938
Merge branch 'master' into aristath/try-column-width
aristath Sep 28, 2020
48adbb0
Update implementation to fix merge conflict
aristath Sep 28, 2020
9c22eab
Change labelPosition to "side"
aristath Sep 28, 2020
e977fda
change labelPosition to edge
aristath Sep 29, 2020
1f6a2af
Merge branch 'master' into aristath/try-column-width
aristath Sep 29, 2020
8ad0e20
Adjust label x control width for UnitControl in Column (edit)
Sep 29, 2020
8310377
Update snapshot test
Sep 29, 2020
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
4 changes: 1 addition & 3 deletions packages/block-library/src/column/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@
"type": "string"
},
"width": {
"type": "number",
"min": 0,
"max": 100
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

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

It also creates some issues when adding a new column because it still uses number values. See #27756.

}
},
"supports": {
Expand Down
85 changes: 85 additions & 0 deletions packages/block-library/src/column/deprecated.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/**
* External dependencies
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { InnerBlocks } from '@wordpress/block-editor';

const deprecated = [
{
attributes: {
verticalAlignment: {
type: 'string',
},
width: {
type: 'number',
min: 0,
max: 100,
},
},
isEligible( attributes ) {
aristath marked this conversation as resolved.
Show resolved Hide resolved
return attributes.width && 'string' !== typeof attributes.width;
},
save( { attributes } ) {
attributes.width = attributes.width.toString() + '%';
aristath marked this conversation as resolved.
Show resolved Hide resolved
aristath marked this conversation as resolved.
Show resolved Hide resolved
const { verticalAlignment, width } = attributes;

const wrapperClasses = classnames( {
[ `is-vertically-aligned-${ verticalAlignment }` ]: verticalAlignment,
} );

let style;
if ( width ) {
style = { flexBasis: width };
}

return (
<div className={ wrapperClasses } style={ style }>
<InnerBlocks.Content />
</div>
);
},
},
{
attributes: {
aristath marked this conversation as resolved.
Show resolved Hide resolved
verticalAlignment: {
type: 'string',
},
width: {
type: 'string',
},
},
isEligible( attributes ) {
return attributes.width && ! isNaN( attributes.width );
},
migrate( attributes ) {
return {
...attributes,
width: `${ attributes.width }%`,
Copy link
Member

@gziolo gziolo Nov 6, 2020

Choose a reason for hiding this comment

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

@tellthemachines, @ntsekouras - this one might need isEligible check. width might be not set at all. It's very likely we can avoid adding another deprecation in #26757.

};
},
save( { attributes } ) {
const { verticalAlignment, width } = attributes;

const wrapperClasses = classnames( {
[ `is-vertically-aligned-${ verticalAlignment }` ]: verticalAlignment,
} );

let style;
if ( width ) {
style = { flexBasis: width };
}

return (
<div className={ wrapperClasses } style={ style }>
<InnerBlocks.Content />
</div>
);
},
},
];

export default deprecated;
78 changes: 68 additions & 10 deletions packages/block-library/src/column/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
InspectorControls,
__experimentalBlock as Block,
} from '@wordpress/block-editor';
import { PanelBody, RangeControl } from '@wordpress/components';
import { PanelBody, TextControl, Notice } from '@wordpress/components';
import { useSelect, useDispatch } from '@wordpress/data';
import { __ } from '@wordpress/i18n';

Expand Down Expand Up @@ -51,7 +51,63 @@ function ColumnEdit( {
} );
};

const hasWidth = Number.isFinite( width );
const hasWidth = ( isNaN( width ) && width ) || Number.isFinite( width );
aristath marked this conversation as resolved.
Show resolved Hide resolved

const isWidthValid = () => {
const validUnits = [
'fr',
'rem',
'em',
'ex',
'%',
'px',
'cm',
'mm',
'in',
'pt',
'pc',
'ch',
'vh',
'vw',
'vmin',
'vmax',
];

// Return true for values that can be passed-on as-is.
if (
aristath marked this conversation as resolved.
Show resolved Hide resolved
! width ||
'' === width ||
0 === width ||
'0' === width ||
'auto' === width ||
'inherit' === width ||
'initial' === width
) {
return true;
}

// Skip checking if calc() or val().
if (
( 0 <= width.indexOf( 'calc(' ) && 0 <= width.indexOf( ')' ) ) ||
( 0 <= width.indexOf( 'var(--' ) && 0 <= width.indexOf( ')' ) )
) {
return true;
}

// Get the numeric value.
const numericValue = parseFloat( width );

// Get the unit
const unit = width.replace( numericValue, '' );

// Do not allow unitless.
if ( ! unit ) {
return false;
}

// Check the validity of the numeric value and units.
return ! isNaN( numericValue ) && -1 !== validUnits.indexOf( unit );
};

return (
<>
Expand All @@ -63,21 +119,23 @@ function ColumnEdit( {
</BlockControls>
<InspectorControls>
<PanelBody title={ __( 'Column settings' ) }>
<RangeControl
label={ __( 'Percentage width' ) }
<TextControl
label={ __( 'Width' ) }
value={ width || '' }
onChange={ ( nextWidth ) => {
setAttributes( { width: nextWidth } );
} }
min={ 0 }
max={ 100 }
step={ 0.1 }
required
allowReset
placeholder={
width === undefined ? __( 'Auto' ) : undefined
}
/>
{ ! isWidthValid() && (
<Notice status="warning" isDismissible={ false }>
{ __(
'Invalid CSS value. Values should include a number and a unit. Example: 30%, 100px, 20em etc.'
) }
</Notice>
) }
</PanelBody>
</InspectorControls>
<InnerBlocks
Expand All @@ -90,7 +148,7 @@ function ColumnEdit( {
__experimentalTagName={ Block.div }
__experimentalPassedProps={ {
className: classes,
style: hasWidth ? { flexBasis: width + '%' } : undefined,
style: hasWidth ? { flexBasis: width } : undefined,
} }
/>
</>
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/column/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { column as icon } from '@wordpress/icons';
/**
* Internal dependencies
*/
import deprecated from './deprecated';
import edit from './edit';
import metadata from './block.json';
import save from './save';
Expand All @@ -21,4 +22,5 @@ export const settings = {
description: __( 'A single column within a columns block.' ),
edit,
save,
deprecated,
};
4 changes: 2 additions & 2 deletions packages/block-library/src/column/save.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ export default function save( { attributes } ) {
} );

let style;
if ( Number.isFinite( width ) ) {
style = { flexBasis: width + '%' };
if ( width ) {
style = { flexBasis: width };
}

return (
Expand Down
14 changes: 7 additions & 7 deletions packages/block-library/src/columns/variations.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ const variations = [
</SVG>
),
innerBlocks: [
[ 'core/column', { width: 33.33 } ],
[ 'core/column', { width: 66.66 } ],
[ 'core/column', { width: '33.33%' } ],
[ 'core/column', { width: '66.66%' } ],
],
scope: [ 'block' ],
},
Expand All @@ -77,8 +77,8 @@ const variations = [
</SVG>
),
innerBlocks: [
[ 'core/column', { width: 66.66 } ],
[ 'core/column', { width: 33.33 } ],
[ 'core/column', { width: '66.66%' } ],
[ 'core/column', { width: '33.33%' } ],
],
scope: [ 'block' ],
},
Expand Down Expand Up @@ -124,9 +124,9 @@ const variations = [
</SVG>
),
innerBlocks: [
[ 'core/column', { width: 25 } ],
[ 'core/column', { width: 50 } ],
[ 'core/column', { width: 25 } ],
[ 'core/column', { width: '25%' } ],
[ 'core/column', { width: '50%' } ],
[ 'core/column', { width: '25%' } ],
],
scope: [ 'block' ],
},
Expand Down