[EuiModal] Wrapping header in H1 if a string is returned

[1Copenut]

Summary

Add a check in the modal_header_title.tsx file to wrap props.children in an H1 if a string is passed into the component. Anything else (not a string) is ignored by this check. Closes #5276.

Checklist

• Check against all themes for compatibility in both light and dark modes
• Checked in mobile
• Checked in Chrome, Safari, Edge, and Firefox
• Props have proper autodocs and playground toggles
• Added documentation
• Checked Code Sandbox works for any docs examples
• Added or updated jest and cypress tests
• Checked for breaking changes and labeled appropriately
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[cee-chen]

I'm not 100% on the typeof children, but I think this should be more robust. I'd try to avoid maping children if possible in the edge case that someone passes an array/map of strings, in which case you'd end up with multiple <h1>s (another accessibility violation)

Suggested change

	const renderChildren = () =>
	React.Children.map(children, (child) => {
	if (typeof child === 'string') return <h1>{child}</h1>;
	return child;
	});
	return (
	<div className={classes} {...rest}>
	{children}
	{renderChildren()}
	const childrenWithHeading = useMemo(() =>
	if (typeof children === 'string') return <h1>{children}</h1>;
	return children;
	}, [children]);
	return (
	<div className={classes} {...rest}>
	{childrenWithHeading}

[1Copenut]

Good call @constancecchen. I refactored using your recommended approach, and found what I think is a better way to reason about children being a string than typeof. Source article here: https://overreacted.io/why-do-react-elements-have-typeof-property/

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5494/

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5494/

[cee-chen]

(continuing convo from #5494 (comment))

Ahh actually I totally forgot about React.isValidElement (https://davidwalsh.name/react-isvalidelement)! I think it's a bit more readable and exactly what we're looking for (distinguishing JSX vs strings):

Suggested change

	const childrenWithHeading = useMemo(() => {
	if (children && !children.hasOwnProperty('$$typeof'))
	return <h1>{children}</h1>;
	return children;
	}, [children]);
	return (
	<div className={classes} {...rest}>
	{children}
	{childrenWithHeading}
	</div>
	);
	return (
	<div className={classes} {...rest}>
	{React.isValidElement(children) ? children : <h1>{children}</h1>}
	</div>
	);

LMK what you think!

[1Copenut]

This feels a lot cleaner, and is the method I was hoping for, but not finding. Works great, and is easy to reason about.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5494/

[cee-chen]

🎉 Love it!! Thanks for the awesome accessibility improvements!

[thompsongl]

LGTM after the changelog update

[thompsongl]

Move up to main

[1Copenut]

Thanks @thompsongl. I'll make this change and merge when tests pass.

[cee-chen]

Don't forget the "Enable auto-merge" option also! It'll auto merge for ya when tests pass

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5494/