-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Try] Customizable toolbar contents #23613
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
344b0db
Customizable toolbar contents
adamziel 18e0047
Export BlockToolbarContents as __experimentalBlockToolbarContents
adamziel 22877dc
Update package-lock.json
adamziel db16478
Update package-lock.json
adamziel 5d13944
Use BlockControlsSlot instead of a custom BlockToolbarContents slot
adamziel 94d986f
Update package-lock.json
adamziel 64c62e1
rename resize to resizeToolbar
adamziel 749ea03
Renamed symbols for increased readability
adamziel 054eedc
Rename CSS class for increased readability
adamziel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
130 changes: 130 additions & 0 deletions
130
packages/block-editor/src/components/block-toolbar/expanded-block-controls-container.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,130 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { TransitionGroup, CSSTransition } from 'react-transition-group'; | ||
import { throttle } from 'lodash'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useRef, useState, useEffect, useCallback } from '@wordpress/element'; | ||
import warning from '@wordpress/warning'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import BlockControls from '../block-controls'; | ||
|
||
export default function ExpandedBlockControlsContainer( { | ||
children, | ||
className, | ||
} ) { | ||
return ( | ||
<BlockControls.Slot __experimentalIsExpanded> | ||
{ ( fills ) => { | ||
return ( | ||
<ExpandedBlockControlsHandler | ||
className={ className } | ||
fills={ fills } | ||
> | ||
{ children } | ||
</ExpandedBlockControlsHandler> | ||
); | ||
} } | ||
</BlockControls.Slot> | ||
); | ||
} | ||
|
||
function ExpandedBlockControlsHandler( { fills, className = '', children } ) { | ||
const containerRef = useRef(); | ||
const fillsRef = useRef(); | ||
const toolbarRef = useRef(); | ||
const [ dimensions, setDimensions ] = useState( {} ); | ||
|
||
const fillsPropRef = useRef(); | ||
fillsPropRef.current = fills; | ||
const resizeToolbar = useCallback( | ||
throttle( () => { | ||
const toolbarContentElement = fillsPropRef.current.length | ||
? fillsRef.current | ||
: toolbarRef.current; | ||
if ( ! toolbarContentElement ) { | ||
return; | ||
} | ||
toolbarContentElement.style.position = 'absolute'; | ||
toolbarContentElement.style.width = 'auto'; | ||
const contentCSS = window.getComputedStyle( | ||
toolbarContentElement, | ||
null | ||
); | ||
setDimensions( { | ||
width: contentCSS.getPropertyValue( 'width' ), | ||
height: contentCSS.getPropertyValue( 'height' ), | ||
} ); | ||
toolbarContentElement.style.position = ''; | ||
toolbarContentElement.style.width = ''; | ||
}, 100 ), | ||
[] | ||
); | ||
|
||
useEffect( () => { | ||
const observer = new window.MutationObserver( function ( | ||
mutationsList | ||
) { | ||
const hasChildList = mutationsList.find( | ||
( { type } ) => type === 'childList' | ||
); | ||
if ( hasChildList ) { | ||
resizeToolbar(); | ||
} | ||
} ); | ||
|
||
observer.observe( containerRef.current, { | ||
childList: true, | ||
subtree: true, | ||
} ); | ||
|
||
return () => observer.disconnect(); | ||
}, [] ); | ||
|
||
useEffect( () => { | ||
if ( fills.length > 1 ) { | ||
warning( | ||
`${ fills.length } <BlockControls isExpanded> slots were registered but only one may be displayed.` | ||
); | ||
} | ||
}, [ fills.length ] ); | ||
|
||
const displayFill = fills[ 0 ]; | ||
return ( | ||
<div | ||
className="block-editor-block-toolbar-animated-width-container" | ||
ref={ containerRef } | ||
style={ dimensions } | ||
> | ||
<TransitionGroup> | ||
{ displayFill ? ( | ||
<CSSTransition | ||
key="fills" | ||
timeout={ 300 } | ||
classNames="block-editor-block-toolbar-content" | ||
> | ||
<div className={ className } ref={ fillsRef }> | ||
{ displayFill } | ||
</div> | ||
</CSSTransition> | ||
) : ( | ||
<CSSTransition | ||
key="default" | ||
timeout={ 300 } | ||
classNames="block-editor-block-toolbar-content" | ||
> | ||
<div className={ className } ref={ toolbarRef }> | ||
{ children } | ||
</div> | ||
</CSSTransition> | ||
) } | ||
</TransitionGroup> | ||
</div> | ||
); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you had to use a different slot name though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something's wrong with GitHub, I only saw your second comment a few minutes after posting mine.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to trying to get rid of the
buildSlotName
, as it implies there are countless options, when in fact we have twoThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
With that in place, the following would be possible:
So far so good - so we can use different logic for both cases. The problem is rendering only expanded or non-expanded fills here:
We don't have a granular control over fills at the slot level -
bubblesVirtually
fill renders itself using portals. So either: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.