[Try] Customizable toolbar contents #23613
Conversation
adamziel
added
[Feature] Blocks
|
Size Change: +6.04 kB (0%) Total Size: 1.14 MB
ℹ️ View Unchanged
|
|
Can you clarify the API? How does a block author use it? Why it's different from BlockControls...? |
|
@youknowriad Sure! The testing PR (#23614) provides an actual usage: The difference is as follows:
A possible future for this could be the concept of programmable "Toolbar modes" with the default mode being the current toolbar (with the drag handle, movers, custom controls, and so on). |
|
What happens if someone uses multiple "BlockContentToolbar" components? How do you decide which one to show? How does it work, does it remove the remaining "BlockControls" that are outside it? |
Good question! Currently it would show all of them which means the block needs to be careful about only returning
This logic is responsible for choosing what gets displayed inside of the BlockToolbar: If there are any |
|
Maybe a better API is something like |
|
Hm maybe? I'm digesting if it's weird to have it configurable as a flag on a slot, it substantially changes the behavior - but maybe within the scope of what's reasonable here? I'm not sure - let's try it and reevaluate. |
|
@youknowriad I just updated this PR to use BlockControls, it's not bad actually. |
| @@ -48,6 +54,9 @@ function BlockControlsFill( { controls, children } ) { | |||
| ); | |||
| } | |||
|
|
|||
| const buildSlotName = ( isExpanded ) => | |||
| `BlockControls${ isExpanded ? '-expanded' : '' }`; | |||
There was a problem hiding this comment.
I don't understand why you had to use a different slot name though?
There was a problem hiding this comment.
I was suggesting to use the exact same slot and fills but have some logic on the wrapper components to avoid rendering if there is already an expanded fill on the slot or something like that. Would that be possible?
There was a problem hiding this comment.
How else would I make sure that expanded fills don't land inside the regular slot OR keep the non-expanded ones from replacing the entire content?
There was a problem hiding this comment.
I tried inspecting the list of fills and looking at the isExpanded prop, but then I realized it's just reinventing the slot name.
There was a problem hiding this comment.
Something's wrong with GitHub, I only saw your second comment a few minutes after posting mine.
I was suggesting to use the exact same slot and fills but have some logic on the wrapper components to avoid rendering if there is already an expanded fill on the slot or something like that. Would that be possible?
I agree this would be the perfect solution. At the moment accessing fills outside of the slot is not possible and some refactoring would be required first so I went for the "wrapping slot" approach. Let me see what would it take to make it happen, it seems like virtually bubbling slots have a hook so maybe it wouldn't be that hard after all?
There was a problem hiding this comment.
+1 to trying to get rid of the buildSlotName, as it implies there are countless options, when in fact we have two
There was a problem hiding this comment.
Yes, I'm not sure either whether it's better or not, the current implementation is also decent. It's just that it feels more conceptually correct. (shouldn't block this PR though)
There was a problem hiding this comment.
A small refactoring of bubbles-virtually/fill would make it possible to detect the presence of expanded fills outside of slot:
Before:
gutenberg/packages/components/src/slot-fill/bubbles-virtually/fill.js
Lines 16 to 18 in 9f2ca8b
After:
export default function Fill( { name, children, refProps } ) {
const slot = useSlot( name );
const ref = useRef( { rerender: useForceUpdate(), ...refProps } );With that in place, the following would be possible:
const { fills } = useSlot( 'BlockControls' );
const expandedFills = fills.filter( ( ref ) => ref.current?.isExpanded );So far so good - so we can use different logic for both cases. The problem is rendering only expanded or non-expanded fills here:
<BlockControls.Slot
bubblesVirtually
className="block-editor-block-toolbar__slot"
/>We don't have a granular control over fills at the slot level - bubblesVirtually fill renders itself using portals. So either:
- All BlockControls fills would have to keep track of all other BlockControl fills and handle that internally (sounds awful), OR
- Expanded and non-expanded fills would have to create portals with different target element, for example based on slot name
So we're back to square one in regard to using slots and fills names. There would be a small win here in that we could get rid of the wrapping slot and make it a sibling of the default content instead, but it would still be a separate slot.
There was a problem hiding this comment.
As this is exposing an experimental prop and also just laying the foundation for #23375 's incoming series of PRs I am approving this with some comments.
Most of the comments are related to how we think of this change. The goal is to allow a component to:
- entirely alter the contents of the block controls
- animate the transition to the new contents
My concerns with the naming (__experimentalCustomizableContent) might seem minor but the content of BlockControls is already customizable. What we do is introduce an optional and possible expanded mode for a control.
cc @youknowriad
| @@ -48,6 +54,9 @@ function BlockControlsFill( { controls, children } ) { | |||
| ); | |||
| } | |||
|
|
|||
| const buildSlotName = ( isExpanded ) => | |||
| `BlockControls${ isExpanded ? '-expanded' : '' }`; | |||
There was a problem hiding this comment.
+1 to trying to get rid of the buildSlotName, as it implies there are countless options, when in fact we have two
|
|
||
| const fillsPropRef = useRef(); | ||
| fillsPropRef.current = fills; | ||
| const resize = useCallback( |
There was a problem hiding this comment.
I think maybe we should name this to resizeToolbarChild or something as it too generic right now.
| } | ||
| elem.style.position = 'absolute'; | ||
| elem.style.width = 'auto'; | ||
| const css = window.getComputedStyle( elem, null ); |
There was a problem hiding this comment.
Like the resize comment above css and elem should point to what they are when resize is used. The reason is that this function is only used in one place and it's specific to what we do here so reading it should lend value to that aspect.
| ); | ||
|
|
||
| useEffect( () => { | ||
| // Create an observer instance linked to the callback function |
There was a problem hiding this comment.
This comment is describing what the next line does, we can remove it
| } | ||
| } ); | ||
|
|
||
| // Start observing the target node for configured mutations |
There was a problem hiding this comment.
same, this comment is describing what the next line does, we can remove it
| classNames="block-editor-block-toolbar-content" | ||
| > | ||
| <div className={ className } ref={ fillsRef }> | ||
| { fills[ 0 ] } |
There was a problem hiding this comment.
Nit pick, maybe make a const displayFill = fills[ 0 ] higher in the code and then use it here?
| export default function BlockToolbar( { hideDragHandle } ) { | ||
| export default function BlockToolbar( { | ||
| hideDragHandle, | ||
| __experimentalCustomizableContent = false, |
There was a problem hiding this comment.
Considering what it does __experimentalCustomizableContent could be named __experimentalExpandedControl
|
Let's keep this PR for after beta1 (next release) |
|
Since packages for beta1 were just published, I'm going to merge this PR |
Description
This PR makes the block toolbar contents customizable via a slot and is a first step towards resolving #23375.
It adds the way to customize toolbar contents, essentially replacing all the default toolbar contents. After implementing the transition between the normal and the customized state, I thought it would be cool if selecting another block would also resize the toolbar smoothly - so now it does that too:
How has this been tested?
This PR only adds the infrastructure. To actually test, you will need to apply both this PR and #23614. Then:
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: