Skip to content

Commit

Permalink
Fix/column width units (#26757)
Browse files Browse the repository at this point in the history
* Fix issues with different units in column widths

* Columns with fixed width should keep width on recalculation

* Address review feedback
  • Loading branch information
tellthemachines committed Nov 11, 2020
1 parent 55fa198 commit bcdd9f3
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 18 deletions.
3 changes: 2 additions & 1 deletion packages/block-library/src/column/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ function ColumnEdit( {
} );
};

const widthWithUnit = Number.isFinite( width ) ? width + '%' : width;
const blockProps = useBlockProps( {
className: classes,
style: width ? { flexBasis: width } : undefined,
style: widthWithUnit ? { flexBasis: widthWithUnit } : undefined,
} );
const innerBlocksProps = useInnerBlocksProps( blockProps, {
templateLock,
Expand Down
4 changes: 3 additions & 1 deletion packages/block-library/src/column/save.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ export default function save( { attributes } ) {
} );

let style;

if ( width ) {
style = { flexBasis: width };
// Numbers are handled for backward compatibility as they can be still provided with templates.
style = { flexBasis: Number.isFinite( width ) ? width + '%' : width };
}

return (
Expand Down
6 changes: 4 additions & 2 deletions packages/block-library/src/columns/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
* Internal dependencies
*/
import {
hasExplicitColumnWidths,
hasExplicitPercentColumnWidths,
getMappedColumnWidths,
getRedistributedColumnWidths,
toWidthPrecision,
Expand Down Expand Up @@ -145,7 +145,9 @@ const ColumnsEditContainerWrapper = withDispatch(
const { getBlocks } = registry.select( 'core/block-editor' );

let innerBlocks = getBlocks( clientId );
const hasExplicitWidths = hasExplicitColumnWidths( innerBlocks );
const hasExplicitWidths = hasExplicitPercentColumnWidths(
innerBlocks
);

// Redistribute available width for existing inner blocks.
const isAddingColumn = newColumns > previousColumns;
Expand Down
52 changes: 45 additions & 7 deletions packages/block-library/src/columns/test/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
getTotalColumnsWidth,
getColumnWidths,
getRedistributedColumnWidths,
hasExplicitColumnWidths,
hasExplicitPercentColumnWidths,
getMappedColumnWidths,
} from '../utils';

Expand All @@ -18,6 +18,14 @@ describe( 'toWidthPrecision', () => {
expect( value ).toBe( 50.11 );
} );

it( 'should convert a string value with unit to a number', () => {
expect( toWidthPrecision( '33.3%' ) ).toBe( 33.3 );
} );

it( 'should return undefined for an invalid string', () => {
expect( toWidthPrecision( 'blahblah' ) ).toBe( undefined );
} );

it( 'should return undefined for invalid number', () => {
expect( toWidthPrecision( null ) ).toBe( undefined );
expect( toWidthPrecision( undefined ) ).toBe( undefined );
Expand Down Expand Up @@ -156,19 +164,27 @@ describe( 'getRedistributedColumnWidths', () => {
} );
} );

describe( 'hasExplicitColumnWidths', () => {
describe( 'hasExplicitPercentColumnWidths', () => {
it( 'returns false if no blocks have explicit width', () => {
const blocks = [ { attributes: {} } ];

const result = hasExplicitColumnWidths( blocks );
const result = hasExplicitPercentColumnWidths( blocks );

expect( result ).toBe( false );
} );

it( 'returns true if a block has explicit width', () => {
it( 'returns true if a block has explicit width defined as a number', () => {
const blocks = [ { attributes: { width: 100 } } ];

const result = hasExplicitColumnWidths( blocks );
const result = hasExplicitPercentColumnWidths( blocks );

expect( result ).toBe( true );
} );

it( 'returns true if a block has explicit percent width defined as a string', () => {
const blocks = [ { attributes: { width: '100%' } } ];

const result = hasExplicitPercentColumnWidths( blocks );

expect( result ).toBe( true );
} );
Expand All @@ -179,7 +195,7 @@ describe( 'hasExplicitColumnWidths', () => {
{ attributes: { width: undefined } },
];

const result = hasExplicitColumnWidths( blocks );
const result = hasExplicitPercentColumnWidths( blocks );

expect( result ).toBe( false );
} );
Expand All @@ -190,10 +206,32 @@ describe( 'hasExplicitColumnWidths', () => {
{ attributes: { width: 90 } },
];

const result = hasExplicitColumnWidths( blocks );
const result = hasExplicitPercentColumnWidths( blocks );

expect( result ).toBe( true );
} );

it( 'returns true if blocks have width defined as percent strings and numbers', () => {
const blocks = [
{ attributes: { width: '10%' } },
{ attributes: { width: 90 } },
];

const result = hasExplicitPercentColumnWidths( blocks );

expect( result ).toBe( true );
} );

it( 'returns false if blocks have width defined as mixed unit strings', () => {
const blocks = [
{ attributes: { width: '20%' } },
{ attributes: { width: '90px' } },
];

const result = hasExplicitPercentColumnWidths( blocks );

expect( result ).toBe( false );
} );
} );

describe( 'getMappedColumnWidths', () => {
Expand Down
22 changes: 15 additions & 7 deletions packages/block-library/src/columns/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ import { sumBy, merge, mapValues } from 'lodash';
*
* @return {number} Value rounded to standard precision.
*/
export const toWidthPrecision = ( value ) =>
Number.isFinite( value ) ? parseFloat( value.toFixed( 2 ) ) : undefined;

export const toWidthPrecision = ( value ) => {
const unitlessValue = parseFloat( value );
return Number.isFinite( unitlessValue )
? parseFloat( unitlessValue.toFixed( 2 ) )
: undefined;
};
/**
* Returns an effective width for a given block. An effective width is equal to
* its attribute value if set, or a computed value assuming equal distribution.
Expand Down Expand Up @@ -96,10 +99,15 @@ export function getRedistributedColumnWidths(
*
* @return {boolean} Whether columns have explicit widths.
*/
export function hasExplicitColumnWidths( blocks ) {
return blocks.every( ( block ) =>
Number.isFinite( block.attributes.width )
);
export function hasExplicitPercentColumnWidths( blocks ) {
return blocks.every( ( block ) => {
const blockWidth = block.attributes.width;
return Number.isFinite(
blockWidth?.endsWith?.( '%' )
? parseFloat( blockWidth )
: blockWidth
);
} );
}

/**
Expand Down

0 comments on commit bcdd9f3

Please sign in to comment.