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,
 	};
 }