Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FontSizePicker: units property does not work in Block Editor #68553

Open
3 of 6 tasks
slaFFik opened this issue Jan 8, 2025 · 5 comments
Open
3 of 6 tasks

FontSizePicker: units property does not work in Block Editor #68553

slaFFik opened this issue Jan 8, 2025 · 5 comments
Labels
[Package] Components /packages/components Storybook Storybook and its stories for components [Type] Developer Documentation Documentation for developers

Comments

@slaFFik
Copy link

slaFFik commented Jan 8, 2025

Description

What problem does this address?

I think units property does not work.
Here is the component properties I have:

<FontSizePicker
	__next40pxDefaultSize
	fontSizes={[
		{
			name: __( 'Normal', 'domain' ),
			size: 16,
			slug: 'normal'
		},
		{
			name: __( 'Big', 'domain' ),
			size: 26,
			slug: 'big'
		},
		{
			name:__( 'Huge', 'domain' ),
			size: 36,
			slug: 'huge'
		}
	]}
	units={[]}
	onChange={( value ) => setAttributes( { text_size: value } )}
	value={attributes.text_size}
/>

I cannot disable units selection in the units property by passing the empty array.
But even if I pass a single value (like units={['pt']}) or multiple items in the array (like units={['px', 'em']} - they are still not picked up by the component.

Styleguid page - https://wordpress.github.io/gutenberg/?path=/docs/components-fontsizepicker--docs - is also failing with Error: allowedUnitValues.includes is not a function error
Image
whenever I try to set custom values here:
Image

Step-by-step reproduction instructions

  1. add the FontSizePicker to the block with the properties from above

Screenshots, screen recording, code snippet

No response

Environment info

WP 6.7.1, Chrome 131, MacOS 15.2

Same behavior with and without Gutenberg 19.9.0.

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes

Please confirm which theme type you used for testing.

  • Block
  • Classic
  • Hybrid (e.g. classic with theme.json)
  • Not sure
@slaFFik slaFFik added the [Type] Bug An existing feature does not function as intended label Jan 8, 2025
@slaFFik
Copy link
Author

slaFFik commented Jan 8, 2025

And UnitControl does not have units property for some reason. But imo it should.

@SainathPoojary
Copy link
Contributor

Hey @slaFFik,

Tried replicating the issue as mentioned above. In my testing, I found that even after passing the units prop to FontSizePicker, the option to change units does not appear.

Edit.js Used:

/**
 * WordPress dependencies
 */
import { __ } from '@wordpress/i18n';
import { FontSizePicker } from '@wordpress/components';
import { useBlockProps } from '@wordpress/block-editor';

export default function Edit( { attributes, setAttributes } ) {
    return (
        <div { ...useBlockProps() }>
            <FontSizePicker
                __next40pxDefaultSize
                fontSizes={ [
                    {
                        name: __( 'Normal' ),
                        size: 16,
                        slug: 'normal',
                    },
                    {
                        name: __( 'Big' ),
                        size: 26,
                        slug: 'big',
                    },
                    {
                        name: __( 'Huge' ),
                        size: 36,
                        slug: 'huge',
                    },
                ] }
                units={ [ 'px', 'em', 'rem', 'vw', 'vh' ] }
                onChange={ ( value ) => setAttributes( { text_size: value } ) }
                value={ attributes.text_size }
            />
        </div>
    );
}
2025-01-09.07-33-46.mp4

@im3dabasia
Copy link
Contributor

Thank you @slaFFik for reporting the issue ,

I was able to reproduce this issue in the storybook for FontSizePicker component.

Below is the reference:
Image

Also a slightly related issue:

The dropdown to select the units does not function as expected. I believe the expected behavior should be that, upon clicking the button, a dropdown of units appears. However, that is not happening currently.

Screen.Recording.2025-01-09.at.10.20.11.AM.mov

@Mamaduka Mamaduka added the [Package] Components /packages/components label Jan 9, 2025
@t-hamano
Copy link
Contributor

t-hamano commented Jan 9, 2025

From what I understand, this component works perfectly according to the spec, but the code, documentation, and Storybook could use some slight improvements.

I think units property does not work. Here is the component properties I have:

I think this is because this component is operating in "unitless mode". If the value or the first font size is not a string, the units prop doesn't work because it's operating in unitless mode. See this code.

In other words, if you give this component props that set hasunits to true, the units prop should work correctly. However, this internal specification is not described in the README.

I was able to reproduce this issue in the storybook for FontSizePicker component.

Because Some stories of this component are running in unitless mode. See this code. Notice that the size field is a number, not a string.

Error: allowedUnitValues.includes is not a function error

Clicking the "Set object" button on the units prop row initializes its value with an empty object, which causes an error here because the includes method does not exist on objects.

We can avoid this critical error by making the following changes, but I'm not sure if this is the ideal approach:

diff --git a/packages/components/src/unit-control/utils.ts b/packages/components/src/unit-control/utils.ts
index c80b34ef3a..0a7c1160d0 100644
--- a/packages/components/src/unit-control/utils.ts
+++ b/packages/components/src/unit-control/utils.ts
@@ -394,7 +394,7 @@ export function filterUnitsWithSettings(
        // Although the `isArray` check shouldn't be necessary (given the signature of
        // this typed function), it's better to stay on the side of caution, since
        // this function may be called from un-typed environments.
-       return Array.isArray( availableUnits )
+       return Array.isArray( availableUnits ) && Array.isArray( allowedUnitValues )
                ? availableUnits.filter( ( unit ) =>
                                allowedUnitValues.includes( unit.value )
                  )

cc @WordPress/gutenberg-components

@t-hamano t-hamano added [Type] Developer Documentation Documentation for developers Storybook Storybook and its stories for components and removed [Type] Bug An existing feature does not function as intended labels Jan 9, 2025
@slaFFik
Copy link
Author

slaFFik commented Jan 9, 2025

@t-hamano Thank you for additional info.

There should be quite a few updates to the docs about all of that.

tl;dr

  • Component: Set the size values with units, so it will be '12px' instead of 12. Only then the units property will work.
  • Readme/Storybook - needs to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components Storybook Storybook and its stories for components [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

No branches or pull requests

5 participants