Skip to content
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

Nav offcanvas editor: add simple back button to inspector controls #45852

Merged
merged 3 commits into from
Nov 18, 2022
Merged

Nav offcanvas editor: add simple back button to inspector controls #45852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3cd445f
Add very simple back button to block inspector
getdave Nov 17, 2022
d612428
Enable for experiment only
getdave Nov 17, 2022
dc213ed
Change button description label
getdave Nov 17, 2022
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
30 changes: 28 additions & 2 deletions packages/block-editor/src/components/block-card/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,48 @@
* WordPress dependencies
*/
import deprecated from '@wordpress/deprecated';

import { Button } from '@wordpress/components';
import { chevronLeft, chevronRight } from '@wordpress/icons';
import { __, isRTL } from '@wordpress/i18n';
/**
* Internal dependencies
*/
import BlockIcon from '../block-icon';

function BlockCard( { title, icon, description, blockType } ) {
function BlockCard( {
title,
icon,
description,
blockType,
parentBlockClientId,
handleBackButton,
} ) {
if ( blockType ) {
deprecated( '`blockType` property in `BlockCard component`', {
since: '5.7',
alternative: '`title, icon and description` properties',
} );
( { title, icon, description } = blockType );
}

const isOffCanvasNavigationEditorEnabled =
window?.__experimentalEnableOffCanvasNavigationEditor === true;

return (
<div className="block-editor-block-card">
{ isOffCanvasNavigationEditorEnabled && parentBlockClientId && (
Copy link
Contributor

@draganescu draganescu Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This should also work only for the navigation block

Copy link
Contributor

Choose a reason for hiding this comment

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

Should it? Why not do it for all child blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to query how folks think we can do that without tightly coupling the block editor to a specific block.

Also I think it's probably great for all blocks 😄

<Button
onClick={ handleBackButton }
label={ __( 'Navigate to parent block' ) }
style={
// TODO: This style override is also used in ToolsPanelHeader.
// It should be supported out-of-the-box by Button.
{ minWidth: 24, padding: 0 }
}
icon={ isRTL() ? chevronRight : chevronLeft }
isSmall
/>
) }
<BlockIcon icon={ icon } showColors />
<div className="block-editor-block-card__content">
<h2 className="block-editor-block-card__title">{ title }</h2>
Expand Down
23 changes: 21 additions & 2 deletions packages/block-editor/src/components/block-inspector/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,15 @@ const BlockInspector = ( { showNoBlockSelectedMessage = true } ) => {
selectedBlockClientId,
blockType,
topLevelLockedBlock,
parentBlockClientId,
} = useSelect( ( select ) => {
const {
getSelectedBlockClientId,
getSelectedBlockCount,
getBlockName,
__unstableGetContentLockingParent,
getTemplateLock,
getBlockParents,
} = select( blockEditorStore );

const _selectedBlockClientId = getSelectedBlockClientId();
Expand All @@ -161,6 +163,10 @@ const BlockInspector = ( { showNoBlockSelectedMessage = true } ) => {
( getTemplateLock( _selectedBlockClientId ) === 'contentOnly'
? _selectedBlockClientId
: undefined ),
parentBlockClientId: getBlockParents(
_selectedBlockClientId,
true
)[ 0 ],
};
}, [] );

Expand Down Expand Up @@ -230,11 +236,16 @@ const BlockInspector = ( { showNoBlockSelectedMessage = true } ) => {
<BlockInspectorSingleBlock
clientId={ selectedBlockClientId }
blockName={ blockType.name }
parentBlockClientId={ parentBlockClientId }
/>
);
};

const BlockInspectorSingleBlock = ( { clientId, blockName } ) => {
const BlockInspectorSingleBlock = ( {
clientId,
blockName,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not introduced in this PR but same question for the block name

parentBlockClientId,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we retrieve that clientId in this component instead of passing it down?

} ) => {
const showTabs = window?.__experimentalEnableBlockInspectorTabs;

const hasBlockStyles = useSelect(
Expand All @@ -247,9 +258,17 @@ const BlockInspectorSingleBlock = ( { clientId, blockName } ) => {
);
const blockInformation = useBlockDisplayInformation( clientId );

const { selectBlock } = useDispatch( blockEditorStore );

return (
<div className="block-editor-block-inspector">
<BlockCard { ...blockInformation } />
<BlockCard
Copy link
Contributor

Choose a reason for hiding this comment

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

It may simplify things if this component just received a "clientId" and handles everything using useSelect/useDispatch internally. There doesn't seem to be a valid reason for all the prop passing down (unless I'm missing something). For the record, I know this is not entirely introduced in this PR :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I raised #45909 to track this.

{ ...blockInformation }
parentBlockClientId={ parentBlockClientId }
handleBackButton={ () => {
selectBlock( parentBlockClientId );
} }
/>
<BlockVariationTransforms blockClientId={ clientId } />
{ showTabs && (
<InspectorControlsTabs
Expand Down