Skip to content

Commit

Permalink
Pass keydown listener into navigable toolbar instead of removing bubb…
Browse files Browse the repository at this point in the history
…lesVirtually from the toolbar group slot

Removing bubblesVirtually from the toolbar group slot prevents any dynamically added toolbar buttons from firing their click event. This can be seen when cropping an image, you press Crop, then the Apply and Cancel buttons get added to the toolbar. Those button events don’t fire properly and is mentioned as an issue in the bubblesVirtually slot fill PR: #19242
  • Loading branch information
jeryj committed Oct 13, 2023
1 parent 484f118 commit 6addc4f
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default function BlockControlsSlot( { group = 'default', ...props } ) {
return null;
}

const slot = <Slot { ...props } fillProps={ fillProps } />;
const slot = <Slot { ...props } bubblesVirtually fillProps={ fillProps } />;

if ( group === 'default' ) {
return slot;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import classnames from 'classnames';
import { __ } from '@wordpress/i18n';
import { hasBlockSupport, store as blocksStore } from '@wordpress/blocks';
import { useSelect } from '@wordpress/data';
import { ESCAPE } from '@wordpress/keycodes';

/**
* Internal dependencies
Expand All @@ -19,40 +20,47 @@ import { store as blockEditorStore } from '../../store';
import { useHasAnyBlockControls } from '../block-controls/use-has-block-controls';

function BlockContextualToolbar( { focusOnMount, isFixed, ...props } ) {
const { blockType, blockEditingMode, hasParents, showParentSelector } =
useSelect( ( select ) => {
const {
getBlockName,
getBlockParents,
getSelectedBlockClientIds,
getBlockEditingMode,
} = select( blockEditorStore );
const { getBlockType } = select( blocksStore );
const selectedBlockClientIds = getSelectedBlockClientIds();
const _selectedBlockClientId = selectedBlockClientIds[ 0 ];
const parents = getBlockParents( _selectedBlockClientId );
const firstParentClientId = parents[ parents.length - 1 ];
const parentBlockName = getBlockName( firstParentClientId );
const parentBlockType = getBlockType( parentBlockName );
const {
blockType,
blockEditingMode,
lastFocus,
hasParents,
showParentSelector,
} = useSelect( ( select ) => {
const {
getBlockName,
getBlockParents,
getLastFocus,
getSelectedBlockClientIds,
getBlockEditingMode,
} = select( blockEditorStore );
const { getBlockType } = select( blocksStore );
const selectedBlockClientIds = getSelectedBlockClientIds();
const _selectedBlockClientId = selectedBlockClientIds[ 0 ];
const parents = getBlockParents( _selectedBlockClientId );
const firstParentClientId = parents[ parents.length - 1 ];
const parentBlockName = getBlockName( firstParentClientId );
const parentBlockType = getBlockType( parentBlockName );

return {
blockType:
_selectedBlockClientId &&
getBlockType( getBlockName( _selectedBlockClientId ) ),
blockEditingMode: getBlockEditingMode( _selectedBlockClientId ),
hasParents: parents.length,
showParentSelector:
parentBlockType &&
getBlockEditingMode( firstParentClientId ) === 'default' &&
hasBlockSupport(
parentBlockType,
'__experimentalParentSelector',
true
) &&
selectedBlockClientIds.length <= 1 &&
getBlockEditingMode( _selectedBlockClientId ) === 'default',
};
}, [] );
return {
blockType:
_selectedBlockClientId &&
getBlockType( getBlockName( _selectedBlockClientId ) ),
blockEditingMode: getBlockEditingMode( _selectedBlockClientId ),
lastFocus: getLastFocus(),
hasParents: parents.length,
showParentSelector:
parentBlockType &&
getBlockEditingMode( firstParentClientId ) === 'default' &&
hasBlockSupport(
parentBlockType,
'__experimentalParentSelector',
true
) &&
selectedBlockClientIds.length <= 1 &&
getBlockEditingMode( _selectedBlockClientId ) === 'default',
};
}, [] );

const isToolbarEnabled =
! blockType ||
Expand All @@ -78,6 +86,12 @@ function BlockContextualToolbar( { focusOnMount, isFixed, ...props } ) {
/* translators: accessibility text for the block toolbar */
aria-label={ __( 'Block tools' ) }
variant={ isFixed ? 'unstyled' : undefined }
onChildrenKeyDown={ ( event ) => {
if ( event.keyCode === ESCAPE && lastFocus?.current ) {
event.preventDefault();
lastFocus.current.focus();
}
} }
{ ...props }
>
<BlockToolbar hideDragHandle={ isFixed } />
Expand Down
30 changes: 30 additions & 0 deletions packages/block-editor/src/components/navigable-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ function NavigableToolbar( {
shouldUseKeyboardFocusShortcut = true,
__experimentalInitialIndex: initialIndex,
__experimentalOnIndexChange: onIndexChange,
onChildrenKeyDown,
...props
} ) {
const ref = useRef();
Expand All @@ -214,6 +215,35 @@ function NavigableToolbar( {
shouldUseKeyboardFocusShortcut
);

useEffect( () => {
const navigableToolbarRef = ref.current;
const toolbarButtons = navigableToolbarRef.querySelectorAll(
'[data-toolbar-item="true"]'
);

if ( onChildrenKeyDown ) {
const handleChildrenKeyDown = ( event ) => {
onChildrenKeyDown( event );
};

toolbarButtons.forEach( ( toolbarButton ) => {
toolbarButton.addEventListener(
'keydown',
handleChildrenKeyDown
);
} );

return () => {
toolbarButtons.forEach( ( toolbarButton ) => {
toolbarButton.removeEventListener(
'keydown',
handleChildrenKeyDown
);
} );
};
}
}, [ onChildrenKeyDown, children ] );

if ( isAccessibleToolbar ) {
return (
<Toolbar label={ props[ 'aria-label' ] } ref={ ref } { ...props }>
Expand Down

0 comments on commit 6addc4f

Please sign in to comment.