Split layout / view options. Use active layout icon for the layout bu…

…tton (WordPress#63205) Co-authored-by: jorgefilipecosta <[email protected]> Co-authored-by: jameskoster <[email protected]>
huubl · Jul 10, 2024 · 9c32524 · 9c32524
1 parent adc1992
commit 9c32524
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 80 deletions.
diff --git a/packages/dataviews/src/view-actions.tsx b/packages/dataviews/src/view-actions.tsx
@@ -9,6 +9,7 @@ import type { ChangeEvent } from 'react';
 import {
 	Button,
 	privateApis as componentsPrivateApis,
+	__experimentalHStack as HStack,
 } from '@wordpress/components';
 import { __, _x } from '@wordpress/i18n';
 import { memo } from '@wordpress/element';
@@ -75,51 +76,34 @@ function ViewTypeMenu( {
 	if ( _availableViews.length === 1 ) {
 		return null;
 	}
-	const activeView = _availableViews.find( ( v ) => view.type === v.type );
-	return (
-		<DropdownMenu
-			trigger={
-				<DropdownMenuItem
-					suffix={
-						<span aria-hidden="true">{ activeView?.label }</span>
+	return _availableViews.map( ( availableView ) => {
+		return (
+			<DropdownMenuRadioItem
+				key={ availableView.type }
+				value={ availableView.type }
+				name="view-actions-available-view"
+				checked={ availableView.type === view.type }
+				hideOnClick
+				onChange={ ( e: ChangeEvent< HTMLInputElement > ) => {
+					switch ( e.target.value ) {
+						case 'list':
+						case 'grid':
+						case 'table':
+							return onChangeView( {
+								...view,
+								type: e.target.value,
+								layout: {},
+							} );
 					}
-				>
-					<DropdownMenuItemLabel>
-						{ __( 'Layout' ) }
-					</DropdownMenuItemLabel>
-				</DropdownMenuItem>
-			}
-		>
-			{ _availableViews.map( ( availableView ) => {
-				return (
-					<DropdownMenuRadioItem
-						key={ availableView.type }
-						value={ availableView.type }
-						name="view-actions-available-view"
-						checked={ availableView.type === view.type }
-						hideOnClick
-						onChange={ ( e: ChangeEvent< HTMLInputElement > ) => {
-							switch ( e.target.value ) {
-								case 'list':
-								case 'grid':
-								case 'table':
-									return onChangeView( {
-										...view,
-										type: e.target.value,
-										layout: {},
-									} );
-							}
-							throw new Error( 'Invalid dataview' );
-						} }
-					>
-						<DropdownMenuItemLabel>
-							{ availableView.label }
-						</DropdownMenuItemLabel>
-					</DropdownMenuRadioItem>
-				);
-			} ) }
-		</DropdownMenu>
-	);
+					throw new Error( 'Invalid dataview' );
+				} }
+			>
+				<DropdownMenuItemLabel>
+					{ availableView.label }
+				</DropdownMenuItemLabel>
+			</DropdownMenuRadioItem>
+		);
+	} );
 }
 
 const PAGE_SIZE_VALUES = [ 10, 20, 50, 100 ];
@@ -305,35 +289,60 @@ function _ViewActions< Item >( {
 	onChangeView,
 	supportedLayouts,
 }: ViewActionsProps< Item > ) {
+	const activeView = VIEW_LAYOUTS.find( ( v ) => view.type === v.type );
 	return (
-		<DropdownMenu
-			trigger={
-				<Button
-					size="compact"
-					icon={ cog }
-					label={ _x( 'View options', 'View is used as a noun' ) }
-				/>
-			}
-		>
-			<DropdownMenuGroup>
-				<ViewTypeMenu
-					view={ view }
-					onChangeView={ onChangeView }
-					supportedLayouts={ supportedLayouts }
-				/>
-				<SortMenu
-					fields={ fields }
-					view={ view }
-					onChangeView={ onChangeView }
-				/>
-				<FieldsVisibilityMenu
-					fields={ fields }
-					view={ view }
-					onChangeView={ onChangeView }
-				/>
-				<PageSizeMenu view={ view } onChangeView={ onChangeView } />
-			</DropdownMenuGroup>
-		</DropdownMenu>
+		<>
+			<HStack
+				spacing={ 1 }
+				expanded={ false }
+				style={ { flexShrink: 0 } }
+			>
+				<DropdownMenu
+					trigger={
+						<Button
+							size="compact"
+							icon={ activeView?.icon }
+							label={ __( 'Layout' ) }
+						/>
+					}
+				>
+					<ViewTypeMenu
+						view={ view }
+						onChangeView={ onChangeView }
+						supportedLayouts={ supportedLayouts }
+					/>
+				</DropdownMenu>
+				<DropdownMenu
+					trigger={
+						<Button
+							size="compact"
+							icon={ cog }
+							label={ _x(
+								'View options',
+								'View is used as a noun'
+							) }
+						/>
+					}
+				>
+					<DropdownMenuGroup>
+						<SortMenu
+							fields={ fields }
+							view={ view }
+							onChangeView={ onChangeView }
+						/>
+						<FieldsVisibilityMenu
+							fields={ fields }
+							view={ view }
+							onChangeView={ onChangeView }
+						/>
+						<PageSizeMenu
+							view={ view }
+							onChangeView={ onChangeView }
+						/>
+					</DropdownMenuGroup>
+				</DropdownMenu>
+			</HStack>
+		</>
 	);
 }
 

diff --git a/test/e2e/specs/site-editor/dataviews-list-layout-keyboard.spec.js b/test/e2e/specs/site-editor/dataviews-list-layout-keyboard.spec.js
@@ -46,6 +46,11 @@ test.describe( 'Dataviews List Layout', () => {
 			page.getByRole( 'button', { name: 'Reset' } )
 		).toBeFocused();
 
+		await page.keyboard.press( 'Tab' );
+		await expect(
+			page.getByRole( 'button', { name: 'Layout' } )
+		).toBeFocused();
+
 		await page.keyboard.press( 'Tab' );
 		await expect(
 			page.getByRole( 'button', { name: 'View options' } )
@@ -69,6 +74,7 @@ test.describe( 'Dataviews List Layout', () => {
 		await page.keyboard.press( 'Tab' );
 		await page.keyboard.press( 'Tab' );
 		await page.keyboard.press( 'Tab' );
+		await page.keyboard.press( 'Tab' );
 
 		// Make sure the items have loaded before reaching for the 1st item in the list.
 		await expect( page.getByRole( 'grid' ) ).toBeVisible();
@@ -98,6 +104,7 @@ test.describe( 'Dataviews List Layout', () => {
 		await page.keyboard.press( 'Tab' );
 		await page.keyboard.press( 'Tab' );
 		await page.keyboard.press( 'Tab' );
+		await page.keyboard.press( 'Tab' );
 
 		// Make sure the items have loaded before reaching for the 1st item in the list.
 		await expect( page.getByRole( 'grid' ) ).toBeVisible();
@@ -121,6 +128,7 @@ test.describe( 'Dataviews List Layout', () => {
 		await page.keyboard.press( 'Tab' );
 		await page.keyboard.press( 'Tab' );
 		await page.keyboard.press( 'Tab' );
+		await page.keyboard.press( 'Tab' );
 
 		// Make sure the items have loaded before reaching for the 1st item in the list.
 		await expect( page.getByRole( 'grid' ) ).toBeVisible();
@@ -162,6 +170,7 @@ test.describe( 'Dataviews List Layout', () => {
 		await page.keyboard.press( 'Tab' );
 		await page.keyboard.press( 'Tab' );
 		await page.keyboard.press( 'Tab' );
+		await page.keyboard.press( 'Tab' );
 
 		// Make sure the items have loaded before reaching for the 1st item in the list.
 		await expect( page.getByRole( 'grid' ) ).toBeVisible();

diff --git a/test/e2e/specs/site-editor/new-templates-list.spec.js b/test/e2e/specs/site-editor/new-templates-list.spec.js
@@ -86,8 +86,7 @@ test.describe( 'Templates', () => {
 	test( 'Field visibility', async ( { admin, page } ) => {
 		await admin.visitSiteEditor( { postType: 'wp_template' } );
 
-		await page.getByRole( 'button', { name: 'View options' } ).click();
-		await page.getByRole( 'menuitem', { name: 'Layout' } ).click();
+		await page.getByRole( 'button', { name: 'Layout' } ).click();
 		await page.getByRole( 'menuitemradio', { name: 'Table' } ).click();
 
 		await page.getByRole( 'button', { name: 'Description' } ).click();

diff --git a/test/performance/specs/site-editor.spec.js b/test/performance/specs/site-editor.spec.js
@@ -222,15 +222,11 @@ test.describe( 'Site Editor Performance', () => {
 				// If it's there, switch to the list layout before running the test.
 				// See https://github.com/WordPress/gutenberg/pull/59792
 				const isDataViewsUI = await page
-					.getByRole( 'button', { name: 'View options' } )
+					.getByRole( 'button', { name: 'Layout' } )
 					.isVisible();
 				if ( isDataViewsUI ) {
 					await page
-						.getByRole( 'button', { name: 'View options' } )
-						.click();
-					await page
-						.getByRole( 'menuitem' )
-						.filter( { has: page.getByText( 'Layout' ) } )
+						.getByRole( 'button', { name: 'Layout' } )
 						.click();
 					await page
 						.getByRole( 'menuitemradio' )