Skip to content

Commit

Permalink
Block gap: Check for splitOnAxis in the onChange callback (#39788)
Browse files Browse the repository at this point in the history
* Check for splitOnAxis in the onChange callback and return a box value object only when the boxvalue control is activated.
Rename vars for clarity
Add some tests for getGapBoxControlValueFromStyle because we call it directly in flow.js (even though the tests for getGapCSSValue cover it indirectly)

* Am I pretty enough for you?
  • Loading branch information
ramonjd committed Mar 28, 2022
1 parent 6a4fd64 commit 77e9bf4
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 20 deletions.
42 changes: 25 additions & 17 deletions packages/block-editor/src/hooks/gap.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,34 +40,36 @@ export function hasGapValue( props ) {
}

/**
* Returns a BoxControl object value from a given blockGap style.
* Returns a BoxControl object value from a given blockGap style value.
* The string check is for backwards compatibility before Gutenberg supported
* split gap values (row and column) and the value was a string n + unit.
*
* @param {string? | Object?} rawBlockGapValue A style object.
* @return {Object?} A value to pass to the BoxControl component.
* @param {string? | Object?} blockGapValue A block gap string or axial object value, e.g., '10px' or { top: '10px', left: '10px'}.
* @return {Object|null} A value to pass to the BoxControl component.
*/
export function getGapValueFromStyle( rawBlockGapValue ) {
if ( ! rawBlockGapValue ) {
return rawBlockGapValue;
export function getGapBoxControlValueFromStyle( blockGapValue ) {
if ( ! blockGapValue ) {
return null;
}

const isValueString = typeof rawBlockGapValue === 'string';
const isValueString = typeof blockGapValue === 'string';
return {
top: isValueString ? rawBlockGapValue : rawBlockGapValue?.top,
left: isValueString ? rawBlockGapValue : rawBlockGapValue?.left,
top: isValueString ? blockGapValue : blockGapValue?.top,
left: isValueString ? blockGapValue : blockGapValue?.left,
};
}

/**
* Returns a CSS value for the `gap` property from a given blockGap style.
*
* @param {string? | Object?} blockGapValue A style object.
* @param {string? | Object?} blockGapValue A block gap string or axial object value, e.g., '10px' or { top: '10px', left: '10px'}.
* @param {string?} defaultValue A default gap value.
* @return {string|null} The concatenated gap value (row and column).
*/
export function getGapCSSValue( blockGapValue, defaultValue = '0' ) {
const blockGapBoxControlValue = getGapValueFromStyle( blockGapValue );
const blockGapBoxControlValue = getGapBoxControlValueFromStyle(
blockGapValue
);
if ( ! blockGapBoxControlValue ) {
return null;
}
Expand Down Expand Up @@ -142,14 +144,22 @@ export function GapEdit( props ) {
return null;
}

const splitOnAxis =
sides && sides.some( ( side ) => AXIAL_SIDES.includes( side ) );

const onChange = ( next ) => {
let blockGap = next;

// If splitOnAxis activated we need to return a BoxControl object to the BoxControl component.
if ( !! next && splitOnAxis ) {
blockGap = { ...getGapBoxControlValueFromStyle( next ) };
}

const newStyle = {
...style,
spacing: {
...style?.spacing,
blockGap: {
...getGapValueFromStyle( next ),
},
blockGap,
},
};

Expand All @@ -171,9 +181,7 @@ export function GapEdit( props ) {
}
};

const splitOnAxis =
sides && sides.some( ( side ) => AXIAL_SIDES.includes( side ) );
const gapValue = getGapValueFromStyle( style?.spacing?.blockGap );
const gapValue = getGapBoxControlValueFromStyle( style?.spacing?.blockGap );

// The BoxControl component expects a full complement of side values.
// Gap row and column values translate to top/bottom and left/right respectively.
Expand Down
26 changes: 25 additions & 1 deletion packages/block-editor/src/hooks/test/gap.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,33 @@
/**
* Internal dependencies
*/
import { getGapCSSValue } from '../gap';
import { getGapCSSValue, getGapBoxControlValueFromStyle } from '../gap';

describe( 'gap', () => {
describe( 'getGapBoxControlValueFromStyle()', () => {
it( 'should return `null` if argument is falsey', () => {
expect( getGapBoxControlValueFromStyle( undefined ) ).toBeNull();
expect( getGapBoxControlValueFromStyle( '' ) ).toBeNull();
} );
it( 'should return box control value from string', () => {
const expectedValue = {
top: '88rem',
left: '88rem',
};
expect( getGapBoxControlValueFromStyle( '88rem' ) ).toEqual(
expectedValue
);
} );
it( 'should return box control value from object', () => {
const blockGapValue = {
top: '222em',
left: '22px',
};
expect( getGapBoxControlValueFromStyle( blockGapValue ) ).toEqual( {
...blockGapValue,
} );
} );
} );
describe( 'getGapCSSValue()', () => {
it( 'should return `null` if argument is falsey', () => {
expect( getGapCSSValue( undefined ) ).toBeNull();
Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/layouts/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { Icon, positionCenter, stretchWide } from '@wordpress/icons';
*/
import useSetting from '../components/use-setting';
import { appendSelectors } from './utils';
import { getGapValueFromStyle } from '../hooks/gap';
import { getGapBoxControlValueFromStyle } from '../hooks/gap';

export default {
name: 'default',
Expand Down Expand Up @@ -110,7 +110,7 @@ export default {
const { contentSize, wideSize } = layout;
const blockGapSupport = useSetting( 'spacing.blockGap' );
const hasBlockGapStylesSupport = blockGapSupport !== null;
const blockGapStyleValue = getGapValueFromStyle(
const blockGapStyleValue = getGapBoxControlValueFromStyle(
style?.spacing?.blockGap
);
const blockGapValue =
Expand Down

0 comments on commit 77e9bf4

Please sign in to comment.