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

ToggleGroupControl : Deprecate 36px default size #66747

Merged
merged 9 commits into from
Nov 20, 2024
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- `TextControl`: Deprecate 36px default size ([#66745](https://github.com/WordPress/gutenberg/pull/66745).
- `FontSizePicker`: Deprecate 36px default size ([#66920](https://github.com/WordPress/gutenberg/pull/66920)).
- `ComboboxControl`: Deprecate 36px default size ([#66900](https://github.com/WordPress/gutenberg/pull/66900)).
- `ToggleGroupControl`: Deprecate 36px default size ([#66747](https://github.com/WordPress/gutenberg/pull/66747)).

### Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const FontSizePickerToggleGroup = ( props: FontSizePickerToggleGroupProps ) => {
<ToggleGroupControl
__nextHasNoMarginBottom
__next40pxDefaultSize={ __next40pxDefaultSize }
__shouldNotWarnDeprecated36pxSize
Copy link
Member

Choose a reason for hiding this comment

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

I think we're also going to need a prop like this to prevent the redundant warnings when used inside another component: 1b7eeec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @mirka should we also update the usage of ToggleGroupControl by adding this prop in other packages as well like block editor and library?

Copy link
Member

Choose a reason for hiding this comment

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

Not that I'm aware of. We only want to suppress the warning when ToggleGroupControl's size is dependent on a parent component's size, and the parent component is responsible for logging the warning. I'm not aware of any other cases like that.

label={ __( 'Font size' ) }
hideLabelFromVision
value={ value }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const Template: StoryFn< typeof ToggleGroupControl > = ( {
return (
<ToggleGroupControl
__nextHasNoMarginBottom
__next40pxDefaultSize
{ ...props }
onChange={ ( ...changeArgs ) => {
setValue( ...changeArgs );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ exports[`ToggleGroupControl controlled should render correctly with icons 1`] =
display: inline-flex;
min-width: 0;
position: relative;
min-height: 36px;
padding: 2px;
min-height: 40px;
padding: 3px;
}

.emotion-8:hover {
Expand Down Expand Up @@ -159,7 +159,7 @@ exports[`ToggleGroupControl controlled should render correctly with icons 1`] =
width: 100%;
z-index: 2;
color: #1e1e1e;
height: 30px;
height: 32px;
aspect-ratio: 1;
padding-left: 0;
padding-right: 0;
Expand Down Expand Up @@ -236,7 +236,7 @@ exports[`ToggleGroupControl controlled should render correctly with icons 1`] =
width: 100%;
z-index: 2;
color: #1e1e1e;
height: 30px;
height: 32px;
aspect-ratio: 1;
padding-left: 0;
padding-right: 0;
Expand Down Expand Up @@ -409,8 +409,8 @@ exports[`ToggleGroupControl controlled should render correctly with text options
display: inline-flex;
min-width: 0;
position: relative;
min-height: 36px;
padding: 2px;
min-height: 40px;
padding: 3px;
}

.emotion-8:hover {
Expand Down Expand Up @@ -678,8 +678,8 @@ exports[`ToggleGroupControl uncontrolled should render correctly with icons 1`]
display: inline-flex;
min-width: 0;
position: relative;
min-height: 36px;
padding: 2px;
min-height: 40px;
padding: 3px;
}

.emotion-8:hover {
Expand Down Expand Up @@ -793,7 +793,7 @@ exports[`ToggleGroupControl uncontrolled should render correctly with icons 1`]
width: 100%;
z-index: 2;
color: #1e1e1e;
height: 30px;
height: 32px;
aspect-ratio: 1;
padding-left: 0;
padding-right: 0;
Expand Down Expand Up @@ -870,7 +870,7 @@ exports[`ToggleGroupControl uncontrolled should render correctly with icons 1`]
width: 100%;
z-index: 2;
color: #1e1e1e;
height: 30px;
height: 32px;
aspect-ratio: 1;
padding-left: 0;
padding-right: 0;
Expand Down Expand Up @@ -1037,8 +1037,8 @@ exports[`ToggleGroupControl uncontrolled should render correctly with text optio
display: inline-flex;
min-width: 0;
position: relative;
min-height: 36px;
padding: 2px;
min-height: 40px;
padding: 3px;
}

.emotion-8:hover {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ const hoverOutside = async () => {
};

const ToggleGroupControl = ( props: ToggleGroupControlProps ) => {
return <_ToggleGroupControl { ...props } __nextHasNoMarginBottom />;
return (
<_ToggleGroupControl
{ ...props }
__nextHasNoMarginBottom
__next40pxDefaultSize
/>
);
};

const ControlledToggleGroupControl = ( {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { formatLowercase, formatUppercase } from '@wordpress/icons';

function Example() {
return (
<ToggleGroupControl __nextHasNoMarginBottom>
<ToggleGroupControl __nextHasNoMarginBottom __next40pxDefaultSize>
<ToggleGroupControlOptionIcon
value="uppercase"
icon={ formatUppercase }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function UnforwardedToggleGroupControlOptionIcon(
*
* function Example() {
* return (
* <ToggleGroupControl __nextHasNoMarginBottom>
* <ToggleGroupControl __nextHasNoMarginBottom __next40pxDefaultSize>
* <ToggleGroupControlOptionIcon
* value="uppercase"
* label="Uppercase"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ This feature is still experimental. “Experimental” means this is an early im

`ToggleGroupControlOption` is a form component and is meant to be used as a child of [`ToggleGroupControl`](/packages/components/src/toggle-group-control/toggle-group-control/README.md).


## Usage

```js
Expand All @@ -22,6 +21,7 @@ function Example() {
value="vertical"
isBlock
__nextHasNoMarginBottom
__next40pxDefaultSize
>
<ToggleGroupControlOption
value="horizontal"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ function UnforwardedToggleGroupControlOption(
* value="vertical"
* isBlock
* __nextHasNoMarginBottom
* __next40pxDefaultSize
* >
* <ToggleGroupControlOption value="horizontal" label="Horizontal" />
* <ToggleGroupControlOption value="vertical" label="Vertical" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ function Example() {
value="vertical"
isBlock
__nextHasNoMarginBottom
__next40pxDefaultSize
>
<ToggleGroupControlOption value="horizontal" label="Horizontal" />
<ToggleGroupControlOption value="vertical" label="Vertical" />
Expand Down Expand Up @@ -100,4 +101,4 @@ Start opting into the larger default height that will become the default size in
Start opting into the new margin-free styles that will become the default in a future version.

- Required: No
- Default: `false`
- Default: `false`
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { ToggleGroupControlAsButtonGroup } from './as-button-group';
import { useTrackElementOffsetRect } from '../../utils/element-rect';
import { useMergeRefs } from '@wordpress/compose';
import { useAnimatedOffsetRect } from '../../utils/hooks/use-animated-offset-rect';
import { maybeWarnDeprecated36pxSize } from '../../utils/deprecated-36px-size';

function UnconnectedToggleGroupControl(
props: WordPressComponentProps< ToggleGroupControlProps, 'div', false >,
Expand All @@ -31,6 +32,7 @@ function UnconnectedToggleGroupControl(
const {
__nextHasNoMarginBottom = false,
__next40pxDefaultSize = false,
__shouldNotWarnDeprecated36pxSize,
className,
isAdaptiveWidth = false,
isBlock = false,
Expand Down Expand Up @@ -81,6 +83,13 @@ function UnconnectedToggleGroupControl(
? ToggleGroupControlAsButtonGroup
: ToggleGroupControlAsRadioGroup;

maybeWarnDeprecated36pxSize( {
componentName: 'ToggleGroupControl',
size,
__next40pxDefaultSize,
__shouldNotWarnDeprecated36pxSize,
} );

return (
<BaseControl
help={ help }
Expand Down Expand Up @@ -135,6 +144,7 @@ function UnconnectedToggleGroupControl(
* value="vertical"
* isBlock
* __nextHasNoMarginBottom
* __next40pxDefaultSize
* >
* <ToggleGroupControlOption value="horizontal" label="Horizontal" />
* <ToggleGroupControlOption value="vertical" label="Vertical" />
Expand Down
7 changes: 7 additions & 0 deletions packages/components/src/toggle-group-control/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,13 @@ export type ToggleGroupControlProps = Pick<
* @default false
*/
__next40pxDefaultSize?: boolean;
/**
* Do not throw a warning for the deprecated 36px default size.
* For internal components of other components that already throw the warning.
*
* @ignore
*/
__shouldNotWarnDeprecated36pxSize?: boolean;
};

export type ToggleGroupControlContextProps = {
Expand Down
3 changes: 3 additions & 0 deletions packages/components/src/tools-panel/stories/index.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export const Default: StoryFn< typeof ToolsPanel > = ( {
>
<ToggleGroupControl
__nextHasNoMarginBottom
__next40pxDefaultSize
label="Scale"
value={ scale }
onChange={ ( next ) => setScale( next ) }
Expand Down Expand Up @@ -457,6 +458,7 @@ export const WithConditionalDefaultControl: StoryFn< typeof ToolsPanel > = ( {
>
<ToggleGroupControl
__nextHasNoMarginBottom
__next40pxDefaultSize
label="Scale"
value={ scale }
onChange={ ( next ) =>
Expand Down Expand Up @@ -559,6 +561,7 @@ export const WithConditionallyRenderedControl: StoryFn<
>
<ToggleGroupControl
__nextHasNoMarginBottom
__next40pxDefaultSize
label="Scale"
value={ scale }
onChange={ ( next ) =>
Expand Down
3 changes: 3 additions & 0 deletions packages/components/src/utils/deprecated-36px-size.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ export function maybeWarnDeprecated36pxSize( {
componentName,
__next40pxDefaultSize,
size,
__shouldNotWarnDeprecated36pxSize,
}: {
componentName: string;
__next40pxDefaultSize: boolean | undefined;
size: string | undefined;
__shouldNotWarnDeprecated36pxSize?: boolean;
} ) {
if (
__shouldNotWarnDeprecated36pxSize ||
__next40pxDefaultSize ||
( size !== undefined && size !== 'default' )
) {
Expand Down
Loading