Block Supports: Switch dimensions inspector controls slot to bubble v…

…irtually (#34725)

packages/block-editor/src/components/block-inspector/index.js

@@ -131,7 +131,7 @@ const BlockInspectorSingleBlock = ( {
  <InspectorControls.Slot bubblesVirtually={ bubblesVirtually } />
  <InspectorControls.Slot
  __experimentalGroup="dimensions"
- bubblesVirtually={ false }
+ bubblesVirtually={ bubblesVirtually }
  label={ __( 'Dimensions' ) }
  />
  <div>

packages/block-editor/src/components/inspector-controls/block-support-slot-container.js

@@ -0,0 +1,10 @@
+/**
+ * WordPress dependencies
+ */
+import { __experimentalToolsPanelContext as ToolsPanelContext } from '@wordpress/components';
+import { useContext } from '@wordpress/element';
+
+export default function BlockSupportSlotContainer( { Slot, ...props } ) {
+ const toolsPanelContext = useContext( ToolsPanelContext );
+ return <Slot { ...props } fillProps={ toolsPanelContext } />;
+}

packages/block-editor/src/components/inspector-controls/block-support-tools-panel.js

@@ -10,7 +10,7 @@ import { useDispatch, useSelect } from '@wordpress/data';
 import { store as blockEditorStore } from '../../store';
 import { cleanEmptyObject } from '../../hooks/utils';
 
-export default function BlockSupportToolsPanel( { children, label, header } ) {
+export default function BlockSupportToolsPanel( { children, label } ) {
  const { clientId, attributes } = useSelect( ( select ) => {
  const { getBlockAttributes, getSelectedBlockClientId } = select(
  blockEditorStore
@@ -47,10 +47,11 @@ export default function BlockSupportToolsPanel( { children, label, header } ) {
  return (
  <ToolsPanel
  label={ label }
- header={ header }
  resetAll={ resetAll }
  key={ clientId }
  panelId={ clientId }
+ hasInnerWrapper={ true }
+ shouldRenderPlaceholderItems={ true } // Required to maintain fills ordering.
  >
  { children }
  </ToolsPanel>

packages/block-editor/src/components/inspector-controls/fill.js

@@ -1,7 +1,15 @@
+/**
+ * External dependencies
+ */
+import { isEmpty } from 'lodash';
+
 /**
  * WordPress dependencies
  */
-import { __experimentalStyleProvider as StyleProvider } from '@wordpress/components';
+import {
+ __experimentalStyleProvider as StyleProvider,
+ __experimentalToolsPanelContext as ToolsPanelContext,
+} from '@wordpress/components';
 import warning from '@wordpress/warning';
 
 /**
@@ -26,7 +34,20 @@ export default function InspectorControlsFill( {
 
  return (
  <StyleProvider document={ document }>
- <Fill>{ children }</Fill>
+ <Fill>
+ { ( fillProps ) => {
+ // Children passed to InspectorControlsFill will not have
+ // access to any React Context whose Provider is part of
+ // the InspectorControlsSlot tree. So we re-create the
+ // Provider in this subtree.
+ const value = ! isEmpty( fillProps ) ? fillProps : null;
+ return (
+ <ToolsPanelContext.Provider value={ value }>
+ { children }
+ </ToolsPanelContext.Provider>
+ );
+ } }
+ </Fill>
  </StyleProvider>
  );
 }

packages/block-editor/src/components/inspector-controls/slot.js

@@ -8,6 +8,7 @@ import warning from '@wordpress/warning';
  * Internal dependencies
  */
 import BlockSupportToolsPanel from './block-support-tools-panel';
+import BlockSupportSlotContainer from './block-support-slot-container';
 import groups from './groups';
 
 export default function InspectorControlsSlot( {
@@ -31,7 +32,11 @@ export default function InspectorControlsSlot( {
  if ( label ) {
  return (
  <BlockSupportToolsPanel group={ group } label={ label }>
- <Slot { ...props } bubblesVirtually={ bubblesVirtually } />
+ <BlockSupportSlotContainer
+ { ...props }
+ bubblesVirtually={ bubblesVirtually }
+ Slot={ Slot }
+ />
  </BlockSupportToolsPanel>
  );
  }

packages/components/src/tools-panel/context.ts

@@ -14,6 +14,7 @@ export const ToolsPanelContext = createContext< ToolsPanelContextType >( {
  menuItems: { default: {}, optional: {} },
  hasMenuItems: false,
  isResetting: false,
+ shouldRenderPlaceholderItems: false,
  registerPanelItem: noop,
  deregisterPanelItem: noop,
  flagItemCustomization: noop,

packages/components/src/tools-panel/styles.ts

@@ -9,22 +9,55 @@ import { css } from '@emotion/react';
 import { COLORS, CONFIG } from '../utils';
 import { space } from '../ui/utils/space';
 
+const toolsPanelGrid = {
+ container: css`
+ column-gap: ${ space( 4 ) };
+ display: grid;
+ grid-template-columns: 1fr 1fr;
+ row-gap: ${ space( 6 ) };
+ `,
+ item: {
+ halfWidth: css`
+ grid-column: span 1;
+ `,
+ fullWidth: css`
+ grid-column: span 2;
+ `,
+ },
+};
+
 export const ToolsPanel = css`
+ ${ toolsPanelGrid.container };
+
  border-top: ${ CONFIG.borderWidth } solid ${ COLORS.gray[ 200 ] };
- column-gap: ${ space( 4 ) };
- display: grid;
- grid-template-columns: 1fr 1fr;
  margin-top: -1px;
  padding: ${ space( 4 ) };
- row-gap: ${ space( 6 ) };
+`;
+
+/**
+ * Items injected into a ToolsPanel via a virtual bubbling slot will require
+ * an inner dom element to be injected. The following rule allows for the
+ * CSS grid display to be re-established.
+ */
+export const ToolsPanelWithInnerWrapper = css`
+ > div {
+ ${ toolsPanelGrid.container }
+ ${ toolsPanelGrid.item.fullWidth }
+ }
+`;
+
+export const ToolsPanelHiddenInnerWrapper = css`
+ > div {
+ display: none;
+ }
 `;
 
 export const ToolsPanelHeader = css`
  align-items: center;
  display: flex;
  font-size: inherit;
  font-weight: 500;
- grid-column: span 2;
+ ${ toolsPanelGrid.item.fullWidth }
  justify-content: space-between;
  line-height: normal;
 
@@ -47,10 +80,10 @@ export const ToolsPanelHeader = css`
 `;
 
 export const ToolsPanelItem = css`
- grid-column: span 2;
+ ${ toolsPanelGrid.item.fullWidth }
 
  &.single-column {
- grid-column: span 1;
+ ${ toolsPanelGrid.item.halfWidth }
  }
 
  /* Clear spacing in and around controls added as panel items. */
@@ -61,6 +94,18 @@ export const ToolsPanelItem = css`
  margin-bottom: 0;
  max-width: 100%;
  }
+
+ & > .components-base-control:last-child {
+ margin-bottom: 0;
+
+ .components-base-control__field {
+ margin-bottom: 0;
+ }
+ }
+`;
+
+export const ToolsPanelItemPlaceholder = css`
+ display: none;
 `;
 
 export const DropdownMenu = css`

packages/components/src/tools-panel/test/index.js

@@ -7,7 +7,9 @@ import { render, screen, fireEvent } from '@testing-library/react';
  * Internal dependencies
  */
 import { ToolsPanel, ToolsPanelItem } from '../';
+import { createSlotFill, Provider as SlotFillProvider } from '../../slot-fill';
 
+const { Fill: ToolsPanelItems, Slot } = createSlotFill( 'ToolsPanelSlot' );
 const resetAll = jest.fn();
 
 // Default props for the tools panel.
@@ -151,6 +153,10 @@ const selectMenuItem = async ( label ) => {
 };
 
 describe( 'ToolsPanel', () => {
+ afterEach( () => {
+ controlProps.attributes.value = true;
+ } );
+
  describe( 'basic rendering', () => {
  it( 'should render panel', () => {
  const { container } = renderPanel();
@@ -310,12 +316,35 @@ describe( 'ToolsPanel', () => {
  // Groups should be: default controls, optional controls & reset all.
  expect( menuGroups.length ).toEqual( 3 );
  } );
+
+ it( 'should render placeholder items when panel opts into that feature', () => {
+ const { container } = render(
+ <ToolsPanel
+ { ...defaultProps }
+ shouldRenderPlaceholderItems={ true }
+ >
+ <ToolsPanelItem { ...altControlProps }>
+ <div>Optional control</div>
+ </ToolsPanelItem>
+ </ToolsPanel>
+ );
+
+ const optionalItem = screen.queryByText( 'Optional control' );
+ const placeholder = container.querySelector(
+ '.components-tools-panel-item'
+ );
+
+ // When rendered as a placeholder a ToolsPanelItem will just omit
+ // all the item's children. So we should still find the container
+ // element but not the text etc within.
+ expect( optionalItem ).not.toBeInTheDocument();
+ expect( placeholder ).toBeInTheDocument();
+ } );
  } );
 
  describe( 'callbacks on menu item selection', () => {
  beforeEach( () => {
  jest.clearAllMocks();
- controlProps.attributes.value = true;
  } );
 
  it( 'should call onDeselect callback when menu item is toggled off', async () => {
@@ -425,4 +454,59 @@ describe( 'ToolsPanel', () => {
  expect( altMenuItem ).toHaveAttribute( 'aria-checked', 'false' );
  } );
  } );
+
+ describe( 'rendering via SlotFills', () => {
+ it( 'should maintain visual order of controls when toggled on and off', async () => {
+ // Multiple fills are added to better simulate panel items being
+ // injected from different locations.
+ render(
+ <SlotFillProvider>
+ <ToolsPanelItems>
+ <ToolsPanelItem { ...altControlProps }>
+ <div>Item 1</div>
+ </ToolsPanelItem>
+ </ToolsPanelItems>
+ <ToolsPanelItems>
+ <ToolsPanelItem { ...controlProps }>
+ <div>Item 2</div>
+ </ToolsPanelItem>
+ </ToolsPanelItems>
+ <ToolsPanel { ...defaultProps }>
+ <Slot />
+ </ToolsPanel>
+ </SlotFillProvider>
+ );
+
+ // Only the second item should be shown initially as it has a value.
+ const firstItem = screen.queryByText( 'Item 1' );
+ const secondItem = screen.getByText( 'Item 2' );
+
+ expect( firstItem ).not.toBeInTheDocument();
+ expect( secondItem ).toBeInTheDocument();
+
+ // Toggle on the first item.
+ await selectMenuItem( altControlProps.label );
+
+ // The order of items should be as per their original source order.
+ let items = screen.getAllByText( /Item [1-2]/ );
+
+ expect( items ).toHaveLength( 2 );
+ expect( items[ 0 ] ).toHaveTextContent( 'Item 1' );
+ expect( items[ 1 ] ).toHaveTextContent( 'Item 2' );
+
+ // Then toggle off both items.
+ await selectMenuItem( controlProps.label );
+ await selectMenuItem( altControlProps.label );
+
+ // Toggle on controls again and ensure order remains.
+ await selectMenuItem( controlProps.label );
+ await selectMenuItem( altControlProps.label );
+
+ items = screen.getAllByText( /Item [1-2]/ );
+
+ expect( items ).toHaveLength( 2 );
+ expect( items[ 0 ] ).toHaveTextContent( 'Item 1' );
+ expect( items[ 1 ] ).toHaveTextContent( 'Item 2' );
+ } );
+ } );
 } );

packages/components/src/tools-panel/tools-panel-item/component.tsx

@@ -18,12 +18,17 @@ const ToolsPanelItem = (
  props: WordPressComponentProps< ToolsPanelItemProps, 'div' >,
  forwardedRef: Ref< any >
 ) => {
- const { children, isShown, ...toolsPanelItemProps } = useToolsPanelItem(
- props
- );
+ const {
+ children,
+ isShown,
+ shouldRenderPlaceholder,
+ ...toolsPanelItemProps
+ } = useToolsPanelItem( props );
 
  if ( ! isShown ) {
- return null;
+ return shouldRenderPlaceholder ? (
+ <View { ...toolsPanelItemProps } ref={ forwardedRef } />
+ ) : null;
  }
 
  return (

packages/components/src/tools-panel/tools-panel-item/hook.ts

@@ -28,18 +28,14 @@ export function useToolsPanelItem(
  ...otherProps
  } = useContextSystem( props, 'ToolsPanelItem' );
 
- const cx = useCx();
- const classes = useMemo( () => {
- return cx( styles.ToolsPanelItem, className );
- }, [ className ] );
-
  const {
  panelId: currentPanelId,
  menuItems,
  registerPanelItem,
  deregisterPanelItem,
  flagItemCustomization,
  isResetting,
+ shouldRenderPlaceholderItems: shouldRenderPlaceholder,
  } = useToolsPanelContext();
 
  const hasValueCallback = useCallback( hasValue, [ panelId ] );
@@ -115,9 +111,19 @@ export function useToolsPanelItem(
  ? menuItems?.[ menuGroup ]?.[ label ] !== undefined
  : isMenuItemChecked;
 
+ const cx = useCx();
+ const classes = useMemo( () => {
+ const placeholderStyle =
+ shouldRenderPlaceholder &&
+ ! isShown &&
+ styles.ToolsPanelItemPlaceholder;
+ return cx( styles.ToolsPanelItem, placeholderStyle, className );
+ }, [ isShown, shouldRenderPlaceholder, className ] );
+
  return {
  ...otherProps,
  isShown,
+ shouldRenderPlaceholder,
  className: classes,
  };
 }