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

DropdownMenu: Deprecate 36px default size for toggle button #68329

Open
wants to merge 6 commits into
base: button-deprecate-size
Choose a base branch
from
Open

DropdownMenu: Deprecate 36px default size for toggle button #68329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b6e5376
DropdownMenu: Deprecate 36px default size for toggle button
mirka Dec 26, 2024
19c8b1f
Update consumers
mirka Dec 26, 2024
f9cd7fc
Update manual code snippet in story
mirka Dec 26, 2024
d0c4317
Remove test story
mirka Dec 26, 2024
8e20d65
Update code snippets
mirka Dec 26, 2024
3d56175
Add changelog
mirka Dec 26, 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
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/alignment-control/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ exports[`AlignmentUI should match snapshot when controls are hidden 1`] = `
aria-expanded="false"
aria-haspopup="true"
aria-label="Align text"
class="components-button components-dropdown-menu__toggle has-icon"
class="components-button components-dropdown-menu__toggle is-compact has-icon"
Copy link
Member Author

Choose a reason for hiding this comment

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

AlignmentControl

data-toolbar-item="true"
type="button"
>
Expand Down
2 changes: 1 addition & 1 deletion ...ages/block-editor/src/components/block-alignment-control/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ exports[`BlockAlignmentUI should match snapshot when controls are hidden 1`] = `
aria-expanded="false"
aria-haspopup="true"
aria-label="Align"
class="components-button components-dropdown-menu__toggle has-icon"
class="components-button components-dropdown-menu__toggle is-compact has-icon"
Copy link
Member Author

Choose a reason for hiding this comment

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

BlockAlignmentControl

data-toolbar-item="true"
type="button"
>
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-settings-menu/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function BlockSettingsMenu( { clientIds, ...props } ) {
{ ( toggleProps ) => (
<BlockSettingsDropdown
clientIds={ clientIds }
toggleProps={ toggleProps }
toggleProps={ { ...toggleProps, size: 'compact' } }
Copy link
Member Author

Choose a reason for hiding this comment

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

BlockSettingsMenu

Copy link
Member

Choose a reason for hiding this comment

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

In some instances, like here, we'll override the size coming from the current toggleProps.

In others, we'll prioritize the user value:

gutenberg/packages/block-editor/src/components/block-switcher/index.js

Line 337 in 19c8b1f

size: 'compact',

Is there a reason for this discrepancy, and should we treat them consistently?

{ ...props }
/>
) }
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/block-switcher/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ export const BlockSwitcher = ( { clientIds } ) => {
}
text={ blockIndicatorText }
toggleProps={ {
size: 'compact',
Copy link
Member Author

Choose a reason for hiding this comment

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

BlockSwitcher

description: blockSwitcherDescription,
...toggleProps,
} }
Expand Down
2 changes: 1 addition & 1 deletion ...k-editor/src/components/block-vertical-alignment-control/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ exports[`BlockVerticalAlignmentUI should match snapshot when controls are hidden
aria-expanded="false"
aria-haspopup="true"
aria-label="Change vertical alignment"
class="components-button components-dropdown-menu__toggle has-icon"
class="components-button components-dropdown-menu__toggle is-compact has-icon"
Copy link
Member Author

Choose a reason for hiding this comment

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

BlockVerticalAlignmentControl

data-toolbar-item="true"
type="button"
>
Expand Down
1 change: 1 addition & 0 deletions packages/block-editor/src/components/warning/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ function Warning( { className, actions, children, secondaryActions } ) {
className:
'block-editor-warning__dropdown',
} }
toggleProps={ { size: 'compact' } }
noIcons
>
{ () => (
Expand Down
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 @@ -15,6 +15,7 @@
- `RadioGroup`: Log deprecation warning ([#68067](https://github.com/WordPress/gutenberg/pull/68067)).
- Soft deprecate `ButtonGroup` component. Use `ToggleGroupControl` instead ([#65429](https://github.com/WordPress/gutenberg/pull/65429)).
- `Navigation`: Log deprecation warning for removal in WP 7.1. Use `Navigator` instead ([#68158](https://github.com/WordPress/gutenberg/pull/68158)).
- `DropdownMenu`: Deprecate 36px default size for default toggle button ([#68329](https://github.com/WordPress/gutenberg/pull/68329)).

### Bug Fixes

Expand Down
7 changes: 6 additions & 1 deletion packages/components/src/dropdown-menu/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const MyDropdownMenu = () => (
<DropdownMenu
icon={ more }
label="Select a direction"
toggleProps={ { size: 'compact' } }
controls={ [
{
title: 'Up',
Expand Down Expand Up @@ -100,7 +101,11 @@ import { DropdownMenu, MenuGroup, MenuItem } from '@wordpress/components';
import { more, arrowUp, arrowDown, trash } from '@wordpress/icons';

const MyDropdownMenu = () => (
<DropdownMenu icon={ more } label="Select a direction">
<DropdownMenu
icon={ more }
label="Select a direction"
toggleProps={ { size: 'compact' } }
>
{ ( { onClose } ) => (
<>
<MenuGroup>
Expand Down
22 changes: 21 additions & 1 deletion packages/components/src/dropdown-menu/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import type {
DropdownOption,
DropdownMenuInternalContext,
} from './types';
import { maybeWarnDeprecated36pxSize } from '../utils/deprecated-36px-size';

function mergeProps<
T extends { className?: string; [ key: string ]: unknown },
Expand Down Expand Up @@ -82,6 +83,17 @@ function UnconnectedDropdownMenu( dropdownMenuProps: DropdownMenuProps ) {
}
}

if ( toggleProps?.as === undefined ) {
maybeWarnDeprecated36pxSize( {
componentName: 'DropdownMenu',
__next40pxDefaultSize: toggleProps?.__next40pxDefaultSize,
size: toggleProps?.size,
feature:
'36px default size for default toggle button in wp.components.DropdownMenu',
hint: 'Set `toggleProps={ __next40pxDefaultSize: true }` to start opting into the new default size, which will become the default in a future version. For icon buttons, consider setting a non-default size like `toggleProps={ size: "compact" }`.',
} );
}

const mergedPopoverProps = mergeProps(
{
className: 'components-dropdown-menu__popover',
Expand Down Expand Up @@ -120,6 +132,9 @@ function UnconnectedDropdownMenu( dropdownMenuProps: DropdownMenuProps ) {
return (
<Toggle
{ ...mergedToggleProps }
{ ...( toggleProps?.as === undefined
? { __shouldNotWarnDeprecated36pxSize: true }
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 is to prevent a redundant warning from Button when DropdownMenu has already warned.

: {} ) }
icon={ icon }
onClick={
( ( event ) => {
Expand Down Expand Up @@ -242,6 +257,7 @@ function UnconnectedDropdownMenu( dropdownMenuProps: DropdownMenuProps ) {
* <DropdownMenu
* icon={ more }
* label="Select a direction"
* toggleProps={ { size: 'compact' } }
* controls={ [
* {
* title: 'Up',
Expand Down Expand Up @@ -276,7 +292,11 @@ function UnconnectedDropdownMenu( dropdownMenuProps: DropdownMenuProps ) {
* import { more, arrowUp, arrowDown, trash } from '@wordpress/icons';
*
* const MyDropdownMenu = () => (
* <DropdownMenu icon={ more } label="Select a direction">
* <DropdownMenu
* icon={ more }
* label="Select a direction"
* toggleProps={ { size: 'compact' } }
* >
* { ( { onClose } ) => (
* <>
* <MenuGroup>
Expand Down
8 changes: 7 additions & 1 deletion packages/components/src/dropdown-menu/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export default meta;
export const Default: StoryObj< typeof DropdownMenu > = {
args: {
label: 'Select a direction.',
toggleProps: { size: 'compact' },
icon: menu,
controls: [
{
Expand All @@ -72,7 +73,11 @@ export const WithChildren: StoryObj< typeof DropdownMenu > = {
parameters: {
docs: {
source: {
code: `<DropdownMenu label="Select a direction." icon={ more }>
code: `<DropdownMenu
label="Select a direction."
icon={ more }
toggleProps={ { size: 'compact' } }
>
<MenuGroup>
<MenuItem icon={ arrowUp } onClick={ onClose }>
Move Up
Expand All @@ -94,6 +99,7 @@ export const WithChildren: StoryObj< typeof DropdownMenu > = {
},
args: {
label: 'Select a direction.',
toggleProps: { size: 'compact' },
icon: more,
children: ( { onClose } ) => (
<>
Expand Down
12 changes: 11 additions & 1 deletion packages/components/src/dropdown-menu/test/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,19 @@ import { arrowLeft, arrowRight, arrowUp, arrowDown } from '@wordpress/icons';
/**
* Internal dependencies
*/
import DropdownMenu from '..';
import _DropdownMenu from '..';
import MenuItem from '../../menu-item';

const DropdownMenu = ( {
toggleProps,
...restProps
}: React.ComponentProps< typeof _DropdownMenu > ) => (
<_DropdownMenu
toggleProps={ { size: 'compact', ...toggleProps } }
{ ...restProps }
/>
);

describe( 'DropdownMenu', () => {
it( 'should not render when neither controls nor children are assigned', () => {
render( <DropdownMenu label="Open dropdown" /> );
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/toolbar/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ Default.args = {
title: 'Align right',
},
] }
toggleProps={ toggleProps }
toggleProps={ { ...toggleProps, size: 'compact' } }
/>
) }
</ToolbarItem>
Expand Down
5 changes: 4 additions & 1 deletion packages/components/src/toolbar/toolbar-dropdown-menu/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ function ToolbarDropdownMenu(
popoverProps={ {
...props.popoverProps,
} }
toggleProps={ toolbarItemProps }
toggleProps={ {
size: 'compact',
...toolbarItemProps,
} }
Comment on lines +40 to +43
Copy link
Member Author

Choose a reason for hiding this comment

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

As seen in the default Storybook story:

ToolbarDropdownMenu

/>
) }
</ToolbarItem>
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/toolbar/toolbar-group/toolbar-group-collapsed.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ function ToolbarGroupCollapsed( {
controls={ controls }
toggleProps={ {
...internalToggleProps,
size: 'compact',
Copy link
Member Author

Choose a reason for hiding this comment

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

As seen in the default Storybook story:

ToolbarGroupCollapsed

'data-toolbar-item': true,
} }
{ ...props }
Expand Down
Loading