Note: A similar helper was intentionally removed in the past - #67238.

@jsnajdr @WordPress/gutenberg-components - Can we add filtering of fills for slots? I believe this would greatly simplify the work for write mode by giving us an ability to implement a consistent, simpler way to flag which block toolbar items and sidebar inspector tools to show depending on mode. I've implemented block toolbar filtering here as a POC of how it might work in a generic way, so don't read into the specifics of the code too much. Just the higher level:

• Slots can receive a filtering function
• Only render the fills that pass the filtering function
• Block Toolbar -> Passes a filtering function to check to see if a child component with x prop with x value exists. (I set it as category="content", but this is unlikely to be the final name. I think the current recursive filtering function is going to be really inefficient, but the main idea here is if we can allow filtering on slots. )

My first impression is that a ReactNode filtering mechanism like this is not very practical in the general case, since every filtering function would pretty much require the level of complexity that you have in packages/block-editor/src/components/block-controls/slot.js.

What are some alternative approaches that were considered? I'm not super familiar with what you're trying to accomplish, but I would assume the straightforward approach is to have some kind of conditional logic before the fills hit the slot.

Or another approach that comes to mind is to selectively hide things at the CSS level, like with data- attributes.

Or another approach that comes to mind is to selectively hide things at the CSS level, like with data- attributes.

I considered this too, but wouldn't all the weight of the rendering still need to happen? It would be nice to avoid rendering a lot of unnecessary things.

I would assume the straightforward approach is to have some kind of conditional logic before the fills hit the slot.

This is what we're doing now - wrapping many things in isContentOnly checks. But it's very ad hoc at the moment with lots of different approaches. For maintainability and to allow block authors a way to easily hook in, being able to add a prop to the top level item would make their lives a lot easier.

Could we filter block toolbar fills within the fill instead? It looks like it's being wrapped in something...

Still probably not ideal, but wrapping on the fill level avoids needing to add a filtering prop to the slot

Thank you for the info! I believe the Toolbar slots are bubblesVirtually, so I guess I can't use the children prop is a function filtering.

OK, I thought it's the other variant because in one of the earlier commits, now reverted, you added the filter prop only to the "base" slot, not bubblesVirtually. And then removed the bubblesVirtually prop from the Slot instance. That would likely break the block controls, as they wouldn't be able to read the current block context.

But bubblesVirtually has another feature that you might use, called fillProps 🙂

const fillProps = { shouldRender: ( element ) => element?.props.category === 'content' };
function MySlot() {
  return <Slot name="content" bubblesVirtually fillProps={ fillProps } />
}

const shouldAlwaysRender = () => true;
function MyFill( { children } ) {
  return (
    <Fill name="content">
      { ( { shouldRender = shouldAlwaysRender } ) => {
        if ( ! shouldRender( children ) ) {
          return null;
        }
        return children;
      } }
    </Fill>
  );
}

You create a custom Slot which can decide which fills it wants to render, by passing a shouldRender callback. And then you create a custom Fill that calls that callback before rendering anything. It's the Slot that decides what it wants to render, and the custom Fill merely executes what the Slot tells it. That's somewhat similar to the bubblesVirtually case where it's also the Slot that implements the children callback, just in a very different slot.

This is a significant API difference between the two variants of slotfills. After #68056 is merged we could attempt to unify that API.

Ah, perfect! Thank you for the extra info! This is working better.

The next tricky part I'm exploring is if it's possible to drill down into components and check for the existence of a component + prop within a child component of the fill. I think this might be a stretch and is probably not recommended, but the idea is seeing if we could let people add semantics to their <ToolbarButtons /> and then we render only the buttons in the toolbar that are applicable for that mode:

<BlockControls group="default">
	<ToolbarButton category="content">Content Button</ToolbarButton><-- Render
	<ToolbarButton>Regular Button</ToolbarButton><-- Don't render
	<ToolbarGroup><-- Render
		<ToolbarButton category="content"><-- Render
			Nested Content Button
		</ToolbarButton>
		<ToolbarButton>Nested Regular Button</ToolbarButton><-- Don't render
	</ToolbarGroup>
</BlockControls>

This is just one idea. I've thought up a few other approaches, but I like the simplicity for authors to add semantics to their <ToolbarButton />, so I'm pushing a bit to figure out if there's a decent technical solution.

The next tricky part I'm exploring is if it's possible to drill down into components

Inspecting the children components is already not a very good idea, as @aduth and @mirka pointed out, and drilling down into subcomponents makes that even worse 🙂 The problem is that it breaks down as soon as the children are not explicitly in the markup, but are rendered by a component. This is enough to break it completely:

function MyToolbarButton() {
  return <ToolbarButton category="content">Content</ToolbarButton>
}

<BlockControls>
  <MyToolbarButton />
</BlockControls>

This is valid code, but the filtering won't work.

However, even after reading about the other approaches in #58233 (comment), I still don't know how to best make this work.

Inspecting the children components is already not a very good idea, as @aduth and @mirka pointed out, and drilling down into subcomponents makes that even worse 🙂

Yes, I figured as much, but figured it was worth pushing the limit to see if it's even possible :)

However, even after reading about the other approaches in #58233 (comment), I still don't know how to best make this work.

Agreed. I feel like there should be some technical solution to making this work, but I'm also unsure how to get there 🤔

Note the approach in #71058 for restricting formatting controls. If your approach is better and we can have one unified mechanism then we'd need to revert that.

I think they work alongside each other. This category="content" says "we want this component to show when in contentOnly mode" and yours is filtering which format controls should be shown within it.

This filter will always create a new Array. For perf reasons I wonder if there's a way to avoid that 🤔

We can certainly look to memoize.

I had the AI write a performance test case to compare:

Memoized - 5 items: 0.188ms average (1000 iterations)
Non-memoized - 5 items: 0.132ms average (1000 iterations)
Difference: 0.055ms (41.7%)

This is significant! The memoized version is actually 41.7% slower than the non-memoized version for small arrays (5 items).

Instead of the Array.isArray check it's better to use React.Children.toArray, which is a targeted helper that converts children to a valid array.

I wouldn't worry about copying children at all. Every React render creates a tremendous amount of arrays and objects and then dumps them after the render is finished.

Also, the fill is typically used as:

<BlockControlsFill>
  <SomeItem />
  <SomeOther Item />
</BlockControlsFill>

Where the children is a new array on every render. It's quite rare that children would stay constant between renders.

Obviously we can make this a private API for now via Symbols, but don't bother until we've got more feedback.

Yeah, I'm not worried about that specific quite yet. I want to see about the overall approach first and then get into the details.