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.

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

Add reset button to color control #67116

Merged
merged 10 commits into from
Dec 10, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ $swatch-gap: 12px;
.block-editor-tools-panel-color-gradient-settings__item {
padding: 0;
max-width: 100%;
position: relative;

// Border styles.
border-left: 1px solid $gray-300;
Expand Down Expand Up @@ -120,3 +121,17 @@ $swatch-gap: 12px;
flex-shrink: 0;
}
}

.block-editor-panel-color-gradient-settings__reset {
position: absolute;
right: 0;
top: 0;
opacity: 0;
Copy link
Member

Choose a reason for hiding this comment

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

Can we confirm that it's better if the button is hidden by default?

I'm having second thoughts if this is good for discovery. A user might be wondering how to reset the color and wouldn't know unless they hover on the tools panel item.

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 intention was to show the reset button only on hover or focus, making it easier to reset colors with fewer clicks.

From my perspective, showing the button on hover is a subtle and unobtrusive way to achieve this without adding visual clutter to the UI. As for discoverability, I think users will naturally figure it out when they try resetting colors the usual way, which should be sufficient.

transition: opacity 0.1s ease-in-out;
juanfra marked this conversation as resolved.
Show resolved Hide resolved

.block-editor-panel-color-gradient-settings__dropdown:hover + &,
&:focus,
&:hover {
opacity: 1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import {
Button,
privateApis as componentsPrivateApis,
} from '@wordpress/components';
import { useCallback } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { useCallback, useRef } from '@wordpress/element';
import { __, sprintf } from '@wordpress/i18n';

/**
* Internal dependencies
Expand All @@ -30,6 +30,7 @@ import { useColorsPerOrigin, useGradientsPerOrigin } from './hooks';
import { getValueFromVariable, useToolsPanelDropdownMenuProps } from './utils';
import { setImmutably } from '../../utils/object';
import { unlock } from '../../lock-unlock';
import { reset as resetIcon } from '@wordpress/icons';

export function useHasColorPanel( settings ) {
const hasTextPanel = useHasTextPanel( settings );
Expand Down Expand Up @@ -208,6 +209,7 @@ function ColorPanelDropdown( {
} ) {
const currentTab = tabs.find( ( tab ) => tab.userValue !== undefined );
const { key: firstTabKey, ...firstTab } = tabs[ 0 ] ?? {};
const colorGradientDropdownButtonRef = useRef( undefined );
return (
<ToolsPanelItem
className="block-editor-tools-panel-color-gradient-settings__item"
Expand All @@ -228,15 +230,36 @@ function ColorPanelDropdown( {
{ 'is-open': isOpen }
),
'aria-expanded': isOpen,
ref: colorGradientDropdownButtonRef,
};

return (
<Button __next40pxDefaultSize { ...toggleProps }>
<LabeledColorIndicators
indicators={ indicators }
label={ label }
/>
</Button>
<>
<Button { ...toggleProps } __next40pxDefaultSize>
<LabeledColorIndicators
indicators={ indicators }
label={ label }
/>
</Button>
{ hasValue() && (
<Button
__next40pxDefaultSize
label={ __( 'Reset' ) }
aria-label={ sprintf(
/* translators: %s: The name of the color property being reset e.g. "Background" */
__( 'Reset %s' ),
label.toLowerCase()
) }
juanfra marked this conversation as resolved.
Show resolved Hide resolved
className="block-editor-panel-color-gradient-settings__reset"
icon={ resetIcon }
onClick={ () => {
resetValue();
// Return focus to parent button
colorGradientDropdownButtonRef.current?.focus();
} }
/>
) }
</>
);
} }
renderContent={ () => (
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/specs/editor/blocks/heading.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ test.describe( 'Heading', () => {
.getByRole( 'region', {
name: 'Editor settings',
} )
.getByRole( 'button', { name: 'Text' } );
.getByRole( 'button', { name: 'Text', exact: true } );

await textColor.click();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ test.describe( 'Style Revisions', () => {
await editor.canvas.locator( 'body' ).click();
await userGlobalStylesRevisions.openStylesPanel();
await page.getByRole( 'button', { name: 'Colors' } ).click();
await page.getByRole( 'button', { name: 'Background' } ).click();
await page
.getByRole( 'button', { name: 'Background', exact: true } )
.click();
await page
.getByRole( 'option', { name: 'Color: Luminous vivid amber' } )
.click( { force: true } );
Expand Down
Loading