Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PaletteEdit: Fix palette item accessibility #58214

Merged
merged 5 commits into from
Jan 25, 2024
Merged

PaletteEdit: Fix palette item accessibility #58214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d85f4be
Improve tests
mirka Jan 24, 2024
0807e15
Increase colors in story
mirka Jan 24, 2024
4d71c11
PaletteEdit: Fix palette item accessibility
mirka Jan 24, 2024
b3c0d36
Update changelog
mirka Jan 24, 2024
3e560ed
Add fallbacks for when unnamed
mirka Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

- `ToggleGroupControl`: Improve controlled value detection ([#57770](https://github.com/WordPress/gutenberg/pull/57770)).
- `Tooltip`: Improve props forwarding to children of nested `Tooltip` components ([#57878](https://github.com/WordPress/gutenberg/pull/57878)).
- `PaletteEdit`: Fix palette item accessibility in details view ([#58214](https://github.com/WordPress/gutenberg/pull/58214)).
- `Tooltip`: revert prop types to only accept component-specific props ([#58125](https://github.com/WordPress/gutenberg/pull/58125)).
- `Button`: prevent the component from trashing and re-creating the HTML element ([#56490](https://github.com/WordPress/gutenberg/pull/56490)).

Expand Down
32 changes: 18 additions & 14 deletions packages/components/src/palette-edit/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,23 +198,22 @@ function Option< T extends Color | Gradient >( {
return (
<PaletteItem
className={ isEditing ? 'is-selected' : undefined }
as="div"
as={ isEditing ? 'div' : 'button' }
onClick={ onStartEditing }
aria-label={
isEditing
? undefined
: sprintf(
// translators: %s is a color or gradient name, e.g. "Red".
__( 'Edit: %s' ),
element.name.trim().length ? element.name : value
)
}
ref={ setPopoverAnchor }
{ ...( isEditing
? { ...focusOutsideProps }
: {
style: {
cursor: 'pointer',
},
} ) }
{ ...( isEditing ? { ...focusOutsideProps } : {} ) }
>
<HStack justify="flex-start">
<FlexItem>
<IndicatorStyled
style={ { background: value, color: 'transparent' } }
/>
</FlexItem>
<IndicatorStyled colorValue={ value } />
<FlexItem>
{ isEditing && ! canOnlyChangeValues ? (
<NameInput
Expand All @@ -235,7 +234,12 @@ function Option< T extends Color | Gradient >( {
}
/>
) : (
<NameContainer>{ element.name }</NameContainer>
<NameContainer>
{ element.name.trim().length
? element.name
: /* Fall back to non-breaking space to maintain height */
'\u00A0' }
</NameContainer>
) }
</FlexItem>
{ isEditing && ! canOnlyChangeValues && (
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/palette-edit/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Default.args = {
colors: [
{ color: '#1a4548', name: 'Primary', slug: 'primary' },
{ color: '#0000ff', name: 'Secondary', slug: 'secondary' },
{ color: '#fb326b', name: 'Tertiary', slug: 'tertiary' },
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Increased this to three colors so we can better check the border radiuses in details view.

],
paletteLabel: 'Colors',
emptyMessage: 'Colors are empty',
Expand Down
72 changes: 59 additions & 13 deletions ...ges/components/src/palette-edit/styles.js → ...ges/components/src/palette-edit/styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import styled from '@emotion/styled';
import { css } from '@emotion/react';

/**
* Internal dependencies
Expand All @@ -10,20 +11,22 @@ import Button from '../button';
import { Heading } from '../heading';
import { HStack } from '../h-stack';
import { space } from '../utils/space';
import { COLORS, CONFIG } from '../utils';
import { COLORS, CONFIG, font } from '../utils';
import { View } from '../view';
import InputControl from '../input-control';
import {
Container as InputControlContainer,
Input,
BackdropUI as InputBackdropUI,
} from '../input-control/styles/input-control-styles';
import CircularOptionPicker from '../circular-option-picker';
import ColorIndicator from '../color-indicator';

export const IndicatorStyled = styled( CircularOptionPicker.Option )`
width: ${ space( 6 ) };
height: ${ space( 6 ) };
pointer-events: none;
export const IndicatorStyled = styled( ColorIndicator )`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this from CircularOptionPicker.Option (which is a button) to a simple ColorIndicator. This isn't supposed to be an independent button — the entire item row is a button with a single onClick handler.

&& {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this added specificity, the intended sizing (24px) was sometimes not being applied due to load order. This was one of the things causing style inconsistencies (#49041).

flex-shrink: 0;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prevents the indicator from shrinking when the color name is too long.

width: ${ space( 6 ) };
height: ${ space( 6 ) };
}
`;

export const NameInputControl = styled( InputControl )`
Expand All @@ -40,20 +43,66 @@ export const NameInputControl = styled( InputControl )`
}
`;

const buttonStyleReset = ( {
as,
}: {
as: React.ComponentProps< typeof View >[ 'as' ];
} ) => {
if ( as === 'button' ) {
return css`
display: flex;
align-items: center;
width: 100%;
appearance: none;
background: transparent;
border: none;
border-radius: 0;
padding: 0;
cursor: pointer;

&:hover {
color: ${ COLORS.theme.accent };
}
`;
}
return null;
};

export const PaletteItem = styled( View )`
${ buttonStyleReset }

padding-block: 3px;
padding-inline-start: ${ space( 3 ) };
border: 1px solid ${ CONFIG.surfaceBorderColor };
border-bottom-color: transparent;
&:first-of-type {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworked this part because :first-of-type will not work anymore with the button/div polymorphism, and Emotion doesn't want us to use :first-child for SSR safety. (Apparently :last-child is ok)

border-top-left-radius: ${ CONFIG.controlBorderRadius };
border-top-right-radius: ${ CONFIG.controlBorderRadius };
font-size: ${ font( 'default.fontSize' ) };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This addresses #49041.


&:focus-visible {
border-color: transparent;
box-shadow: 0 0 0 var( --wp-admin-border-width-focus )
var(
--wp-components-color-accent,
var( --wp-admin-theme-color, ${ COLORS.theme.accent } )
);
// Windows high contrast mode.
outline: 2px solid transparent;
outline-offset: 0;
}

border-top-left-radius: ${ CONFIG.controlBorderRadius };
border-top-right-radius: ${ CONFIG.controlBorderRadius };

& + & {
border-top-left-radius: 0;
border-top-right-radius: 0;
}
&:last-of-type {

&:last-child {
border-bottom-left-radius: ${ CONFIG.controlBorderRadius };
border-bottom-right-radius: ${ CONFIG.controlBorderRadius };
border-bottom-color: ${ CONFIG.surfaceBorderColor };
}

&.is-selected + & {
border-top-color: transparent;
}
Expand All @@ -68,9 +117,6 @@ export const NameContainer = styled.div`
margin-right: ${ space( 2 ) };
white-space: nowrap;
overflow: hidden;
${ PaletteItem }:hover & {
color: ${ COLORS.theme.accent };
}
`;

export const PaletteHeading = styled( Heading )`
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/palette-edit/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ describe( 'PaletteEdit', () => {
name: 'Show details',
} )
);
await click( screen.getByText( 'Primary' ) );
await click( screen.getByRole( 'button', { name: 'Edit: Primary' } ) );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The faux button is now properly accessible 🎉

await click(
screen.getByRole( 'button', {
name: 'Remove color',
Expand Down Expand Up @@ -328,7 +328,7 @@ describe( 'PaletteEdit', () => {
name: 'Show details',
} )
);
await click( screen.getByText( 'Primary' ) );
await click( screen.getByRole( 'button', { name: 'Edit: Primary' } ) );
const nameInput = screen.getByRole( 'textbox', {
name: 'Color name',
} );
Expand Down
Loading