-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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.
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
Changes from all commits
1c59074
eb4c13c
321a478
fd1ab6b
29dbb0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3040,62 +3040,22 @@ export function __unstableIsWithinBlockOverlay( state, clientId ) { | |
| * @return {BlockEditingMode} The block editing mode. One of `'disabled'`, | ||
| * `'contentOnly'`, or `'default'`. | ||
| */ | ||
| export const getBlockEditingMode = createRegistrySelector( | ||
| ( select ) => | ||
| ( state, clientId = '' ) => { | ||
| // Some selectors that call this provide `null` as the default | ||
| // rootClientId, but the default rootClientId is actually `''`. | ||
| if ( clientId === null ) { | ||
| clientId = ''; | ||
| } | ||
|
|
||
| const isNavMode = isNavigationMode( state ); | ||
|
|
||
| // 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 ) | ||
| ) { | ||
| 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 ) { | ||
| return blockEditingMode; | ||
| } | ||
| export const getBlockEditingMode = ( state, clientId = '' ) => { | ||
| // Some selectors that call this provide `null` as the default | ||
| // rootClientId, but the default rootClientId is actually `''`. | ||
| if ( clientId === null ) { | ||
| clientId = ''; | ||
| } | ||
|
|
||
| // In normal mode, top level is default mode. | ||
| if ( clientId === '' ) { | ||
| return 'default'; | ||
| } | ||
| const isNavMode = isNavigationMode( state ); | ||
|
|
||
| const rootClientId = getBlockRootClientId( state, clientId ); | ||
| const templateLock = getTemplateLock( state, rootClientId ); | ||
| // If the parent of the block is contentOnly locked, check whether it's a content block. | ||
| if ( templateLock === 'contentOnly' ) { | ||
| const name = getBlockName( state, clientId ); | ||
| const { hasContentRoleAttribute } = unlock( | ||
| select( blocksStore ) | ||
| ); | ||
| const isContent = hasContentRoleAttribute( name ); | ||
| return isContent ? 'contentOnly' : 'disabled'; | ||
| } | ||
| return 'default'; | ||
| } | ||
| ); | ||
| if ( isNavMode ) { | ||
| // If the editor *is* in navigation mode, the block editing mode states | ||
| // are stored in the derivedNavModeBlockEditingModes map. | ||
| return state.derivedNavModeBlockEditingModes.get( clientId ); | ||
| } | ||
| return state.derivedBlockEditingModes.get( clientId ); | ||
| }; | ||
|
Comment on lines
+3052
to
+3058
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was curious if everything was stored in derivedBlockEditingModes. It appears not 😅 . It looks like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think
For the template locking code I have another PR that I was working on - #67606. IIRC, block editing modes don't handle every type of template lock, only contentOnly. I think there are a few things block editing modes can't do, so other types of template lock use completely different APIs. An example of something block editing modes can't do is enable a block, but prevent inner blocks from being added/moved/removed from that enabled block.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll close this PR in favor of #67606. That seems like it's further ahead on the refactor. |
||
|
|
||
| /** | ||
| * Indicates if a block is ungroupable. | ||
|
|
||
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.
contentOnlyis also used outside of navigation (write) mode:contentOnlyThose 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:

After:

Uh oh!
There was an error while loading. Please reload this page.
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.
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.
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.
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).