Skip to content

Consolidate getBlockEditingMode to return 'contentOnly' or 'disabled' while in NavMode #71119

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.

Sign up for GitHub

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

Closed
jeryj wants to merge 5 commits into from
Closed

Consolidate getBlockEditingMode to return 'contentOnly' or 'disabled' while in NavMode #71119

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1c59074
Force getBlockEditingMode to return 'contentOnly' or 'disabled' in is…
jeryj Aug 7, 2025
eb4c13c
Shouldn't use useBlockEditingMode if only getting
jeryj Aug 7, 2025
321a478
Always show black toolbar on contentOnly editing mode for blocks
jeryj Aug 7, 2025
fd1ab6b
Remove manual fallback to contentOnly since it is handled by derivedN…
jeryj Aug 8, 2025
29dbb0d
Test: Can we rely solely on derivedBlockEditingMode?
jeryj Aug 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions packages/block-editor/src/components/block-toolbar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export function PrivateBlockToolbar( {
const {
blockClientId,
blockClientIds,
isContentOnlyMode,
isDefaultEditingMode,
blockType,
toolbarKey,
Expand All @@ -75,7 +76,6 @@ export function PrivateBlockToolbar( {
showLockButtons,
showSwitchSectionStyleButton,
hasFixedToolbar,
isNavigationMode,
} = useSelect( ( select ) => {
const {
getBlockName,
Expand All @@ -90,7 +90,6 @@ export function PrivateBlockToolbar( {
getSettings,
getParentSectionBlock,
isZoomOut,
isNavigationMode: _isNavigationMode,
isSectionBlock,
} = unlock( select( blockEditorStore ) );
const selectedBlockClientIds = getSelectedBlockClientIds();
Expand All @@ -101,8 +100,8 @@ export function PrivateBlockToolbar( {
const parentBlockName = getBlockName( parentClientId );
const parentBlockType = getBlockType( parentBlockName );
const editingMode = getBlockEditingMode( selectedBlockClientId );
const isNavigationModeEnabled = _isNavigationMode();
const _isDefaultEditingMode = editingMode === 'default';
const _isContentOnlyMode = editingMode === 'contentOnly';
const _blockName = getBlockName( selectedBlockClientId );
const isValid = selectedBlockClientIds.every( ( id ) =>
isBlockValid( id )
Expand Down Expand Up @@ -138,7 +137,7 @@ export function PrivateBlockToolbar( {
showParentSelector:
! _isZoomOut &&
parentBlockType &&
editingMode !== 'contentOnly' &&
! _isContentOnlyMode &&
getBlockEditingMode( parentClientId ) !== 'disabled' &&
hasBlockSupport(
parentBlockType,
Expand All @@ -153,13 +152,12 @@ export function PrivateBlockToolbar( {
showSlots: ! _isZoomOut,
showGroupButtons: ! _isZoomOut,
showLockButtons: ! _isZoomOut,
isContentOnlyMode: _isContentOnlyMode,
showSwitchSectionStyleButton:
_isZoomOut ||
( isNavigationModeEnabled &&
editingMode === 'contentOnly' &&
( _isContentOnlyMode &&
isSectionBlock( selectedBlockClientId ) ), // Zoom out or Write Mode Section Blocks
hasFixedToolbar: getSettings().hasFixedToolbar,
isNavigationMode: isNavigationModeEnabled,
};
}, [] );

Expand All @@ -186,7 +184,7 @@ export function PrivateBlockToolbar( {
// Shifts the toolbar to make room for the parent block selector.
const classes = clsx( 'block-editor-block-contextual-toolbar', {
'has-parent': showParentSelector,
'is-inverted-toolbar': isNavigationMode && ! hasFixedToolbar,
'is-inverted-toolbar': isContentOnlyMode && ! hasFixedToolbar,
Copy link
Contributor

@talldan talldan Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contentOnly is also used outside of navigation (write) mode:

  • block overrides in synced patterns
  • templateLock contentOnly
  • Post content blocks when editing a post with the 'Show Template'

Those all now have the inverted toolbar after this change, whereas my understanding is that it's only for navigation mode to indicate the different mode.

Before:
Screenshot 2025-08-08 at 5 37 13 pm

After:
Screenshot 2025-08-08 at 5 37 36 pm

Copy link
Contributor Author

@jeryj jeryj Aug 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was my understanding that we'd surface if a block was contentOnly with the toolbar color, but maybe I misunderstood. @richtabor, can you clarify? I could go either way on if the toolbar should turn black or not if it's contentOnly. It'd be surprising to see the black toolbar, but also nice to have a visual indicator to know why the tools are missing.

Copy link
Contributor

@talldan talldan Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that's fine if so.

There might be some contrast issues to work out with the purple icon color (also not sure about the grey text color).

} );

const innerClasses = clsx( 'block-editor-block-toolbar', {
Expand Down
22 changes: 9 additions & 13 deletions packages/block-editor/src/store/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3051,26 +3051,22 @@ export const getBlockEditingMode = createRegistrySelector(

const isNavMode = isNavigationMode( state );

if ( isNavMode ) {
// If the editor *is* in navigation mode, the block editing mode states
// are stored in the derivedNavModeBlockEditingModes map.
return state.derivedNavModeBlockEditingModes?.has( clientId )
? state.derivedNavModeBlockEditingModes.get( clientId )
: 'contentOnly';
Copy link
Contributor

@getdave getdave Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this short circuits the hierarchy / cascade which is:

  • Derived state = "Based on current editor context, this block should be X",
  • Explicit modes = "This block explicitly wants to be Y" (if no derived state),
  • Template locks = "Based on structural constraints, this block should be Z" (if no derived or explicit mode)
  • Default - the default editing mode.

Copy link
Contributor

@talldan talldan Aug 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the ping. TBH, it's difficult to understand this selector after such a long time away (even though it's much simpler than it used to be). I've forgotten all about it at some point.

The change might be ok, but I'm not sure it'll result in much gain. I think most blocks that need contentOnly mode when in nav mode are given it in the derivedNavModeBlockEditingModes, so I expect the fallback might not do much. Hard to say without a lot of testing.

I have #67606 that I want to pick up again, I rebased it recently. I need to understand the changes again. Unfortunately it was still WIP when I left it and I didn't write a PR description. It simplifies the selector even more though by moving all the template locking code out.

}

// If the editor is currently not in navigation mode, check if the clientId
// has an editing mode set in the regular derived map.
// There may be an editing mode set here for synced patterns or in zoomed out
// mode.
if (
! isNavMode &&
state.derivedBlockEditingModes?.has( clientId )
) {
if ( state.derivedBlockEditingModes?.has( clientId ) ) {
return state.derivedBlockEditingModes.get( clientId );
}

// If the editor *is* in navigation mode, the block editing mode states
// are stored in the derivedNavModeBlockEditingModes map.
if (
isNavMode &&
state.derivedNavModeBlockEditingModes?.has( clientId )
) {
return state.derivedNavModeBlockEditingModes.get( clientId );
}

// In normal mode, consider that an explicitly set editing mode takes over.
const blockEditingMode = state.blockEditingModes.get( clientId );
if ( blockEditingMode ) {
Expand Down
11 changes: 6 additions & 5 deletions packages/block-library/src/social-link/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
InspectorControls,
URLPopover,
URLInput,
useBlockEditingMode,
useBlockProps,
store as blockEditorStore,
} from '@wordpress/block-editor';
Expand Down Expand Up @@ -138,16 +137,18 @@ const SocialLinkEdit = ( {
// Use internal state instead of a ref to make sure that the component
// re-renders when the popover's anchor updates.
const [ popoverAnchor, setPopoverAnchor ] = useState( null );
const isContentOnlyMode = useBlockEditingMode() === 'contentOnly';

const { activeVariation } = useSelect(
const { activeVariation, isContentOnlyMode } = useSelect(
( select ) => {
const { getActiveBlockVariation } = select( blocksStore );
const { getActiveBlockVariation, getBlockEditingMode } =
select( blocksStore );
return {
activeVariation: getActiveBlockVariation( name, attributes ),
isContentOnlyMode:
getBlockEditingMode( clientId ) === 'contentOnly',
};
},
[ name, attributes ]
[ name, attributes, clientId ]
);

const { icon, label: socialLinkName } = getSocialService( activeVariation );
Expand Down
Loading