Global styles menu: Avoid visible labels and accessible names mismatch (

WordPress#65124) * Simplify global styles root menu labels. * Fix and simplify labels in various global styles submenus. * Adjust failing test. * Fix more tests. * Fix one more test. * Entirely remove palette icons and move color randomizer. * Restore randomize colors position and add visible label. Co-authored-by: afercia <[email protected]> Co-authored-by: richtabor <[email protected]> Co-authored-by: ntsekouras <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: Mamaduka <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: ciampo <[email protected]>
MaxMood96 · Oct 18, 2024 · d330f30 · d330f30
1 parent 8b4fd4c
commit d330f30
Show file tree

Hide file tree

Showing 19 changed files with 81 additions and 148 deletions.
diff --git a/packages/block-editor/src/components/global-styles/color-panel.js b/packages/block-editor/src/components/global-styles/color-panel.js
@@ -20,7 +20,7 @@ import {
 	privateApis as componentsPrivateApis,
 } from '@wordpress/components';
 import { useCallback } from '@wordpress/element';
-import { __, sprintf } from '@wordpress/i18n';
+import { __ } from '@wordpress/i18n';
 
 /**
  * Internal dependencies
@@ -167,10 +167,7 @@ const LabeledColorIndicators = ( { indicators, label } ) => (
 				</Flex>
 			) ) }
 		</ZStack>
-		<FlexItem
-			className="block-editor-panel-color-gradient-settings__color-name"
-			title={ label }
-		>
+		<FlexItem className="block-editor-panel-color-gradient-settings__color-name">
 			{ label }
 		</FlexItem>
 	</HStack>
@@ -231,11 +228,6 @@ function ColorPanelDropdown( {
 							{ 'is-open': isOpen }
 						),
 						'aria-expanded': isOpen,
-						'aria-label': sprintf(
-							/* translators: %s is the type of color property, e.g., "background" */
-							__( 'Color %s styles' ),
-							label
-						),
 					};
 
 					return (

diff --git a/packages/edit-site/src/components/global-styles/font-sizes/font-sizes-count.js b/packages/edit-site/src/components/global-styles/font-sizes/font-sizes-count.js
@@ -23,10 +23,7 @@ function FontSizes() {
 				<Subtitle level={ 3 }>{ __( 'Font Sizes' ) }</Subtitle>
 			</HStack>
 			<ItemGroup isBordered isSeparated>
-				<NavigationButtonAsItem
-					path="/typography/font-sizes"
-					aria-label={ __( 'Edit font size presets' ) }
-				>
+				<NavigationButtonAsItem path="/typography/font-sizes">
 					<HStack direction="row">
 						<FlexItem>{ __( 'Font size presets' ) }</FlexItem>
 						<Icon icon={ isRTL() ? chevronLeft : chevronRight } />

diff --git a/packages/edit-site/src/components/global-styles/palette.js b/packages/edit-site/src/components/global-styles/palette.js
@@ -54,32 +54,35 @@ function Palette( { name } ) {
 	const screenPath = ! name
 		? '/colors/palette'
 		: '/blocks/' + encodeURIComponent( name ) + '/colors/palette';
-	const paletteButtonText =
-		colors.length > 0 ? __( 'Edit palette' ) : __( 'Add colors' );
 
 	return (
 		<VStack spacing={ 3 }>
 			<Subtitle level={ 3 }>{ __( 'Palette' ) }</Subtitle>
 			<ItemGroup isBordered isSeparated>
-				<NavigationButtonAsItem
-					path={ screenPath }
-					aria-label={ paletteButtonText }
-				>
+				<NavigationButtonAsItem path={ screenPath }>
 					<HStack direction="row">
-						{ colors.length <= 0 && (
+						{ colors.length > 0 ? (
+							<>
+								<ZStack isLayered={ false } offset={ -8 }>
+									{ colors
+										.slice( 0, 5 )
+										.map( ( { color }, index ) => (
+											<ColorIndicatorWrapper
+												key={ `${ color }-${ index }` }
+											>
+												<ColorIndicator
+													colorValue={ color }
+												/>
+											</ColorIndicatorWrapper>
+										) ) }
+								</ZStack>
+								<FlexItem isBlock>
+									{ __( 'Edit palette' ) }
+								</FlexItem>
+							</>
+						) : (
 							<FlexItem>{ __( 'Add colors' ) }</FlexItem>
 						) }
-						<ZStack isLayered={ false } offset={ -8 }>
-							{ colors
-								.slice( 0, 5 )
-								.map( ( { color }, index ) => (
-									<ColorIndicatorWrapper
-										key={ `${ color }-${ index }` }
-									>
-										<ColorIndicator colorValue={ color } />
-									</ColorIndicatorWrapper>
-								) ) }
-						</ZStack>
 						<Icon icon={ isRTL() ? chevronLeft : chevronRight } />
 					</HStack>
 				</NavigationButtonAsItem>

diff --git a/packages/edit-site/src/components/global-styles/root-menu.js b/packages/edit-site/src/components/global-styles/root-menu.js
@@ -48,17 +48,12 @@ function RootMenu() {
 					<NavigationButtonAsItem
 						icon={ typography }
 						path="/typography"
-						aria-label={ __( 'Typography styles' ) }
 					>
 						{ __( 'Typography' ) }
 					</NavigationButtonAsItem>
 				) }
 				{ hasColorPanel && (
-					<NavigationButtonAsItem
-						icon={ color }
-						path="/colors"
-						aria-label={ __( 'Colors styles' ) }
-					>
+					<NavigationButtonAsItem icon={ color } path="/colors">
 						{ __( 'Colors' ) }
 					</NavigationButtonAsItem>
 				) }
@@ -72,20 +67,12 @@ function RootMenu() {
 					</NavigationButtonAsItem>
 				) }
 				{ hasShadowPanel && (
-					<NavigationButtonAsItem
-						icon={ shadowIcon }
-						path="/shadows"
-						aria-label={ __( 'Shadow styles' ) }
-					>
+					<NavigationButtonAsItem icon={ shadowIcon } path="/shadows">
 						{ __( 'Shadows' ) }
 					</NavigationButtonAsItem>
 				) }
 				{ hasLayoutPanel && (
-					<NavigationButtonAsItem
-						icon={ layout }
-						path="/layout"
-						aria-label={ __( 'Layout styles' ) }
-					>
+					<NavigationButtonAsItem icon={ layout } path="/layout">
 						{ __( 'Layout' ) }
 					</NavigationButtonAsItem>
 				) }

diff --git a/packages/edit-site/src/components/global-styles/screen-block-list.js b/packages/edit-site/src/components/global-styles/screen-block-list.js
@@ -86,16 +86,9 @@ function BlockMenuItem( { block } ) {
 		return null;
 	}
 
-	const navigationButtonLabel = sprintf(
-		// translators: %s: is the name of a block e.g., 'Image' or 'Table'.
-		__( '%s block styles' ),
-		block.title
-	);
-
 	return (
 		<NavigationButtonAsItem
 			path={ '/blocks/' + encodeURIComponent( block.name ) }
-			aria-label={ navigationButtonLabel }
 		>
 			<HStack justify="flex-start">
 				<BlockIcon icon={ block.icon } />

diff --git a/packages/edit-site/src/components/global-styles/screen-root.js b/packages/edit-site/src/components/global-styles/screen-root.js
@@ -67,10 +67,7 @@ function ScreenRoot() {
 					</Card>
 					{ hasVariations && (
 						<ItemGroup>
-							<NavigationButtonAsItem
-								path="/variations"
-								aria-label={ __( 'Browse styles' ) }
-							>
+							<NavigationButtonAsItem path="/variations">
 								<HStack justify="space-between">
 									<FlexItem>
 										{ __( 'Browse styles' ) }
@@ -107,10 +104,7 @@ function ScreenRoot() {
 					) }
 				</Spacer>
 				<ItemGroup>
-					<NavigationButtonAsItem
-						path="/blocks"
-						aria-label={ __( 'Blocks styles' ) }
-					>
+					<NavigationButtonAsItem path="/blocks">
 						<HStack justify="space-between">
 							<FlexItem>{ __( 'Blocks' ) }</FlexItem>
 							<IconWithCurrentColor
@@ -136,10 +130,7 @@ function ScreenRoot() {
 							) }
 						</Spacer>
 						<ItemGroup>
-							<NavigationButtonAsItem
-								path="/css"
-								aria-label={ __( 'Additional CSS' ) }
-							>
+							<NavigationButtonAsItem path="/css">
 								<HStack justify="space-between">
 									<FlexItem>
 										{ __( 'Additional CSS' ) }

diff --git a/packages/edit-site/src/components/global-styles/shadows-panel.js b/packages/edit-site/src/components/global-styles/shadows-panel.js
@@ -135,10 +135,6 @@ function ShadowItem( { shadow, category } ) {
 	return (
 		<NavigationButtonAsItem
 			path={ `/shadows/edit/${ category }/${ shadow.slug }` }
-			aria-label={
-				// translators: %s: name of the shadow
-				sprintf( 'Edit shadow %s', shadow.name )
-			}
 			icon={ shadowIcon }
 		>
 			{ shadow.name }

diff --git a/packages/edit-site/src/components/global-styles/typography-elements.js b/packages/edit-site/src/components/global-styles/typography-elements.js
@@ -1,7 +1,7 @@
 /**
  * WordPress dependencies
  */
-import { __, sprintf } from '@wordpress/i18n';
+import { __ } from '@wordpress/i18n';
 import {
 	__experimentalItemGroup as ItemGroup,
 	__experimentalVStack as VStack,
@@ -36,17 +36,8 @@ function ElementItem( { parentMenu, element, label } ) {
 	const [ gradientValue ] = useGlobalStyle( prefix + 'color.gradient' );
 	const [ color ] = useGlobalStyle( prefix + 'color.text' );
 
-	const navigationButtonLabel = sprintf(
-		// translators: %s: is a subset of Typography, e.g., 'text' or 'links'.
-		__( 'Typography %s styles' ),
-		label
-	);
-
 	return (
-		<NavigationButtonAsItem
-			path={ parentMenu + '/typography/' + element }
-			aria-label={ navigationButtonLabel }
-		>
+		<NavigationButtonAsItem path={ parentMenu + '/typography/' + element }>
 			<HStack justify="flex-start">
 				<FlexItem
 					className="edit-site-global-styles-screen-typography__indicator"
@@ -61,6 +52,7 @@ function ElementItem( { parentMenu, element, label } ) {
 						fontWeight,
 						...extraStyles,
 					} }
+					aria-hidden="true"
 				>
 					{ __( 'Aa' ) }
 				</FlexItem>

diff --git a/packages/edit-site/src/components/global-styles/variations/variations-panel.js b/packages/edit-site/src/components/global-styles/variations/variations-panel.js
@@ -55,7 +55,6 @@ export function VariationsPanel( { name } ) {
 							'/variations/' +
 							encodeURIComponent( style.name )
 						}
-						aria-label={ style.label }
 					>
 						{ style.label }
 					</NavigationButtonAsItem>

diff --git a/test/e2e/specs/editor/blocks/buttons.spec.js b/test/e2e/specs/editor/blocks/buttons.spec.js
@@ -291,11 +291,11 @@ test.describe( 'Buttons', () => {
 			`role=region[name="Editor settings"i] >> role=tab[name="Styles"i]`
 		);
 		await page.click(
-			'role=region[name="Editor settings"i] >> role=button[name="Color Text styles"i]'
+			'role=region[name="Editor settings"i] >> role=button[name="Text"i]'
 		);
 		await page.click( 'role=option[name="Color: Cyan bluish gray"i]' );
 		await page.click(
-			'role=region[name="Editor settings"i] >> role=button[name="Color Background styles"i]'
+			'role=region[name="Editor settings"i] >> role=button[name="Background"i]'
 		);
 		await page.click( 'role=option[name="Color: Vivid red"i]' );
 
@@ -320,13 +320,13 @@ test.describe( 'Buttons', () => {
 			`role=region[name="Editor settings"i] >> role=tab[name="Styles"i]`
 		);
 		await page.click(
-			'role=region[name="Editor settings"i] >> role=button[name="Color Text styles"i]'
+			'role=region[name="Editor settings"i] >> role=button[name="Text"i]'
 		);
 		await page.click( 'role=button[name="Custom color picker."i]' );
 		await page.fill( 'role=textbox[name="Hex color"i]', 'ff0000' );
 
 		await page.click(
-			'role=region[name="Editor settings"i] >> role=button[name="Color Background styles"i]'
+			'role=region[name="Editor settings"i] >> role=button[name="Background"i]'
 		);
 		await page.click( 'role=button[name="Custom color picker."i]' );
 		await page.fill( 'role=textbox[name="Hex color"i]', '00ff00' );
@@ -355,7 +355,7 @@ test.describe( 'Buttons', () => {
 			`role=region[name="Editor settings"i] >> role=tab[name="Styles"i]`
 		);
 		await page.click(
-			'role=region[name="Editor settings"i] >> role=button[name="Color Background styles"i]'
+			'role=region[name="Editor settings"i] >> role=button[name="Background"i]'
 		);
 		await page.click( 'role=tab[name="Gradient"i]' );
 		await page.click( 'role=option[name="Gradient: Purple to yellow"i]' );
@@ -384,7 +384,7 @@ test.describe( 'Buttons', () => {
 			`role=region[name="Editor settings"i] >> role=tab[name="Styles"i]`
 		);
 		await page.click(
-			'role=region[name="Editor settings"i] >> role=button[name="Color Background styles"i]'
+			'role=region[name="Editor settings"i] >> role=button[name="Background"i]'
 		);
 		await page.click( 'role=tab[name="Gradient"i]' );
 		await page.click(

diff --git a/test/e2e/specs/editor/blocks/navigation-colors.spec.js b/test/e2e/specs/editor/blocks/navigation-colors.spec.js
@@ -91,13 +91,13 @@ test.describe( 'Navigation colors', () => {
 		// In the sidebar inspector we add a link color and link hover color to the group block.
 		await editor.openDocumentSettingsSidebar();
 		await page.getByRole( 'tab', { name: 'Styles' } ).click();
-		await page.getByRole( 'button', { name: 'Color Text styles' } ).click();
+		await page.getByRole( 'button', { name: 'Text' } ).click();
 		await page
 			.getByRole( 'option', { name: 'Color: White' } )
 			.click( { force: true } );
 
 		await page
-			.getByRole( 'button', { name: 'Color Background styles' } )
+			.getByRole( 'button', { name: 'Background', exact: true } )
 			.click();
 		await page
 			.getByRole( 'option', { name: 'Color: Black' } )
@@ -148,7 +148,7 @@ test.describe( 'Navigation colors', () => {
 			.getByRole( 'menuitemcheckbox', { name: 'Show Link' } )
 			.click();
 		await page.getByRole( 'tab', { name: 'Styles' } ).click();
-		await page.getByRole( 'button', { name: 'Color Link styles' } ).click();
+		await page.getByRole( 'button', { name: 'Link', exact: true } ).click();
 		// rga(207, 46 ,46) is the color of the "vivid red" color preset.
 		await page
 			.getByRole( 'option', { name: 'Color: Vivid red' } )

diff --git a/test/e2e/specs/editor/various/keep-styles-on-block-transforms.spec.js b/test/e2e/specs/editor/various/keep-styles-on-block-transforms.spec.js
@@ -17,7 +17,7 @@ test.describe( 'Keep styles on block transforms', () => {
 			.locator( 'role=button[name="Add default block"i]' )
 			.click();
 		await page.keyboard.type( '## Heading' );
-		await page.click( 'role=button[name="Color Text styles"i]' );
+		await page.click( 'role=button[name="Text"i]' );
 		await page.click( 'role=option[name="Color: Luminous vivid orange"i]' );
 
 		await page.click( 'role=button[name="Heading"i]' );