Skip to content

Commit

Permalink
Global styles menu: Avoid visible labels and accessible names mismatch (
Browse files Browse the repository at this point in the history
#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]>
  • Loading branch information
8 people authored Oct 18, 2024
1 parent 8b4fd4c commit d330f30
Show file tree
Hide file tree
Showing 19 changed files with 81 additions and 148 deletions.
12 changes: 2 additions & 10 deletions packages/block-editor/src/components/global-styles/color-panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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 (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 } />
Expand Down
39 changes: 21 additions & 18 deletions packages/edit-site/src/components/global-styles/palette.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
19 changes: 3 additions & 16 deletions packages/edit-site/src/components/global-styles/root-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>
) }
Expand All @@ -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>
) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 } />
Expand Down
15 changes: 3 additions & 12 deletions packages/edit-site/src/components/global-styles/screen-root.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' ) }
Expand Down Expand Up @@ -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
Expand All @@ -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' ) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { __ } from '@wordpress/i18n';
import {
__experimentalItemGroup as ItemGroup,
__experimentalVStack as VStack,
Expand Down Expand Up @@ -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"
Expand All @@ -61,6 +52,7 @@ function ElementItem( { parentMenu, element, label } ) {
fontWeight,
...extraStyles,
} }
aria-hidden="true"
>
{ __( 'Aa' ) }
</FlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export function VariationsPanel( { name } ) {
'/variations/' +
encodeURIComponent( style.name )
}
aria-label={ style.label }
>
{ style.label }
</NavigationButtonAsItem>
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/specs/editor/blocks/buttons.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]' );

Expand All @@ -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' );
Expand Down Expand Up @@ -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]' );
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/specs/editor/blocks/navigation-colors.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' } )
Expand Down Expand Up @@ -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' } )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]' );
Expand Down
Loading

0 comments on commit d330f30

Please sign in to comment.