Add second parameter to createSlotFill to define wrapper element for Slot

[ryanwelcher]

Description

This PR adds an optional second parameter to createSlotFill that allows control over the element that wraps the children. This fixes #6839 and relates to #6867 and feels like a better approach than one I proposed in #6905

How has this been tested?

I have tested via npm run test and using the following:

const { registerPlugin } = wp.plugins;
const { PluginPostStatusInfo } = wp.editPost;

const PluginRender = () => {
	return (
		<PluginPostStatusInfo>
			<p>Hello!</p>
		</PluginPostStatusInfo>
	);
}
registerPlugin( 'my-plugin-slug', { render: PluginRender } )

I am seeing a React warning that is due to adding a ref to a stateless component which is due to using PanelRow as the wrapper component. I think we can address if this approach is deemed suitable.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[ryanwelcher]

cc @gziolo

[youknowriad]

Did you try just replacing the wrapper div with Fragment? I believe the wrapper is necessary in case bubblesVirtually is set to true but maybe we can just remove it in the other case.

[gziolo]

@aduth - can we use Fragment here and omit ref part?

[ryanwelcher]

@gziolo would using Fragment be preferable to passing the component? If so, I can do another PR to that end.

[aduth]

Yes, I don't think we want this to be a meaningful node. Currently it's used as the target for rendering portal children.

gutenberg/components/slot-fill/fill.js

Line 73 in 5ebefc3

return createPortal( children, slot.node );

So, we probably need the ref still (since createPortal requires a DOM node target). While I agree #6839 is a problem, I don't think we want developers to be concerned with controlling this wrapper (since, again, it needn't exist as a meaningful node).

Two things we could explore:

• The bubbleVirtually = false variation of a Fill may not require the wrapper, since it's not using createPortal
  • Though, I'd like to encourage the virtual variation, maybe eventually deprecate bubbleVirtually = false unless this wrapper business becomes too much an issue
• Assign the portal target node as the parent from which the slot is rendered, and avoid the wrapper node.
  • Maybe assign ref to a temporary node, register slot with temporary node's parent, then remove temporary node?

[gziolo]

Should we open an issue based on @aduth's comment and close this one? I don't think the changes proposed in this PR align with how we want to move forward. We definitely want to improve the general experience with Slot & Fill pair and eventually remove the need to wrap Slots with <div />.

[aduth]

See #7231

[gziolo]

Closing this one in favor of #7231. I also think I fixed the original issue raised in #6867, but what @aduth proposes will help us to avoid finding tweaks in the future 👍