Skip to content

Fix: Fit text not working in site editor preview #73020

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
+55 −4
Closed

Fix: Fit text not working in site editor preview #73020

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f81e80f
initial working
jorgefilipecosta Nov 5, 2025
5bbe54a
Clean up
jorgefilipecosta Nov 5, 2025
21b3f74
minimize diff
jorgefilipecosta Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 55 additions & 4 deletions packages/block-editor/src/hooks/fit-text.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,19 @@
*/
import { addFilter } from '@wordpress/hooks';
import { hasBlockSupport } from '@wordpress/blocks';
import { useEffect, useCallback } from '@wordpress/element';
import {
useEffect,
useCallback,
useState,
useLayoutEffect,
} from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
import {
ToggleControl,
__experimentalToolsPanelItem as ToolsPanelItem,
} from '@wordpress/components';
import { useRefEffect } from '@wordpress/compose';

/**
* Internal dependencies
Expand Down Expand Up @@ -60,7 +66,54 @@ function addAttributes( settings ) {
*/
function useFitText( { fitText, name, clientId } ) {
const hasFitTextSupport = hasBlockSupport( name, FIT_TEXT_SUPPORT_KEY );
const blockElement = useBlockElement( clientId );
const refBlockElement = useBlockElement( clientId );
Copy link
Contributor

@youknowriad youknowriad Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if we try to find why useBlockElement is failing to work in this context. Maybe @ellatrix knows.

Copy link
Member Author

@jorgefilipecosta jorgefilipecosta Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I will give another try at finding the cause, this was like a desesperate attemp for a fix 😅
I'm also considering if an alternative like, render a style element after the block get its ref never rerender again and then just inject on that element ref the styles you need may not be a better alternative to what we have. We would never need to query the dom directly and would just use a ref for an element we rendered and control.

Copy link
Contributor

@mcsf mcsf Nov 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took me a while to fish it out, but it's a RichText-specific issue. I believe it's been around since #60544. When the block editor context has isPreviewMode set, RichText will render little more than just the basic content element, and in the process it omits the ref that it received:

https://github.com/WordPress/gutenberg/pull/60544/files#diff-bec072c72a2f961596701aef67f67575b0a3b918135a6587229a0cd79bb3db44R523

This solves it for me:

diff --git a/packages/block-editor/src/components/rich-text/index.js b/packages/block-editor/src/components/rich-text/index.js
index 13188d49fcc..f0c6e756307 100644
--- a/packages/block-editor/src/components/rich-text/index.js
+++ b/packages/block-editor/src/components/rich-text/index.js
@@ -569,6 +569,7 @@ const PublicForwardedRichTextContainer = forwardRef( ( props, ref ) => {
 		} = removeNativeProps( props );
 		return (
 			<Tag
+				ref={ ref }
 				{ ...contentProps }
 				dangerouslySetInnerHTML={ {
 					__html: valueToHTMLString( value, multiline ),

I'll check with Ella to make sure there's no reason not to add the ref.

Copy link
Member

@ellatrix ellatrix Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason, sounds good. Sounds like an oversight

Copy link
Member Author

@jorgefilipecosta jorgefilipecosta Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you a lot for finding this @mcsf and thank you @ellatrix for confirming the fix should be ok.


// Fallback: Try to query the DOM directly when refBlockElement is null
// This is needed because in preview mode useBlockElement always returns null
// that probably is an existing bug but the fix is not obvious so for now lets
// try this workaround on fit text only.
const [ fallbackElement, setFallbackElement ] = useState( null );
useLayoutEffect( () => {
Copy link
Member

@oandregal oandregal Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block painting, but the operation is cheap (finding a couple of DOM nodes), so it should be fine.

if ( ! fitText || ! hasFitTextSupport || ! clientId ) {
return;
}

// If we already have refBlockElement from the ref system, use it
if ( refBlockElement ) {
setFallbackElement( null );
return;
}

// Otherwise, try to query the DOM directly to get the block element
// We need to check both the main document and the editor canvas iframe
let element = document.getElementById( `block-${ clientId }` );

// If not found, check the editor canvas iframe
if ( ! element ) {
const iframe = document.querySelector(
'iframe[name="editor-canvas"]'
Copy link
Member

@oandregal oandregal Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go with this approach, this is fragile. We'd need an e2e test to communicate this works the way it does, catch regressions in case something changes, etc.

);
if ( iframe ) {
try {
const iframeDoc =
iframe.contentDocument ||
iframe.contentWindow?.document;
if ( iframeDoc ) {
element = iframeDoc.getElementById(
`block-${ clientId }`
);
}
} catch ( e ) {}
}
}

if ( element ) {
setFallbackElement( element );
Copy link
Member

@oandregal oandregal Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sets the elements if some was found, but how do we unset it in the preview context? Imagine an element is set, but the next time the hook runs, we don't find an element. Wouldn't be risk using a stale element as fallback?

}
}, [ refBlockElement, clientId, fitText, hasFitTextSupport ] );

// Use whichever element is available
const blockElement = refBlockElement || fallbackElement;

// Monitor block attribute changes
// Any attribute may change the available space.
Expand All @@ -78,7 +131,6 @@ function useFitText( { fitText, name, clientId } ) {
if ( ! blockElement || ! hasFitTextSupport || ! fitText ) {
return;
}

// Get or create style element with unique ID
const styleId = `fit-text-${ clientId }`;
let styleElement = blockElement.ownerDocument.getElementById( styleId );
Expand Down Expand Up @@ -110,7 +162,6 @@ function useFitText( { fitText, name, clientId } ) {
) {
return;
}

// Store current element value for cleanup
const currentElement = blockElement;
const previousVisibility = currentElement.style.visibility;
Expand Down
Loading