Typography Panel in the Block Inspector for `core/paragraph` never goes away even with all features disabled #61231

kraftner · 2024-04-30T10:11:46Z

Description

If you disable all typography features the corresponding panel should completely disappear. Unfortunately for paragraph blocks this never happens.

The reason for this is that the <DropCapControl> always triggers the rendering of the panel. This comes from the fact that if you add an <InspectorControl> with a group it renders the panel even if the only content is null. So to conditionally render the control you must not even return the <InspectorControl>.

But this PR moved the check inside the <InspectorControl> dc95863#diff-6fffd1b2241c181cc9077eb4508a03fe473b7bf22078a72ba9b2b2dfebd8fb92L101-R134

This makes this a regression in WP 6.5.

Step-by-step reproduction instructions

Disable all typography settings in theme.json

   "typography": {
	"fontSizes": [],
	"customFontSize": false,
	"defaultFontSizes": false,
	"writingMode": false,
	"dropCap": false,
	"textDecoration": false,
	"letterSpacing": false,
	"textTransform": false,
	"fontWeight": false,
	"fontStyle": false,
	"lineHeight": false
},

You'll also be fighting against the fact that the font sizes setting doesn't fully go away. I used this workaround to fix this first:

add_filter('wp_theme_json_data_default', function($themeJson) {
    $data = [
        'version'  => 2,
        'settings' => [
            'typography' => [
                'fontSizes' => []
            ]
        ]
    ];
    return $themeJson->update_with($data);
});

Insert a paragraph in the block editor.
You'll still see an empty "Typography" panel.

To further demonstrate that this is the problem you can use this code to trigger the same problem with any other block that would normally not show the Typography Panel if all features are disabled. I used the core/heading block in the following code:

add_action('enqueue_block_editor_assets', function () {
    $script = <<<HTML
    (function () {
        var el = wp.element.createElement;
        wp.hooks.addFilter(
            "editor.BlockEdit",
            "kraftner/non-empty-typography-slot",
            wp.compose.createHigherOrderComponent( function( BlockEdit ) {
                return function( props ) {
                    if(props.name !== 'core/heading'){
                        return el(BlockEdit, props);
                    }
                    return el(
                        wp.element.Fragment,
                        {},
                        el(
                            wp.blockEditor.InspectorControls,
                            { group: "typography" },
                            null
                        ),
                        el(BlockEdit, props),
                    );
                };
            }, 'withInspectorControls' ),
        );
    })();
    HTML;
    wp_add_inline_script('wp-blocks', $script);
});

You'll notice that I added an <InspectorControl> with only null as content but the panel still renders.

Screenshots, screen recording, code snippet

Environment info

WordPress 6.5 with and without Gutenberg 18.2.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

The text was updated successfully, but these errors were encountered:

t-hamano · 2024-04-30T11:28:30Z

I was able to reproduce it.

I've set all the fields defined in the JSON Schema to empty or false, but I can't completely dismiss the panel. On the other hand, the Color panel and Dimension panel can be deleted.

Below is the theme.json I used for testing.

{
	"$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 3,
	"settings": {
		"appearanceTools": true,
		"layout": {
			"contentSize": "840px",
			"wideSize": "1100px"
		},
		"color": {
			"background": false,
			"text": false,
			"link": false
		},
		"typography": {
			"fontSizes": [],
			"customFontSize": false,
			"defaultFontSizes": false,
			"writingMode": false,
			"dropCap": false,
			"textDecoration": false,
			"letterSpacing": false,
			"textTransform": false,
			"fontWeight": false,
			"fontStyle": false,
			"lineHeight": false,
			"textAlign": false,
			"textColumns": false,
			"fontFamilies": [],
			"fluid": false
		},
		"spacing": {
			"margin": false,
			"padding": false
		}
	}
}

kraftner · 2024-04-30T11:34:29Z

As a quick and dirty workaround I've come up with this for now:

$css = <<<HTML
.typography-block-support-panel:has(.components-tools-panel-header + :empty:last-child) {
    display: none;
}        
HTML;

wp_add_inline_style('wp-edit-post', $css);

What it basically does is hide the typography panel if the element following the panel header is empty and at the same time the last child which should only apply with this issue.

ramonjd · 2024-05-03T02:16:27Z

I optimistically threw up a PR to (hopefully) address this:

Paragraph: conditionally render inspector controls in drop cap component #61342

But only after digested @ellatrix's comments to not call useSettings unconditionally.

Maybe <DropCapControl /> needs to be in https://github.com/WordPress/gutenberg/blob/ca99cb90ba86483d594296fb6162daf2eb2f1215/packages/block-editor/src/components/global-styles/typography-panel.js?

ramonjd · 2024-05-03T03:33:01Z

Maybe needs to be in https://github.com/WordPress/gutenberg/blob/ca99cb90ba86483d594296fb6162daf2eb2f1215/packages/block-editor/src/components/global-styles/typography-panel.js?

This is the direction I was thinking:

WIP DON'T MERGE - move drop cap to typography-panel.js #61343

Might be way off as I didn't get time to look at all angles. Hope it helps someone though.

If I get more time I can circle back.

kraftner · 2024-05-03T08:59:48Z

@ramonjd Thanks for the attempts! I think your second attempt, while interesting, also isn't an ideal solution. In contrast to the the other settings in there DropCap is a paragraph-specific setting and hence should live with the block and not the Typography Panel if you ask me.

I thought about it some more and maybe the solution lies in stepping back and looking at the big picture:

Why is the panel rendering in the first place if all it contains is a slot with null?

So I looked into where this seems to be happening. It seems that there is a check for an empty slot, but it only checks for an empty array but doesn't care about what is in that array:

gutenberg/packages/block-editor/src/components/inspector-controls/slot.js

Lines 58 to 60 in 0612581

    
           if ( ! fills?.length ) { 
        
           	return null; 
        
           }

So maybe we should just deepen that check and also bail out if the slot only contains null values?

I then tried to go down the rabbit hole some more and see if that was a general problem with Slots that they do not filter out null.
But this is were I've hit a dead end with my React (debugging) skills for now. It seems to happen somewhere here. Maybe somebody else knows where to look.

ramonjd · 2024-05-03T09:45:18Z

I think your second attempt, while interesting, also isn't an ideal solution. In contrast to the the other settings in there DropCap is a paragraph-specific setting and hence should live with the block and not the Typography Panel if you ask me.

Fair point.

There's an argument that other blocks could still use it, eg. Pullquote, but as it stands it's just for paragraph.

If there's a way to handle it in the ToolsPanel, then it sounds like neater way! Good one!

So maybe we should just deepen that check and also bail out if the slot only contains null values?

I had a quick look and I think the array is populated with ref items, which are the InspectorControls themselves 🤔

~~But it looks like we could test for undefined so maybe something like the following would work:~~

	if ( ! fills?.length && ! fills.every( ( fill ) => !! fill ) ) {
		return null;
	}

Nope, I'm having a great day 😆 Checking for children on the fill doesn't appear to yield anything either.

Might come back when my mind is fresher.

ramonjd · 2024-05-03T12:04:54Z

Something like this might work, but I'm not sure.

Basically checking for null on a React component being called as a function before rendering the JSX.

I think it's bad practice to render a component this way, but maybe only to check for null first?

I'm out of ideas 🤣

Call DropCapEnabled as a function to check for `null` before rendering DropCapControl

diff --git a/packages/block-editor/src/components/inspector-controls/fill.js b/packages/block-editor/src/components/inspector-controls/fill.js
index 456b33af91..c8cc86cb7b 100644
--- a/packages/block-editor/src/components/inspector-controls/fill.js
+++ b/packages/block-editor/src/components/inspector-controls/fill.js
@@ -37,6 +37,11 @@ export default function InspectorControlsFill( {
 	}
 
 	const context = useBlockEditContext();
+
+	if ( ! children ) {
+		return null;
+	}
+
 	const Fill = groups[ group ]?.Fill;
 	if ( ! Fill ) {
 		warning( `Unknown InspectorControls group "${ group }" provided.` );
diff --git a/packages/block-library/src/paragraph/edit.js b/packages/block-library/src/paragraph/edit.js
index 061cb672ae..c1706467c6 100644
--- a/packages/block-library/src/paragraph/edit.js
+++ b/packages/block-library/src/paragraph/edit.js
@@ -47,7 +47,7 @@ function hasDropCapDisabled( align ) {
 	return align === ( isRTL() ? 'left' : 'right' ) || align === 'center';
 }
 
-function DropCapControl( { clientId, attributes, setAttributes } ) {
+function DropCapEnabled() {
 	// Please do not add a useSelect call to the paragraph block unconditionally.
 	// Every useSelect added to a (frequently used) block will degrade load
 	// and type performance. By moving it within InspectorControls, the subscription is
@@ -58,6 +58,10 @@ function DropCapControl( { clientId, attributes, setAttributes } ) {
 		return null;
 	}
 
+	return <></>;
+}
+
+function DropCapControl( { clientId, attributes, setAttributes } ) {
 	const { align, dropCap } = attributes;
 
 	let helpText;
@@ -132,11 +136,19 @@ function ParagraphBlock( {
 				</BlockControls>
 			) }
 			<InspectorControls group="typography">
-				<DropCapControl
-					clientId={ clientId }
-					attributes={ attributes }
-					setAttributes={ setAttributes }
-				/>
+				{ /*
+				 * Check return value of function before rendering JSX element to avoid rendering
+				 * the inspector control, which checks for React children.
+				 * Ideally we shouldn't call a React function component directly,
+				 * so only render component if it passes.
+				 */ }
+				{ null !== DropCapEnabled() ? (
+					<DropCapControl
+						clientId={ clientId }
+						attributes={ attributes }
+						setAttributes={ setAttributes }
+					/>
+				) : null }
 			</InspectorControls>
 			<RichText
 				identifier="content"

t-hamano · 2024-05-03T15:38:55Z

@ramonjd Thank you for all your ideas🙇

If I understand correctly, the root cause of the empty Typography panel appearing is because the InspectorControls is always rendered regardless of whether the Paragraph block supports DropCap or not. In order to conditionally render the InspectorConrols component, we need to run useSettings() inside the Edit component and check if DropCap is supported. However, from a performance perspective, this is not recommended. Is this correct?

I'm also trying to find a way to solve the problem without making changes to the InspectorControl component itself, but I haven't come up with a good idea yet 😅

ramonjd · 2024-05-03T21:30:49Z

I'm also trying to find a way to solve the problem without making changes to the InspectorControl component itself, but I haven't come up with a good idea yet

Thanks for looking. 🍺

This does work, but I haven't checked for side-effects, e.g., if the parent component were to call any of the same hooks so I don't think it's safe according to the warnings in that link I cited.

Maybe the typography hook could mutate the props/attributes for the block? I'm getting desperate 🤣

kraftner added the [Type] Bug An existing feature does not function as intended label Apr 30, 2024

Mamaduka added Needs Testing Needs further testing to be confirmed. [Block] Paragraph Affects the Paragraph Block [Feature] Typography Font and typography-related issues and PRs labels Apr 30, 2024

kraftner mentioned this issue Apr 30, 2024

When font size is turned off in theme.json, the typography panel does not completely hide the font size option. #59732

Open

ramonjd mentioned this issue May 3, 2024

Paragraph: conditionally render inspector controls in drop cap component #61342

Closed

github-actions bot assigned ramonjd May 3, 2024

github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 3, 2024

Mamaduka removed [Status] In Progress Tracking issues with work in progress Needs Testing Needs further testing to be confirmed. labels May 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typography Panel in the Block Inspector for `core/paragraph` never goes away even with all features disabled #61231

Typography Panel in the Block Inspector for `core/paragraph` never goes away even with all features disabled #61231

kraftner commented Apr 30, 2024

t-hamano commented Apr 30, 2024

kraftner commented Apr 30, 2024

ramonjd commented May 3, 2024

ramonjd commented May 3, 2024 •

edited

kraftner commented May 3, 2024

ramonjd commented May 3, 2024 •

edited

ramonjd commented May 3, 2024

t-hamano commented May 3, 2024

ramonjd commented May 3, 2024

Typography Panel in the Block Inspector for core/paragraph never goes away even with all features disabled #61231

Typography Panel in the Block Inspector for core/paragraph never goes away even with all features disabled #61231

Comments

kraftner commented Apr 30, 2024

Description

Step-by-step reproduction instructions

Screenshots, screen recording, code snippet

Environment info

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

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

t-hamano commented Apr 30, 2024

kraftner commented Apr 30, 2024

ramonjd commented May 3, 2024

ramonjd commented May 3, 2024 • edited

kraftner commented May 3, 2024

ramonjd commented May 3, 2024 • edited

ramonjd commented May 3, 2024

t-hamano commented May 3, 2024

ramonjd commented May 3, 2024

Typography Panel in the Block Inspector for `core/paragraph` never goes away even with all features disabled #61231

Typography Panel in the Block Inspector for `core/paragraph` never goes away even with all features disabled #61231

ramonjd commented May 3, 2024 •

edited

ramonjd commented May 3, 2024 •

edited