From d3b3749d22e81686dbb05aa5fdb61fe1e55e8211 Mon Sep 17 00:00:00 2001
From: Jorge Costa <jorge.costa@developer.pt>
Date: Tue, 9 Jul 2024 10:01:56 +0200
Subject: [PATCH] Split layout / view options. Use active layout icon for the
 layout button (#63205)

Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
---
 packages/dataviews/src/view-actions.tsx       | 153 +++++++++---------
 .../dataviews-list-layout-keyboard.spec.js    |   9 ++
 .../site-editor/new-templates-list.spec.js    |   3 +-
 test/performance/specs/site-editor.spec.js    |   8 +-
 4 files changed, 93 insertions(+), 80 deletions(-)

diff --git a/packages/dataviews/src/view-actions.tsx b/packages/dataviews/src/view-actions.tsx
index 20e55db03459a..eac09443197cc 100644
--- 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
index 3d98358a7aeb7..f6b01db63d911 100644
--- 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
index ab37244df107c..7273e3169db34 100644
--- 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
index 58f2cf5cd4b4e..520ad76de22ee 100644
--- 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' )