Truncate: improve handling of non-string children

[ciampo]

What?

This PR introduces a few changes to the Truncate component to make it more resilient to different types of children received:

• it makes the component able to truncate children of type number
• when children have types other than string or number, the component renders the children without any attempted truncation (instead of not rendering the children at all)

Why?

While working on #55625, I discovered that Truncate only handles children of type string, and otherwise renders an empty <span /> tag.

The component falsely advertises that its children are of type ReactNode, while in reality the component only knows how to deal with string.

This behaviour is not great, and can cause confusion and disruption in the consumers of Truncate

How?

Tweaked the logic of the truncate hook:

• numbers are converted to string so that they can be truncated
• when detecting children that are not string of number type, the component simply no-ops and renders children as-is
• added unit tests for this new behaviour

[ntsekouras]

LGTM, thanks!

[mirka]

Interesting, yes it looks like the Truncate/Text primitives are designed to be quite permissive with the type of their children!

I think the approach here is good (and of course the added tests!), although I'd like to suggest a few more things to mitigate consumer confusion:

• We should mention in the children prop description that truncation won't happen unless it's string/number.
• I noticed that Text actually throws when you try to use the highlight feature with a non-string. Obviously for back-compat reasons we can't start throwing here now, but maybe it merits a warn()? (No strong opinion on this one)

[ciampo]

Thank you for the review, @ntsekouras and @mirka !

I'd like to suggest a few more things to mitigate consumer confusion:

Sounds good, I went ahead and implemented the suggested changes.

[mirka]

I think it's preferable to use @wordpress/warning?

[mirka]

Suggested change

	`Truncate: text truncation has been disabled, since it is only available when passing 'children' of type 'string' or 'number'`
	`wp.components.Truncate: text truncation has been disabled, since it is only available when passing 'children' of type 'string' or 'number'`

Probably better for clarity, since Truncate is used under the hood for a handful of more common components and the consumer may not be aware of what "Truncate" is specifically.

[ciampo]

Looking more into unit tests failures, it emerges that a lot of components use Truncate indirectly via the Text and Heading components.

The problem with that is that the Text component adds some extra logic to its hook which in some cases completely ignores the result of calling useTruncate:

gutenberg/packages/components/src/text/hook.ts

Lines 144 to 183 in 830d13b

	let finalEllipsizeMode: undefined | 'auto' | 'none';
	if ( truncate === true ) {
	finalEllipsizeMode = 'auto';
	}
	if ( truncate === false ) {
	finalEllipsizeMode = 'none';
	}
	const finalComponentProps = {
	...otherProps,
	className: classes,
	children,
	ellipsizeMode: ellipsizeMode || finalEllipsizeMode,
	};
	const truncateProps = useTruncate( finalComponentProps );
	/**
	* Enhance child `<Link />` components to inherit font size.
	*/
	if ( ! truncate && Array.isArray( children ) ) {
	content = Children.map( children, ( child ) => {
	if (
	typeof child !== 'object' ||
	child === null ||
	! ( 'props' in child )
	) {
	return child;
	}
	const isLink = hasConnectNamespace( child, [ 'Link' ] );
	if ( isLink ) {
	return cloneElement( child, {
	size: child.props.size || 'inherit',
	} );
	}
	return child;
	} );
	}

Therefore, I don't think that showing a warning makes a lot of sense because it would just pollute the console for a lot of Text` consumers.

[github-actions]

Flaky tests detected in 23818de.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7286939991
📝 Reported issues:

• [Flaky Test] Should show Columns block only if selected blocks are between limits (1-6) #40369 in specs/editor/various/block-switcher.test.js