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

FontSizePicker: Add 40px size variant #42716

Merged
merged 4 commits into from
Aug 5, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

### Enhancements

- `FontSizePicker`: Add large size variant ([#42716](https://github.com/WordPress/gutenberg/pull/42716/)).
- `BorderControl`: Improve labelling, tooltips and DOM structure ([#42348](https://github.com/WordPress/gutenberg/pull/42348/)).
- `BaseControl`: Set zero padding on `StyledLabel` to ensure cross-browser styling ([#42348](https://github.com/WordPress/gutenberg/pull/42348/)).
- `InputControl`: Implement wrapper subcomponents for adding responsive padding to `prefix`/`suffix` ([#42378](https://github.com/WordPress/gutenberg/pull/42378)).
Expand Down
8 changes: 7 additions & 1 deletion packages/components/src/font-size-picker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ function FontSizePicker(
fontSizes = [],
disableCustomFontSizes = false,
onChange,
/** @type {'default' | '__unstable-large'} */
size = 'default',
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding a size prop for this is mostly to orchestrate the roll-out. For this one I think it would make sense to eventually remove the prop and make it large-only.

If we plan on removing it, should we prefix this prop with __unstable or __experimental?

Is the roll-out strategy going to be similar to other style deprecations, or are just planning on "flipping a switch"?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are great questions. It prompted me to think about it again, and now I'm less sure about "eventually remove the prop and make it large-only". That might not be a good idea at all, it's hard to say at the moment. So I guess we should just treat this as a normal size prop for the time being.

value,
withSlider = false,
withReset = true,
Expand All @@ -52,7 +54,7 @@ function FontSizePicker(
* than six and a select control when they are more.
*/
const fontSizesContainComplexValues = fontSizes.some(
( { size } ) => ! isSimpleCssValue( size )
( { size: sizeArg } ) => ! isSimpleCssValue( sizeArg )
);
const shouldUseSelectControl = fontSizes.length > 5;
const options = useMemo(
Expand Down Expand Up @@ -163,6 +165,7 @@ function FontSizePicker(
shouldUseSelectControl &&
! showCustomValueControl && (
<CustomSelectControl
__nextUnconstrainedWidth
className={ `${ baseClassName }__select` }
label={ __( 'Font size' ) }
hideLabelFromVision
Expand All @@ -181,6 +184,7 @@ function FontSizePicker(
setShowCustomValueControl( true );
}
} }
size={ size }
/>
) }
{ ! shouldUseSelectControl && ! showCustomValueControl && (
Expand All @@ -194,6 +198,7 @@ function FontSizePicker(
);
} }
isBlock
size={ size }
>
{ options.map( ( option ) => (
<ToggleGroupControlOption
Expand Down Expand Up @@ -233,6 +238,7 @@ function FontSizePicker(
);
}
} }
size={ size }
units={ hasUnits ? units : [] }
/>
</FlexItem>
Expand Down
6 changes: 6 additions & 0 deletions packages/components/src/font-size-picker/stories/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ export default {
description:
'If no value exists, this prop defines the starting position for the font size picker slider. Only relevant if `withSlider` is `true`.',
},
size: {
control: {
type: 'radio',
options: [ 'default', '__unstable-large' ],
},
},
withReset: {
description:
'If `true`, a reset button will be displayed alongside the input field when a custom font size is active. Has no effect when `disableCustomFontSizes` or `withSlider` is `true`.',
Expand Down