Skip to content

Commit

Permalink
[RNMobile] Fix/button radius update more units (#33434)
Browse files Browse the repository at this point in the history
* Fix: Button button value to be stored as a value + unit

* Update the test to include the px values.

* Fix toFixed to work more as expected.

This resulted in somecase the up or down clicks with proper incoments.

* Remove decimalNum propertly from unit control

This unit now gets calculated from the step value.

* Update Button Component to now accept any unit.

* fix toFixed to return a number instead of a string

* Update with comments for more clarity for getDecimal

* Only calculate decimalNum once.

* Make the stepper cell return integers instead of floats for nicer numbers

* Normalize floats to always include leading zero

* Make the code not repeat itself

* Display nicer numbers when the numbers are integers

from 1.00 to 1

* Adjust the intergration test so it passes again

* minor
  • Loading branch information
enejb authored and ceyhun committed Jul 16, 2021
1 parent b4f428e commit 7ee5882
Show file tree
Hide file tree
Showing 12 changed files with 221 additions and 29 deletions.
82 changes: 67 additions & 15 deletions packages/block-library/src/button/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@ import {
} from '@wordpress/block-editor';
import {
PanelBody,
RangeControl,
ToolbarGroup,
ToolbarButton,
LinkSettingsNavigation,
UnitControl,
getValueAndUnit,
BottomSheetSelectControl,
CSS_UNITS,
filterUnitsWithSettings,
} from '@wordpress/components';
import { Component } from '@wordpress/element';
import { withSelect, withDispatch } from '@wordpress/data';
Expand Down Expand Up @@ -88,6 +91,9 @@ class ButtonEdit extends Component {
super( props );
this.onChangeText = this.onChangeText.bind( this );
this.onChangeBorderRadius = this.onChangeBorderRadius.bind( this );
this.onChangeBorderRadiusUnit = this.onChangeBorderRadiusUnit.bind(
this
);
this.onClearSettings = this.onClearSettings.bind( this );
this.onLayout = this.onLayout.bind( this );
this.onSetMaxWidth = this.onSetMaxWidth.bind( this );
Expand All @@ -100,11 +106,15 @@ class ButtonEdit extends Component {
this.onRemove = this.onRemove.bind( this );
this.getPlaceholderWidth = this.getPlaceholderWidth.bind( this );

const borderRadius = props?.attributes?.style?.border?.radius;
const { valueUnit = 'px' } = getValueAndUnit( borderRadius ) || {};

this.state = {
maxWidth: INITIAL_MAX_WIDTH,
isLinkSheetVisible: false,
isButtonFocused: true,
placeholderTextWidth: 0,
borderRadiusUnit: valueUnit,
};

this.linkSettingsActions = [
Expand Down Expand Up @@ -237,17 +247,33 @@ class ButtonEdit extends Component {
}

onChangeBorderRadius( newRadius ) {
const { setAttributes, attributes } = this.props;
const { borderRadiusUnit } = this.state;
const { style } = attributes;
const newStyle = this.getNewStyle( style, newRadius, borderRadiusUnit );

setAttributes( { style: newStyle } );
}

onChangeBorderRadiusUnit( newRadiusUnit ) {
const { setAttributes, attributes } = this.props;
const { style } = attributes;
const newStyle = {
const borderRadius = this.getBorderRadiusValue(
attributes?.style?.border?.radius
);
const newStyle = this.getNewStyle( style, borderRadius, newRadiusUnit );
setAttributes( { style: newStyle } );
this.setState( { borderRadiusUnit: newRadiusUnit } );
}

getNewStyle( style, radius, radiusUnit ) {
return {
...style,
border: {
...style?.border,
radius: newRadius,
radius: `${ radius }${ radiusUnit }`, // Store the value with the unit so that it works as expected.
},
};

setAttributes( { style: newStyle } );
}

onShowLinkSettings() {
Expand Down Expand Up @@ -368,6 +394,14 @@ class ButtonEdit extends Component {
}
}

getBorderRadiusValue( borderRadius, defaultBorderRadius ) {
const valueAndUnit = getValueAndUnit( borderRadius );
if ( Number.isInteger( parseInt( valueAndUnit?.valueToConvert ) ) ) {
return parseFloat( valueAndUnit.valueToConvert );
}
return defaultBorderRadius;
}

render() {
const {
attributes,
Expand All @@ -387,21 +421,31 @@ class ButtonEdit extends Component {
align = 'center',
width,
} = attributes;
const { maxWidth, isButtonFocused, placeholderTextWidth } = this.state;
const {
maxWidth,
isButtonFocused,
placeholderTextWidth,
borderRadiusUnit,
} = this.state;
const { paddingTop: spacing, borderWidth } = styles.defaultButton;

if ( parentWidth === 0 ) {
return null;
}

const borderRadius = buttonStyle?.border?.radius;
const borderRadiusValue = this.getBorderRadiusValue(
borderRadius,
styles.defaultButton.borderRadius
);

const borderRadiusValue = Number.isInteger( borderRadius )
? borderRadius
: styles.defaultButton.borderRadius;
const buttonBorderRadiusValue =
borderRadiusUnit === 'px' || borderRadiusUnit === '%'
? borderRadiusValue
: Math.floor( 14 * borderRadiusValue ); // lets assume that the font size is set to 14px; TO get a nicer preview.
const outlineBorderRadius =
borderRadiusValue > 0
? borderRadiusValue + spacing + borderWidth
buttonBorderRadiusValue > 0
? buttonBorderRadiusValue + spacing + borderWidth
: 0;

// To achieve proper expanding and shrinking `RichText` on iOS, there is a need to set a `minWidth`
Expand Down Expand Up @@ -433,7 +477,7 @@ class ButtonEdit extends Component {
<View onLayout={ this.onLayout }>
{ this.getPlaceholderWidth( placeholderText ) }
<ColorBackground
borderRadiusValue={ borderRadiusValue }
borderRadiusValue={ buttonBorderRadiusValue }
backgroundColor={ backgroundColor }
isSelected={ isSelected }
>
Expand Down Expand Up @@ -504,12 +548,20 @@ class ButtonEdit extends Component {
{ this.getLinkSettings( false ) }
<InspectorControls>
<PanelBody title={ __( 'Border Settings' ) }>
<RangeControl
<UnitControl
label={ __( 'Border Radius' ) }
minimumValue={ MIN_BORDER_RADIUS_VALUE }
maximumValue={ MAX_BORDER_RADIUS_VALUE }
min={ MIN_BORDER_RADIUS_VALUE }
max={ MAX_BORDER_RADIUS_VALUE }
value={ borderRadiusValue }
onChange={ this.onChangeBorderRadius }
onUnitChange={
this.onChangeBorderRadiusUnit
}
unit={ this.state.borderRadiusUnit }
units={ filterUnitsWithSettings(
[ 'px', 'em', 'rem' ],
CSS_UNITS
) }
/>
</PanelBody>
<WidthPanel
Expand Down
81 changes: 81 additions & 0 deletions packages/block-library/src/buttons/test/edit.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/**
* External dependencies
*/
import {
fireEvent,
waitFor,
getEditorHtml,
within,
initializeEditor,
} from 'test/helpers';

/**
* WordPress dependencies
*/
import { getBlockTypes, unregisterBlockType } from '@wordpress/blocks';
import { registerCoreBlocks } from '@wordpress/block-library';

beforeAll( () => {
// Register all core blocks
registerCoreBlocks();
} );

afterAll( () => {
// Clean up registered blocks
getBlockTypes().forEach( ( block ) => {
unregisterBlockType( block.name );
} );
} );

describe( 'when a button is shown', () => {
it( 'adjusts the border radius', async () => {
const initialHtml = `<!-- wp:buttons -->
<div class="wp-block-buttons"><!-- wp:button {"style":{"border":{"radius":"5%"}}} -->
<div class="wp-block-button"><a class="wp-block-button__link" style="border-radius:5%" >Hello</a></div>
<!-- /wp:button --></div>
<!-- /wp:buttons -->`;
const { getByA11yLabel, getByTestId } = await initializeEditor( {
initialHtml,
} );

const buttonsBlock = await waitFor( () =>
getByA11yLabel( /Buttons Block\. Row 1/ )
);
fireEvent.press( buttonsBlock );

// onLayout event has to be explicitly dispatched in BlockList component,
// otherwise the inner blocks are not rendered.
const innerBlockListWrapper = await waitFor( () =>
within( buttonsBlock ).getByTestId( 'block-list-wrapper' )
);
fireEvent( innerBlockListWrapper, 'layout', {
nativeEvent: {
layout: {
width: 100,
},
},
} );

const buttonInnerBlock = await waitFor( () =>
within( buttonsBlock ).getByA11yLabel( /Button Block\. Row 1/ )
);
fireEvent.press( buttonInnerBlock );

const settingsButton = await waitFor( () =>
getByA11yLabel( 'Open Settings' )
);
fireEvent.press( settingsButton );

const radiusSlider = await waitFor( () =>
getByTestId( 'Slider Border Radius' )
);
fireEvent( radiusSlider, 'valueChange', '25' );

const expectedHtml = `<!-- wp:buttons -->
<div class="wp-block-buttons"><!-- wp:button {"style":{"border":{"radius":"25%"}}} -->
<div class="wp-block-button"><a class="wp-block-button__link" href="" style="border-radius:25%">Hello</a></div>
<!-- /wp:button --></div>
<!-- /wp:buttons -->`;
expect( getEditorHtml() ).toBe( expectedHtml );
} );
} );
1 change: 0 additions & 1 deletion packages/block-library/src/column/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ function ColumnEdit( {
onChange={ onChange }
onComplete={ onChangeWidth }
onUnitChange={ onChangeUnit }
decimalNum={ 1 }
value={
getWidths( columns )[ selectedColumnIndex ]
}
Expand Down
1 change: 0 additions & 1 deletion packages/block-library/src/columns/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ function ColumnsEditContainer( {
? 100
: undefined
}
decimalNum={ 1 }
value={ getWidths( innerWidths )[ index ] }
onChange={ ( nextWidth ) => {
onChange( nextWidth, valueUnit, column.clientId );
Expand Down
12 changes: 12 additions & 0 deletions packages/blocks/src/api/test/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@ describe( 'validation', () => {

expect( normalizedLength ).toBe( '50%' );
} );

it( 'adds leading zero to percentage', () => {
const normalizedLength = getNormalizedLength( '.5%' );

expect( normalizedLength ).toBe( '0.5%' );
} );
} );

describe( 'getNormalizedStyleValue()', () => {
Expand All @@ -208,6 +214,12 @@ describe( 'validation', () => {
expect( normalizedValue ).toBe( '44% 0 18em 0' );
} );

it( 'add leading zero to units that have it missing', () => {
const normalizedValue = getNormalizedStyleValue( '.23% .75em' );

expect( normalizedValue ).toBe( '0.23% 0.75em' );
} );

it( 'leaves zero values in calc() expressions alone', () => {
const normalizedValue = getNormalizedStyleValue(
'calc(0em + 5px)'
Expand Down
10 changes: 9 additions & 1 deletion packages/blocks/src/api/validation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,15 @@ export function isEquivalentTextTokens(
* @return {string} Normalized CSS length value.
*/
export function getNormalizedLength( value ) {
return 0 === parseFloat( value ) ? '0' : value;
if ( 0 === parseFloat( value ) ) {
return '0';
}
// Normalize strings with floats to always include a leading zero.
if ( value.indexOf( '.' ) === 0 ) {
return '0' + value;
}

return value;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ export {
default as UnitControl,
useCustomUnits as __experimentalUseCustomUnits,
} from './unit-control';
export {
CSS_UNITS as CSS_UNITS,
filterUnitsWithSettings as filterUnitsWithSettings,
} from './unit-control/utils';
export { default as Disabled } from './disabled';

// Higher-Order Components
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ class BottomSheetStepperCell extends Component {

onIncrementValue() {
const { step, max, onChange, value, decimalNum } = this.props;
const newValue = toFixed( value + step, decimalNum );

let newValue = toFixed( value + step, decimalNum );
newValue =
parseInt( newValue ) === newValue ? parseInt( newValue ) : newValue;
if ( newValue <= max || max === undefined ) {
onChange( newValue );
this.setState( {
Expand All @@ -70,8 +71,9 @@ class BottomSheetStepperCell extends Component {

onDecrementValue() {
const { step, min, onChange, value, decimalNum } = this.props;
const newValue = toFixed( value - step, decimalNum );

let newValue = toFixed( value - step, decimalNum );
newValue =
parseInt( newValue ) === newValue ? parseInt( newValue ) : newValue;
if ( newValue >= min ) {
onChange( newValue );
this.setState( {
Expand Down Expand Up @@ -238,7 +240,7 @@ class BottomSheetStepperCell extends Component {
label={ label }
onChange={ onChange }
defaultValue={ `${ inputValue }` }
value={ inputValue }
value={ value }
min={ min }
step={ 1 }
decimalNum={ decimalNum }
Expand Down
6 changes: 4 additions & 2 deletions packages/components/src/mobile/utils/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ export function removeNonDigit( text, decimalNum ) {
}

export function toFixed( num, decimalNum = 0 ) {
const fixed = Math.pow( 10, decimalNum < 0 ? 0 : decimalNum );
return Math.floor( num * fixed ) / fixed;
const decimalOffset = decimalNum < 0 ? 0 : decimalNum;
return Number.parseFloat(
Number.parseFloat( num ).toFixed( decimalOffset )
);
}
21 changes: 20 additions & 1 deletion packages/components/src/mobile/utils/test/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,26 @@ describe( 'toFixed', () => {

it( 'function returns the number applying `decimalNum`', () => {
const result = toFixed( '123.4567', 2 );
expect( result ).toBe( 123.45 );
expect( result ).toBe( 123.46 );
} );

it( 'function returns the number applying `decimalNum` all point numbers', () => {
const toCheck = [
1.01,
1.02,
1.03,
1.04,
1.05,
1.06,
1.07,
1.08,
1.09,
1.1,
];
toCheck.forEach( ( num ) => {
const result = toFixed( num, 2 );
expect( result ).toBe( num );
} );
} );

it( 'function returns number without decimals if `decimalNum` is negative', () => {
Expand Down
Loading

0 comments on commit 7ee5882

Please sign in to comment.