Skip to content

Commit

Permalink
[Mobile] - RichText - Parse CSS values and avoid setting undefined on…
Browse files Browse the repository at this point in the history 
…es (#47080)

* parseCssUnitToPx - Fix issue with decimals in math expressions

* Mobile - RichText - Parse CSS values from line height values and avoid undefined or NaN values being sent to the native TextInput
  • Loading branch information
Gerardo Pacheco authored Jan 13, 2023
1 parent e7f07a9 commit 2d44f06
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 22 deletions.
4 changes: 3 additions & 1 deletion packages/block-editor/src/utils/parse-css-unit-to-px.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ function isMathExpression( cssUnit ) {
function evalMathExpression( cssUnit ) {
let errorFound = false;
// Convert every part of the expression to px values.
const cssUnitsBits = cssUnit.split( /[+-/*/]/g ).filter( Boolean );
const cssUnitsBits = cssUnit
.split( /(?!^-)[+*\/-](\s?-)?/g )
.filter( Boolean );
for ( const unit of cssUnitsBits ) {
// Standardize the unit to px and extract the value.
const parsedUnit = parseUnit( getPxFromCssUnit( unit ) );
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/utils/test/parse-css-unit-to-px.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ describe( 'getPxFromCssUnit', () => {
[ 'abc', null ],
[ 'console.log("howdy"); + 10px', null ],
[ 'calc(12vw * 10px', null ], // Missing closing bracket.
[ 'calc( 1em + 0.875rem )', '30px' ], // Decimals
];

test.each( testData )( 'getPxFromCssUnit( %s )', ( unit, expected ) => {
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/audio/test/__snapshots__/edit.native.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ exports[`Audio block renders audio block error state without crashing 1`] = `
fontFamily="serif"
fontSize={14}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBackspace={[Function]}
onBlur={[Function]}
Expand Down Expand Up @@ -306,6 +307,7 @@ exports[`Audio block renders audio file without crashing 1`] = `
fontFamily="serif"
fontSize={14}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBackspace={[Function]}
onBlur={[Function]}
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/file/test/__snapshots__/edit.native.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ exports[`File block renders file error state without crashing 1`] = `
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBackspace={[Function]}
onBlur={[Function]}
Expand Down Expand Up @@ -172,6 +173,7 @@ exports[`File block renders file error state without crashing 1`] = `
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
minWidth={40}
onBackspace={[Function]}
Expand Down Expand Up @@ -294,6 +296,7 @@ exports[`File block renders file without crashing 1`] = `
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBackspace={[Function]}
onBlur={[Function]}
Expand Down Expand Up @@ -374,6 +377,7 @@ exports[`File block renders file without crashing 1`] = `
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
minWidth={40}
onBackspace={[Function]}
Expand Down
2 changes: 2 additions & 0 deletions packages/block-library/src/preformatted/test/__snapshots__/edit.native.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ exports[`core/more/edit/native should match snapshot when content is empty 1`] =
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBlur={[Function]}
onChange={[Function]}
Expand Down Expand Up @@ -77,6 +78,7 @@ exports[`core/more/edit/native should match snapshot when content is not empty 1
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBlur={[Function]}
onChange={[Function]}
Expand Down
7 changes: 7 additions & 0 deletions packages/block-library/src/search/test/__snapshots__/edit.native.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ exports[`Search Block renders block with button inside option 1`] = `
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBackspace={[Function]}
onBlur={[Function]}
Expand Down Expand Up @@ -147,6 +148,7 @@ exports[`Search Block renders block with button inside option 1`] = `
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
minWidth={75}
onBackspace={[Function]}
Expand Down Expand Up @@ -226,6 +228,7 @@ exports[`Search Block renders block with icon button option matches snapshot 1`]
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBackspace={[Function]}
onBlur={[Function]}
Expand Down Expand Up @@ -401,6 +404,7 @@ exports[`Search Block renders block with label hidden matches snapshot 1`] = `
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
minWidth={75}
onBackspace={[Function]}
Expand Down Expand Up @@ -480,6 +484,7 @@ exports[`Search Block renders with default configuration matches snapshot 1`] =
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBackspace={[Function]}
onBlur={[Function]}
Expand Down Expand Up @@ -595,6 +600,7 @@ exports[`Search Block renders with default configuration matches snapshot 1`] =
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
minWidth={75}
onBackspace={[Function]}
Expand Down Expand Up @@ -674,6 +680,7 @@ exports[`Search Block renders with no-button option matches snapshot 1`] = `
fontFamily="serif"
fontSize={16}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBackspace={[Function]}
onBlur={[Function]}
Expand Down
62 changes: 45 additions & 17 deletions packages/rich-text/src/component/index.native.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -854,9 +854,13 @@ export class RichText extends Component {
// we compare previous values to refresh the selected font size,
// this is also used when the tag name changes
// e.g Heading block and a level change like h1->h2.
const currentFontSizeStyle = this.getParsedFontSize( style?.fontSize );
const prevFontSizeStyle = this.getParsedFontSize(
prevProps?.style?.fontSize
const currentFontSizeStyle = this.getParsedCssValue(
style?.fontSize,
DEFAULT_FONT_SIZE
);
const prevFontSizeStyle = this.getParsedCssValue(
prevProps?.style?.fontSize,
DEFAULT_FONT_SIZE
);
const isDifferentTag = prevProps.tagName !== tagName;
if (
Expand Down Expand Up @@ -911,16 +915,16 @@ export class RichText extends Component {
};
}

getParsedFontSize( fontSize ) {
getParsedCssValue( value, defaultValue, fontSize = DEFAULT_FONT_SIZE ) {
const { height, width } = Dimensions.get( 'window' );
const cssUnitOptions = { height, width, fontSize: DEFAULT_FONT_SIZE };
const cssUnitOptions = { height, width, fontSize };

if ( ! fontSize ) {
return fontSize;
if ( ! value ) {
return value;
}

const selectedPxValue =
getPxFromCssUnit( fontSize, cssUnitOptions ) ?? DEFAULT_FONT_SIZE;
getPxFromCssUnit( value, cssUnitOptions ) ?? defaultValue;

return parseFloat( selectedPxValue );
}
Expand Down Expand Up @@ -957,38 +961,40 @@ export class RichText extends Component {

// We need to always convert to px units because the selected value
// could be coming from the web where it could be stored as a different unit.
const selectedPxValue = this.getParsedFontSize( newFontSize );
const selectedPxValue = this.getParsedCssValue(
newFontSize,
DEFAULT_FONT_SIZE
);

return selectedPxValue;
}

getLineHeight() {
const { baseGlobalStyles, tagName, lineHeight, style } = this.props;
const { currentFontSize } = this.state;
const tagNameLineHeight =
baseGlobalStyles?.elements?.[ tagName ]?.typography?.lineHeight;
let newLineHeight;

if ( ! this.getIsBlockBasedTheme() ) {
return;
return MIN_LINE_HEIGHT;
}

// For block-based themes, get the default editor line height.
if ( baseGlobalStyles?.typography?.lineHeight && tagName === 'p' ) {
newLineHeight = parseFloat(
baseGlobalStyles?.typography?.lineHeight
);
newLineHeight = baseGlobalStyles?.typography?.lineHeight;
}

// For block-based themes, get the default element line height
// e.g h1, h2.
if ( tagNameLineHeight ) {
newLineHeight = parseFloat( tagNameLineHeight );
newLineHeight = tagNameLineHeight;
}

// For line height values provided from the styles,
// usually from values set from the line height picker.
if ( style?.lineHeight ) {
newLineHeight = parseFloat( style.lineHeight );
newLineHeight = style.lineHeight;
}

// Fall-back to a line height provided from its props (if there's any)
Expand All @@ -997,12 +1003,34 @@ export class RichText extends Component {
newLineHeight = lineHeight;
}

// If it can't be converted into a floating point number is probably a CSS value
if ( ! parseFloat( newLineHeight ) ) {
const parsedLineHeight = this.getParsedCssValue(
newLineHeight,
MIN_LINE_HEIGHT,
currentFontSize
);

// Line height is in pixels
if ( Number.isInteger( parsedLineHeight ) ) {
const pxValue = parsedLineHeight / currentFontSize;
newLineHeight = parseFloat(
parseFloat( pxValue ).toFixed( 2 )
);
} else {
newLineHeight = parsedLineHeight;
}
}

// Check the final value is not over the minimum supported value.
if ( newLineHeight && newLineHeight < MIN_LINE_HEIGHT ) {
if (
! newLineHeight ||
( newLineHeight && newLineHeight < MIN_LINE_HEIGHT )
) {
newLineHeight = MIN_LINE_HEIGHT;
}

return newLineHeight;
return parseFloat( newLineHeight );
}

getIsBlockBasedTheme() {
Expand Down
2 changes: 2 additions & 0 deletions packages/rich-text/src/test/__snapshots__/index.native.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ exports[`<RichText/> Font Size should update the font size when style prop with
fontFamily="serif"
fontSize={12}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBlur={[Function]}
onChange={[Function]}
Expand Down Expand Up @@ -63,6 +64,7 @@ exports[`<RichText/> Font Size should update the font size with decimals when st
fontFamily="serif"
fontSize={13}
isMultiline={false}
lineHeight={1}
maxImagesWidth={200}
onBlur={[Function]}
onChange={[Function]}
Expand Down
33 changes: 29 additions & 4 deletions packages/rich-text/src/test/index.native.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ import RichText from '../component/index.native';
const mockGlobalSettings = (
settings = { fontSize: 'var(--wp--preset--font-size--normal)' }
) => {
const { fontSize } = settings;
const { fontSize, lineHeight } = settings;
const DEFAULT_GLOBAL_STYLES = {
__experimentalGlobalStylesBaseStyles: { typography: { fontSize } },
__experimentalGlobalStylesBaseStyles: {
typography: { fontSize, lineHeight },
},
};
jest.spyOn( select( blockEditorStore ), 'getSettings' ).mockReturnValue(
DEFAULT_GLOBAL_STYLES
Expand Down Expand Up @@ -262,7 +264,9 @@ describe( '<RichText/>', () => {
// Assert.
expect( screen.toJSON() ).toMatchSnapshot();
} );
} );

describe( 'Line height', () => {
it( 'should set the default minimum line height value if the provided value from the styles is lower', () => {
// Arrange.
const expectedLineHeight = 1;
Expand All @@ -272,8 +276,29 @@ describe( '<RichText/>', () => {
<RichText accessibilityLabel={ 'editor' } style={ style } />
);
// Assert.
const actualFontSize = getByLabelText( 'editor' ).props.lineHeight;
expect( actualFontSize ).toBe( expectedLineHeight );
const actualLineHeight =
getByLabelText( 'editor' ).props.lineHeight;
expect( actualLineHeight ).toBe( expectedLineHeight );
} );

it( 'should set a line height from a CSS value', () => {
// Arrange.
const expectedFontSize = 16;
const expectedLineHeight = 1.88;
mockGlobalSettings( {
fontSize: 'min(1em, 2em)',
lineHeight: 'calc( 1em + 0.875rem )',
} );
// Act.
const { getByLabelText } = render(
<RichText accessibilityLabel={ 'editor' } tagName="p" />
);
// Assert.
const actualFontSize = getByLabelText( 'editor' ).props.fontSize;
expect( actualFontSize ).toBe( expectedFontSize );
const actualLineHeight =
getByLabelText( 'editor' ).props.lineHeight;
expect( actualLineHeight ).toBe( expectedLineHeight );
} );
} );
} );

0 comments on commit 2d44f06

Please sign in to comment.