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.

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

Determine initial inspector tab from block editor state #46907

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions docs/reference-guides/data/data-core-block-editor.md
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,10 @@ _Returns_

- `Array`: ids of top-level and descendant blocks.

### getDefaultInspectorControlsTab
Copy link
Contributor

Choose a reason for hiding this comment

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

As it's likely that these will be superceeded by a dedicated package or mechanic for interacting with the UI (as suggested by @talldan) should we mark as unstable?


Undocumented declaration.

### getDraggedBlockClientIds

Returns the client ids of any blocks being directly dragged.
Expand Down Expand Up @@ -1485,6 +1489,10 @@ _Parameters_

- _updates_ `Record<string,boolean>`: For each block's clientId, its new visibility setting.

### setDefaultInspectorControlsTab

Undocumented declaration.

### setHasControlledInnerBlocks

Action that sets whether a block has controlled inner blocks.
Expand Down
8 changes: 6 additions & 2 deletions packages/block-editor/src/components/block-card/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,17 @@ function BlockCard( { title, icon, description, blockType, className } ) {
};
}, [] );

const { selectBlock } = useDispatch( blockEditorStore );
const { selectBlock, setDefaultInspectorControlsTab } =
useDispatch( blockEditorStore );

return (
<div className={ classnames( 'block-editor-block-card', className ) }>
{ isOffCanvasNavigationEditorEnabled && parentNavBlockClientId && (
<Button
onClick={ () => selectBlock( parentNavBlockClientId ) }
onClick={ () => {
setDefaultInspectorControlsTab( 'list' );
selectBlock( parentNavBlockClientId );
} }
label={ __( 'Go to parent Navigation block' ) }
style={
// TODO: This style override is also used in ToolsPanelHeader.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* WordPress dependencies
*/
import { useSelect } from '@wordpress/data';
import { TabPanel } from '@wordpress/components';

/**
Expand All @@ -10,22 +11,18 @@ import { TAB_SETTINGS, TAB_STYLES, TAB_LIST_VIEW } from './utils';
import SettingsTab from './settings-tab';
import StylesTab from './styles-tab';
import InspectorControls from '../inspector-controls';
import useIsListViewTabDisabled from './use-is-list-view-tab-disabled';
import { store as blockEditorStore } from '../../store';

export default function InspectorControlsTabs( {
blockName,
clientId,
hasBlockStyles,
tabs,
} ) {
// The tabs panel will mount before fills are rendered to the list view
// slot. This means the list view tab isn't initially included in the
// available tabs so the panel defaults selection to the styles tab
// which at the time is the first tab. This check allows blocks known to
// include the list view tab to set it as the tab selected by default.
const initialTabName = ! useIsListViewTabDisabled( blockName )
? TAB_LIST_VIEW.name
: undefined;
const initialTabName = useSelect( ( select ) => {
const { getDefaultInspectorControlsTab } = select( blockEditorStore );
return getDefaultInspectorControlsTab() ?? TAB_STYLES.name;
}, [] );

return (
<TabPanel
Expand Down Expand Up @@ -53,7 +50,9 @@ export default function InspectorControlsTabs( {

if ( tab.name === TAB_LIST_VIEW.name ) {
return (
<InspectorControls.Slot __experimentalGroup="list" />
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we add the <div>?

<InspectorControls.Slot __experimentalGroup="list" />
</div>
);
}
} }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,13 @@ export default function useInspectorControlsTabs( blockName ) {
color: colorGroup,
default: defaultGroup,
dimensions: dimensionsGroup,
list: listGroup,
typography: typographyGroup,
} = InspectorControlsGroups;

// List View Tab: If there are any fills for the list group add that tab.
const listViewDisabled = useIsListViewTabDisabled( blockName );
const listFills = useSlotFills( listGroup.Slot.__unstableName );

if ( ! listViewDisabled && !! listFills && listFills.length ) {
Comment on lines -45 to -53
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't follow these changes. Why did they need to happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check && !! listFills && listFills.length made the list view tab unavailable untill the fills loaded. I have no idea what this achieved, but removing this check allows the state to be 'list` and to set the default.

With the check in place the tabs reverted to 'styles' despite the state for initial tab was 'list' and that happened because of waiting for fills.

The other line is just removed because only the check used that const.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know @aaronrobertshaw worked on some of this so tagging him for additional context.

Copy link
Contributor

Choose a reason for hiding this comment

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

From memory, the check was in place due to an earlier requirement that tabs were only to be rendered if they had fills. Given the list view tab is a special case using an allow list, it looks superfluous to me as well.

if ( ! listViewDisabled ) {
tabs.push( TAB_LIST_VIEW );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ export default forwardRef( function BlockEditButton(
{ clientId, ...props },
ref
) {
const { selectBlock } = useDispatch( blockEditorStore );
const { selectBlock, setDefaultInspectorControlsTab } =
useDispatch( blockEditorStore );

const onClick = () => {
selectBlock( clientId );
setDefaultInspectorControlsTab( 'settings' );
};

return (
Expand Down
6 changes: 6 additions & 0 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -1501,6 +1501,12 @@ export const __unstableMarkAutomaticChange =
} );
};

export const setDefaultInspectorControlsTab =
( tab ) =>
( { dispatch } ) => {
dispatch( { type: 'SET_DEFAULT_INSPECTOR_CONTROLS_TAB', tab } );
};

/**
* Action that enables or disables the navigation mode.
*
Expand Down
10 changes: 10 additions & 0 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1673,6 +1673,15 @@ export const blockListSettings = ( state = {}, action ) => {
return state;
};

export function defaultInspectorControlsTab( state = null, action ) {
switch ( action.type ) {
case 'SET_DEFAULT_INSPECTOR_CONTROLS_TAB':
return action.tab;
}

return state;
}

/**
* Reducer returning which mode is enabled.
*
Expand Down Expand Up @@ -1879,6 +1888,7 @@ export default combineReducers( {
settings,
preferences,
lastBlockAttributesChange,
defaultInspectorControlsTab,
editorMode,
hasBlockMovingClientId,
automaticChangeStatus,
Expand Down
4 changes: 4 additions & 0 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -2534,6 +2534,10 @@ export function isNavigationMode( state ) {
return state.editorMode === 'navigation';
}

export function getDefaultInspectorControlsTab( state ) {
return state.defaultInspectorControlsTab;
}

/**
* Returns the current editor mode.
*
Expand Down