From 5ca405170742f9690b0ac4ab2b59175cfbcef190 Mon Sep 17 00:00:00 2001 From: Gerardo Pacheco Date: Wed, 13 Dec 2023 18:18:07 +0100 Subject: [PATCH] [Mobile] Fix regressions with wrapper props and font size customization (#56985) * Mobile - Hooks - Use createBlockListBlockFilter * Mobile - Typography - Refactor the code to incorporate the latest changes from its web counterpart * Mobile - BlockList: Apply editor.BlockListBlock filter to fix issue with missing block props, as well as refactoring the getEditWrapperProps logic to use the same approach as its web counterpart * Mobile - Test helpers - Add more global styles data: font sizes and line height * Mobile - Font Size Picker - Improvde the accessibility label for the Font Size selector * Mobile - Paragraph tests - Add test for font size and line height customization * Mobile - Safe guard from an undefined wrapperProps value * Mobile - Fix having the default font sizes when there are theme font sizes available * Mobile - Global styles context test - Remove default font sizes * Mobile - Paragraph tests - Update tests to use modal helpers * Mobile - Paragraph tests - Adds test to check if the available font sizes are the ones expected with no duplicates * Update Changelog --- .../src/components/block-list/block.native.js | 59 +++++--- .../block-editor/src/hooks/index.native.js | 7 +- .../src/hooks/typography.native.js | 64 +++++---- .../test/__snapshots__/edit.native.js.snap | 12 ++ .../src/paragraph/test/edit.native.js | 114 ++++++++++++++++ .../src/font-size-picker/index.native.js | 8 +- .../test/fixtures/theme.native.js | 20 --- .../global-styles-context/utils.native.js | 45 +++--- packages/react-native-editor/CHANGELOG.md | 1 + .../get-global-styles.js | 128 ++++++++++++++++++ 10 files changed, 363 insertions(+), 95 deletions(-) diff --git a/packages/block-editor/src/components/block-list/block.native.js b/packages/block-editor/src/components/block-list/block.native.js index 70a66c445f58f9..027ed12a7483ae 100644 --- a/packages/block-editor/src/components/block-list/block.native.js +++ b/packages/block-editor/src/components/block-list/block.native.js @@ -2,6 +2,7 @@ * External dependencies */ import { Pressable, View } from 'react-native'; +import classnames from 'classnames'; /** * WordPress dependencies @@ -12,6 +13,7 @@ import { getMergedGlobalStyles, useMobileGlobalStylesColors, useGlobalStyles, + withFilters, } from '@wordpress/components'; import { __experimentalGetAccessibleBlockLabel as getAccessibleBlockLabel, @@ -42,20 +44,36 @@ import { useSettings } from '../use-settings'; const EMPTY_ARRAY = []; -// Helper function to memoize the wrapperProps since getEditWrapperProps always returns a new reference. -const wrapperPropsCache = new WeakMap(); -const emptyObj = {}; -function getWrapperProps( value, getWrapperPropsFunction ) { - if ( ! getWrapperPropsFunction ) { - return emptyObj; +/** + * Merges wrapper props with special handling for classNames and styles. + * + * @param {Object} propsA + * @param {Object} propsB + * + * @return {Object} Merged props. + */ +function mergeWrapperProps( propsA, propsB ) { + const newProps = { + ...propsA, + ...propsB, + }; + + // May be set to undefined, so check if the property is set! + if ( + propsA?.hasOwnProperty( 'className' ) && + propsB?.hasOwnProperty( 'className' ) + ) { + newProps.className = classnames( propsA.className, propsB.className ); } - const cachedValue = wrapperPropsCache.get( value ); - if ( ! cachedValue ) { - const wrapperProps = getWrapperPropsFunction( value ); - wrapperPropsCache.set( value, wrapperProps ); - return wrapperProps; + + if ( + propsA?.hasOwnProperty( 'style' ) && + propsB?.hasOwnProperty( 'style' ) + ) { + newProps.style = { ...propsA.style, ...propsB.style }; } - return cachedValue; + + return newProps; } function BlockWrapper( { @@ -136,6 +154,7 @@ function BlockListBlock( { rootClientId, setAttributes, toggleSelection, + wrapperProps, } ) { const { baseGlobalStyles, @@ -252,12 +271,11 @@ function BlockListBlock( { [ blockWidth, setBlockWidth ] ); - // Block level styles. - let wrapperProps = {}; + // Determine whether the block has props to apply to the wrapper. if ( blockType?.getEditWrapperProps ) { - wrapperProps = getWrapperProps( - attributes, - blockType.getEditWrapperProps + wrapperProps = mergeWrapperProps( + wrapperProps, + blockType.getEditWrapperProps( attributes ) ); } @@ -266,7 +284,7 @@ function BlockListBlock( { return getMergedGlobalStyles( baseGlobalStyles, globalStyle, - wrapperProps.style, + wrapperProps?.style, attributes, defaultColors, name, @@ -284,7 +302,7 @@ function BlockListBlock( { // eslint-disable-next-line react-hooks/exhaustive-deps JSON.stringify( globalStyle ), // eslint-disable-next-line react-hooks/exhaustive-deps - JSON.stringify( wrapperProps.style ), + JSON.stringify( wrapperProps?.style ), // eslint-disable-next-line react-hooks/exhaustive-deps JSON.stringify( Object.fromEntries( @@ -651,5 +669,6 @@ export default compose( // Block is sometimes not mounted at the right time, causing it be undefined // see issue for more info // https://github.com/WordPress/gutenberg/issues/17013 - ifCondition( ( { block } ) => !! block ) + ifCondition( ( { block } ) => !! block ), + withFilters( 'editor.BlockListBlock' ) )( BlockListBlock ); diff --git a/packages/block-editor/src/hooks/index.native.js b/packages/block-editor/src/hooks/index.native.js index 3f1a4473c13891..c0530aedb37ca4 100644 --- a/packages/block-editor/src/hooks/index.native.js +++ b/packages/block-editor/src/hooks/index.native.js @@ -1,18 +1,19 @@ /** * Internal dependencies */ -import { createBlockEditFilter } from './utils'; +import { createBlockEditFilter, createBlockListBlockFilter } from './utils'; import './compat'; import align from './align'; import anchor from './anchor'; import './custom-class-name'; import './generated-class-name'; import style from './style'; -import './color'; -import './font-size'; +import color from './color'; +import fontSize from './font-size'; import './layout'; createBlockEditFilter( [ align, anchor, style ] ); +createBlockListBlockFilter( [ align, style, color, fontSize ] ); export { getBorderClassesAndStyles, useBorderProps } from './use-border-props'; export { getColorClassesAndStyles, useColorProps } from './use-color-props'; diff --git a/packages/block-editor/src/hooks/typography.native.js b/packages/block-editor/src/hooks/typography.native.js index f6fe58edb28703..d8cbf71d84e13f 100644 --- a/packages/block-editor/src/hooks/typography.native.js +++ b/packages/block-editor/src/hooks/typography.native.js @@ -1,10 +1,8 @@ /** * WordPress dependencies */ -import { hasBlockSupport } from '@wordpress/blocks'; -/** - * External dependencies - */ +import { useSelect } from '@wordpress/data'; +import { pure } from '@wordpress/compose'; import { PanelBody } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; @@ -12,17 +10,12 @@ import { __ } from '@wordpress/i18n'; * Internal dependencies */ import InspectorControls from '../components/inspector-controls'; +import { useHasTypographyPanel } from '../components/global-styles/typography-panel'; -import { - LINE_HEIGHT_SUPPORT_KEY, - LineHeightEdit, - useIsLineHeightDisabled, -} from './line-height'; -import { - FONT_SIZE_SUPPORT_KEY, - FontSizeEdit, - useIsFontSizeDisabled, -} from './font-size'; +import { store as blockEditorStore } from '../store'; + +import { LINE_HEIGHT_SUPPORT_KEY, LineHeightEdit } from './line-height'; +import { FONT_SIZE_SUPPORT_KEY, FontSizeEdit } from './font-size'; export const TYPOGRAPHY_SUPPORT_KEY = 'typography'; export const TYPOGRAPHY_SUPPORT_KEYS = [ @@ -30,11 +23,26 @@ export const TYPOGRAPHY_SUPPORT_KEYS = [ FONT_SIZE_SUPPORT_KEY, ]; -export function TypographyPanel( props ) { - const isDisabled = useIsTypographyDisabled( props ); - const isSupported = hasTypographySupport( props.name ); - - if ( isDisabled || ! isSupported ) return null; +function TypographyPanelPure( { clientId, setAttributes, settings } ) { + function selector( select ) { + const { style, fontFamily, fontSize } = + select( blockEditorStore ).getBlockAttributes( clientId ) || {}; + return { style, fontFamily, fontSize }; + } + const { style, fontSize } = useSelect( selector, [ clientId ] ); + const isEnabled = useHasTypographyPanel( settings ); + + if ( ! isEnabled ) { + return null; + } + + const props = { + attributes: { + fontSize, + style, + }, + setAttributes, + }; return ( @@ -46,17 +54,7 @@ export function TypographyPanel( props ) { ); } -const hasTypographySupport = ( blockName ) => { - return TYPOGRAPHY_SUPPORT_KEYS.some( ( key ) => - hasBlockSupport( blockName, key ) - ); -}; - -function useIsTypographyDisabled( props = {} ) { - const configs = [ - useIsFontSizeDisabled( props ), - useIsLineHeightDisabled( props ), - ]; - - return configs.filter( Boolean ).length === configs.length; -} +// We don't want block controls to re-render when typing inside a block. `pure` +// will prevent re-renders unless props change, so only pass the needed props +// and not the whole attributes object. +export const TypographyPanel = pure( TypographyPanelPure ); diff --git a/packages/block-library/src/paragraph/test/__snapshots__/edit.native.js.snap b/packages/block-library/src/paragraph/test/__snapshots__/edit.native.js.snap index adc6ab4210efa5..2910d1551ca28b 100644 --- a/packages/block-library/src/paragraph/test/__snapshots__/edit.native.js.snap +++ b/packages/block-library/src/paragraph/test/__snapshots__/edit.native.js.snap @@ -38,3 +38,15 @@ exports[`Paragraph block should render without crashing and match snapshot 1`] = /> `; + +exports[`Paragraph block should set a font size value 1`] = ` +" +

A quick brown fox jumps over the lazy dog.

+" +`; + +exports[`Paragraph block should set a line height value 1`] = ` +" +

A quick brown fox jumps over the lazy dog.

+" +`; diff --git a/packages/block-library/src/paragraph/test/edit.native.js b/packages/block-library/src/paragraph/test/edit.native.js index fdb082246171ba..3d09068c24930c 100644 --- a/packages/block-library/src/paragraph/test/edit.native.js +++ b/packages/block-library/src/paragraph/test/edit.native.js @@ -4,6 +4,7 @@ import { act, addBlock, + dismissModal, getBlock, typeInRichText, fireEvent, @@ -15,6 +16,7 @@ import { within, withFakeTimers, waitForElementToBeRemoved, + waitForModalVisible, } from 'test/helpers'; import Clipboard from '@react-native-clipboard/clipboard'; import TextInputState from 'react-native/Libraries/Components/TextInput/TextInputState'; @@ -687,6 +689,118 @@ describe( 'Paragraph block', () => { ` ); } ); + it( 'should show the expected font sizes values', async () => { + // Arrange + const screen = await initializeEditor( { withGlobalStyles: true } ); + await addBlock( screen, 'Paragraph' ); + + // Act + const paragraphBlock = getBlock( screen, 'Paragraph' ); + fireEvent.press( paragraphBlock ); + const paragraphTextInput = + within( paragraphBlock ).getByPlaceholderText( 'Start writing…' ); + typeInRichText( + paragraphTextInput, + 'A quick brown fox jumps over the lazy dog.' + ); + // Open Block Settings. + fireEvent.press( screen.getByLabelText( 'Open Settings' ) ); + + // Wait for Block Settings to be visible. + const blockSettingsModal = screen.getByTestId( 'block-settings-modal' ); + await waitForModalVisible( blockSettingsModal ); + + // Open Font size settings + fireEvent.press( screen.getByLabelText( 'Font Size, Custom' ) ); + await waitFor( () => screen.getByLabelText( 'Selected: Default' ) ); + + // Assert + const modalContent = within( blockSettingsModal ); + expect( modalContent.getByLabelText( 'Small' ) ).toBeVisible(); + expect( modalContent.getByText( '14px' ) ).toBeVisible(); + expect( modalContent.getByLabelText( 'Medium' ) ).toBeVisible(); + expect( modalContent.getByText( '17px' ) ).toBeVisible(); + expect( modalContent.getByLabelText( 'Large' ) ).toBeVisible(); + expect( modalContent.getByText( '30px' ) ).toBeVisible(); + expect( modalContent.getByLabelText( 'Extra Large' ) ).toBeVisible(); + expect( modalContent.getByText( '40px' ) ).toBeVisible(); + expect( + modalContent.getByLabelText( 'Extra Extra Large' ) + ).toBeVisible(); + expect( modalContent.getByText( '52px' ) ).toBeVisible(); + } ); + + it( 'should set a font size value', async () => { + // Arrange + const screen = await initializeEditor( { withGlobalStyles: true } ); + await addBlock( screen, 'Paragraph' ); + + // Act + const paragraphBlock = getBlock( screen, 'Paragraph' ); + fireEvent.press( paragraphBlock ); + const paragraphTextInput = + within( paragraphBlock ).getByPlaceholderText( 'Start writing…' ); + typeInRichText( + paragraphTextInput, + 'A quick brown fox jumps over the lazy dog.' + ); + // Open Block Settings. + fireEvent.press( screen.getByLabelText( 'Open Settings' ) ); + + // Wait for Block Settings to be visible. + const blockSettingsModal = screen.getByTestId( 'block-settings-modal' ); + await waitForModalVisible( blockSettingsModal ); + + // Open Font size settings + fireEvent.press( screen.getByLabelText( 'Font Size, Custom' ) ); + + // Tap one font size + fireEvent.press( screen.getByLabelText( 'Large' ) ); + + // Dismiss the Block Settings modal. + await dismissModal( blockSettingsModal ); + + // Assert + expect( getEditorHtml() ).toMatchSnapshot(); + } ); + + it( 'should set a line height value', async () => { + // Arrange + const screen = await initializeEditor( { withGlobalStyles: true } ); + await addBlock( screen, 'Paragraph' ); + + // Act + const paragraphBlock = getBlock( screen, 'Paragraph' ); + fireEvent.press( paragraphBlock ); + const paragraphTextInput = + within( paragraphBlock ).getByPlaceholderText( 'Start writing…' ); + typeInRichText( + paragraphTextInput, + 'A quick brown fox jumps over the lazy dog.' + ); + // Open Block Settings. + fireEvent.press( screen.getByLabelText( 'Open Settings' ) ); + + // Wait for Block Settings to be visible. + const blockSettingsModal = screen.getByTestId( 'block-settings-modal' ); + await waitForModalVisible( blockSettingsModal ); + + const lineHeightControl = screen.getByLabelText( /Line Height/ ); + fireEvent.press( + within( lineHeightControl ).getByText( '1.5', { hidden: true } ) + ); + const lineHeightTextInput = within( + lineHeightControl + ).getByDisplayValue( '1.5', { hidden: true } ); + fireEvent.changeText( lineHeightTextInput, '1.8' ); + + // Dismiss the Block Settings modal. + await dismissModal( blockSettingsModal ); + + // Assert + expect( getEditorHtml() ).toMatchSnapshot(); + } ); + it( 'should focus on the previous Paragraph block when backspacing in an empty Paragraph block', async () => { // Arrange const screen = await initializeEditor(); diff --git a/packages/components/src/font-size-picker/index.native.js b/packages/components/src/font-size-picker/index.native.js index 9659c1dbe225fa..1d5d25cbc1b734 100644 --- a/packages/components/src/font-size-picker/index.native.js +++ b/packages/components/src/font-size-picker/index.native.js @@ -62,6 +62,12 @@ function FontSizePicker( { availableUnits: [ 'px', 'em', 'rem' ], } ); + const accessibilityLabel = sprintf( + // translators: %1$s: Font size name e.g. Small + __( 'Font Size, %1$s' ), + selectedOption.name + ); + return ( { - if ( fontSizes[ key ] ) { - normalizedFontSizes[ key ] = fontSizes[ key ]?.map( - ( fontSizeObject ) => { - fontSizeObject.sizePx = getPxFromCssUnit( - fontSizeObject.size, - { - width: dimensions.width, - height: dimensions.height, - fontSize: DEFAULT_FONT_SIZE, - } - ); - return fontSizeObject; - } - ); - } + // Check if 'theme' or 'custom' keys exist and add them to keysToProcess array + if ( fontSizes?.theme ) { + keysToProcess.push( 'theme' ); + } + if ( fontSizes?.custom ) { + keysToProcess.push( 'custom' ); + } + + // If neither 'theme' nor 'custom' exist, add 'default' if it exists + if ( keysToProcess.length === 0 && fontSizes?.default ) { + keysToProcess.push( 'default' ); + } + + keysToProcess.forEach( ( key ) => { + normalizedFontSizes[ key ] = fontSizes[ key ].map( + ( fontSizeObject ) => { + fontSizeObject.sizePx = getPxFromCssUnit( fontSizeObject.size, { + width: dimensions.width, + height: dimensions.height, + fontSize: DEFAULT_FONT_SIZE, + } ); + return fontSizeObject; + } + ); } ); return normalizedFontSizes; diff --git a/packages/react-native-editor/CHANGELOG.md b/packages/react-native-editor/CHANGELOG.md index 33fa3d26e6c0ad..6f4c1ee783e198 100644 --- a/packages/react-native-editor/CHANGELOG.md +++ b/packages/react-native-editor/CHANGELOG.md @@ -15,6 +15,7 @@ For each user feature we should also add a importance categorization label to i - [*] Fix crash when blockType wrapperProps are not defined [#56846] - [*] Guard against an Image block styles crash due to null block values [#56903] - [**] Fix crash when sharing unsupported media types on Android [#56791] +- [**] Fix regressions with wrapper props and font size customization [#56985] ## 1.109.2 - [**] Fix issue related to text color format and receiving in rare cases an undefined ref from `RichText` component [#56686] diff --git a/test/native/integration-test-helpers/get-global-styles.js b/test/native/integration-test-helpers/get-global-styles.js index 1156018358c3ca..405b3d93c5d90f 100644 --- a/test/native/integration-test-helpers/get-global-styles.js +++ b/test/native/integration-test-helpers/get-global-styles.js @@ -41,6 +41,133 @@ const GLOBAL_STYLES_RAW_FEATURES = { ], }, }, + typography: { + fontSizes: { + default: [ + { + name: 'Small', + size: '13px', + slug: 'small', + }, + { + name: 'Medium', + size: '20px', + slug: 'medium', + }, + { + name: 'Large', + size: '36px', + slug: 'large', + }, + { + name: 'Extra Large', + size: '42px', + slug: 'x-large', + }, + ], + theme: [ + { + fluid: false, + name: 'Small', + size: '0.9rem', + slug: 'small', + }, + { + fluid: false, + name: 'Medium', + size: '1.05rem', + slug: 'medium', + }, + { + fluid: { + max: '1.85rem', + min: '1.39rem', + }, + name: 'Large', + size: '1.85rem', + slug: 'large', + }, + { + fluid: { + max: '2.5rem', + min: '1.85rem', + }, + name: 'Extra Large', + size: '2.5rem', + slug: 'x-large', + }, + { + fluid: { + max: '3.27rem', + min: '2.5rem', + }, + name: 'Extra Extra Large', + size: '3.27rem', + slug: 'xx-large', + }, + ], + }, + }, +}; + +const GLOBAL_STYLES_RAW_STYLES = { + color: { + background: 'var(--wp--preset--color--foreground)', + text: 'var(--wp--preset--color--tertiary)', + }, + elements: { + h1: { + typography: { + fontSize: 'var(--wp--preset--font-size--xx-large)', + lineHeight: '1.15', + }, + }, + h2: { + typography: { + fontSize: 'var(--wp--preset--font-size--x-large)', + }, + }, + h3: { + typography: { + fontSize: 'var(--wp--preset--font-size--large)', + }, + }, + h4: { + typography: { + fontSize: + 'clamp(1.1rem, 1.1rem + ((1vw - 0.2rem) * 0.767), 1.5rem)', + }, + }, + h5: { + typography: { + fontSize: 'var(--wp--preset--font-size--medium)', + }, + }, + h6: { + typography: { + fontSize: 'var(--wp--preset--font-size--small)', + }, + }, + heading: { + color: { + text: 'var(--wp--preset--color--tertiary)', + }, + typography: { + fontFamily: 'var(--wp--preset--font-family--heading)', + lineHeight: '1.2', + }, + }, + link: { + color: { + text: 'var(--wp--preset--color--tertiary)', + }, + }, + }, + typography: { + fontFamily: 'var(--wp--preset--font-family--body)', + fontSize: 'var(--wp--preset--font-size--medium)', + lineHeight: '1.55', + }, }; /** @@ -51,5 +178,6 @@ const GLOBAL_STYLES_RAW_FEATURES = { export function getGlobalStyles() { return { rawFeatures: JSON.stringify( GLOBAL_STYLES_RAW_FEATURES ), + rawStyles: JSON.stringify( GLOBAL_STYLES_RAW_STYLES ), }; }