From fdce407ae43fc121f36797b96e6d808ed1f3077e Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Fri, 11 Feb 2022 14:33:38 +0900 Subject: [PATCH 1/7] FontSizePicker: Refactor stories to use Controls --- .../src/font-size-picker/stories/index.js | 167 ++++++------------ 1 file changed, 58 insertions(+), 109 deletions(-) diff --git a/packages/components/src/font-size-picker/stories/index.js b/packages/components/src/font-size-picker/stories/index.js index a1ee37c036df53..510dfbd5308d73 100644 --- a/packages/components/src/font-size-picker/stories/index.js +++ b/packages/components/src/font-size-picker/stories/index.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import { number, object, boolean } from '@storybook/addon-knobs'; - /** * WordPress dependencies */ @@ -16,14 +11,13 @@ import FontSizePicker from '../'; export default { title: 'Components/FontSizePicker', component: FontSizePicker, - parameters: { - knobs: { disable: false }, + argTypes: { + initialValue: { table: { disable: true } }, // hide prop because it's not actually part of FontSizePicker }, }; const FontSizePickerWithState = ( { initialValue, ...props } ) => { - const [ fontSize, setFontSize ] = useState( initialValue || 16 ); - + const [ fontSize, setFontSize ] = useState( initialValue ); return ( { ); }; -export const _default = () => { - const fontSizes = object( 'Font Sizes', [ - { - name: 'Small', - slug: 'small', - size: 12, - }, - { - name: 'Normal', - slug: 'normal', - size: 16, - }, - { - name: 'Big', - slug: 'big', - size: 26, - }, - ] ); - return ; -}; - -export const withSlider = () => { - const fontSizes = object( 'Font Sizes', [ - { - name: 'Small', - slug: 'small', - size: 12, - }, - { - name: 'Normal', - slug: 'normal', - size: 16, - }, - { - name: 'Big', - slug: 'big', - size: 26, - }, - ] ); - const fallbackFontSize = number( 'Fallback Font Size - Slider Only', 16 ); +const TwoFontSizePickersWithState = ( { fontSizes, ...props } ) => { return ( - + <> +

Fewer font sizes

+ + +

More font sizes

+ + ); }; -export const withoutCustomSizes = () => { - const fontSizes = object( 'Font Sizes', [ +export const Default = FontSizePickerWithState.bind( {} ); +Default.args = { + disableCustomFontSizes: false, + fontSizes: [ { name: 'Small', slug: 'small', @@ -99,17 +61,30 @@ export const withoutCustomSizes = () => { slug: 'big', size: 26, }, - ] ); - return ( - - ); + ], + initialValue: 16, + withSlider: false, + withReset: true, +}; + +export const WithSlider = FontSizePickerWithState.bind( {} ); +WithSlider.args = { + ...Default.args, + fallbackFontSize: 16, + initialValue: undefined, + withSlider: true, }; -export const differentControlBySize = () => { - const options = [ +export const WithoutCustomSizes = FontSizePickerWithState.bind( {} ); +WithoutCustomSizes.args = { + ...Default.args, + disableCustomFontSizes: true, +}; + +export const WithMoreFontSizes = FontSizePickerWithState.bind( {} ); +WithMoreFontSizes.args = { + ...Default.args, + fontSizes: [ { name: 'Tiny', slug: 'tiny', @@ -140,29 +115,29 @@ export const differentControlBySize = () => { slug: 'huge', size: 36, }, - ]; - const optionsWithUnits = options.map( ( option ) => ( { + ], + initialValue: 8, +}; + +export const WithUnits = TwoFontSizePickersWithState.bind( {} ); +WithUnits.args = { + ...WithMoreFontSizes.args, + fontSizes: WithMoreFontSizes.args.fontSizes.map( ( option ) => ( { ...option, size: `${ option.size }px`, - } ) ); - const showMoreFontSizes = boolean( 'Add more font sizes', false ); - const addUnitsToSizes = boolean( 'Add units to font sizes', false ); - const _options = addUnitsToSizes ? optionsWithUnits : options; - const fontSizes = _options.slice( - 0, - showMoreFontSizes ? _options.length : 4 - ); - return ( - - ); + } ) ), + initialValue: '8px', }; -export const withComplexCSSValues = () => { - const options = [ +export const WithComplexCSSValues = TwoFontSizePickersWithState.bind( {} ); +WithComplexCSSValues.args = { + ...Default.args, + fontSizes: [ { name: 'Small', slug: 'small', - size: '0.65rem', + // Adding just one complex css value is enough + size: 'clamp(1.75rem, 3vw, 2.25rem)', }, { name: 'Medium', @@ -189,32 +164,6 @@ export const withComplexCSSValues = () => { slug: 'huge', size: '2.8rem', }, - ]; - const showMoreFontSizes = boolean( 'Add more font sizes', false ); - const addComplexCssValues = boolean( - 'Add some complex css values(calc, var, etc..)', - true - ); - - const _options = options.map( ( option, index ) => { - const _option = { ...option }; - // Adding just one complex css value is enough (first element); - if ( addComplexCssValues && ! index ) { - _option.size = 'clamp(1.75rem, 3vw, 2.25rem)'; - } - return _option; - } ); - - const fontSizes = _options.slice( - 0, - showMoreFontSizes ? _options.length : 5 - ); - return ( -
- -
- ); + ], + initialValue: '1.125rem', }; From b3928bb5a3c7ab0192b9405742600f872cdcf3ae Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Sat, 19 Feb 2022 01:50:14 +0900 Subject: [PATCH 2/7] Fix docs for `withReset` --- packages/components/src/font-size-picker/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/components/src/font-size-picker/README.md b/packages/components/src/font-size-picker/README.md index 5e95dfa51580ce..48fdf6a4c6f6b3 100644 --- a/packages/components/src/font-size-picker/README.md +++ b/packages/components/src/font-size-picker/README.md @@ -100,8 +100,7 @@ If `true`, the UI will contain a slider, instead of a numeric text input field. ### withReset -If `true`, a reset button will be displayed alongside the predefined and custom -font size fields. +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`. - Type: `Boolean` - Required: no From 7daac71f179f7cb205275a0522087dff4a2e9cf2 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Sat, 19 Feb 2022 03:27:25 +0900 Subject: [PATCH 3/7] Expand code snippets and show descriptions in Controls --- packages/components/src/font-size-picker/stories/index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/src/font-size-picker/stories/index.js b/packages/components/src/font-size-picker/stories/index.js index 510dfbd5308d73..389e5cd452bc57 100644 --- a/packages/components/src/font-size-picker/stories/index.js +++ b/packages/components/src/font-size-picker/stories/index.js @@ -14,6 +14,10 @@ export default { argTypes: { initialValue: { table: { disable: true } }, // hide prop because it's not actually part of FontSizePicker }, + parameters: { + controls: { expanded: true }, + docs: { source: { state: 'open' } }, + }, }; const FontSizePickerWithState = ( { initialValue, ...props } ) => { From 06bfa773295814d0813e5205e4e8c07c175301b2 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Sat, 19 Feb 2022 03:35:30 +0900 Subject: [PATCH 4/7] Tweak `withReset` setup for better code snippets These can be removed once the component is TS converted --- packages/components/src/font-size-picker/stories/index.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/components/src/font-size-picker/stories/index.js b/packages/components/src/font-size-picker/stories/index.js index 389e5cd452bc57..0bf062a94e8d66 100644 --- a/packages/components/src/font-size-picker/stories/index.js +++ b/packages/components/src/font-size-picker/stories/index.js @@ -13,6 +13,13 @@ export default { component: FontSizePicker, argTypes: { initialValue: { table: { disable: true } }, // hide prop because it's not actually part of FontSizePicker + withReset: { + control: { type: 'boolean' }, + table: { + type: 'boolean', + defaultValue: { summary: true }, + }, + }, }, parameters: { controls: { expanded: true }, @@ -68,7 +75,6 @@ Default.args = { ], initialValue: 16, withSlider: false, - withReset: true, }; export const WithSlider = FontSizePickerWithState.bind( {} ); From 71130faaba2c5656cc7aa41412d821617dec6564 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Sat, 19 Feb 2022 03:35:56 +0900 Subject: [PATCH 5/7] Add manual descriptions for confusing props These can be removed once the component is TS converted --- packages/components/src/font-size-picker/stories/index.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/components/src/font-size-picker/stories/index.js b/packages/components/src/font-size-picker/stories/index.js index 0bf062a94e8d66..7cb92f89b09514 100644 --- a/packages/components/src/font-size-picker/stories/index.js +++ b/packages/components/src/font-size-picker/stories/index.js @@ -13,7 +13,13 @@ export default { component: FontSizePicker, argTypes: { initialValue: { table: { disable: true } }, // hide prop because it's not actually part of FontSizePicker + fallbackFontSize: { + description: + 'If no value exists, this prop defines the starting position for the font size picker slider. Only relevant if `withSlider` is `true`.', + }, 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`.', control: { type: 'boolean' }, table: { type: 'boolean', From ad5a988e8ba3401d563393f18b84309682023d8d Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Sat, 19 Feb 2022 03:36:54 +0900 Subject: [PATCH 6/7] Add descriptions to confusing stories This would be more convenient https://github.com/izhan/storybook-description-loader --- .../src/font-size-picker/stories/index.js | 36 +++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/components/src/font-size-picker/stories/index.js b/packages/components/src/font-size-picker/stories/index.js index 7cb92f89b09514..4ad792592e78a2 100644 --- a/packages/components/src/font-size-picker/stories/index.js +++ b/packages/components/src/font-size-picker/stories/index.js @@ -91,11 +91,19 @@ WithSlider.args = { withSlider: true, }; -export const WithoutCustomSizes = FontSizePickerWithState.bind( {} ); -WithoutCustomSizes.args = { +export const WithCustomSizesDisabled = FontSizePickerWithState.bind( {} ); +WithCustomSizesDisabled.args = { ...Default.args, disableCustomFontSizes: true, }; +WithCustomSizesDisabled.parameters = { + docs: { + description: { + story: + 'When disabled, the user will only be able to pick one of the predefined sizes passed in `fontSizes`.', + }, + }, +}; export const WithMoreFontSizes = FontSizePickerWithState.bind( {} ); WithMoreFontSizes.args = { @@ -134,6 +142,14 @@ WithMoreFontSizes.args = { ], initialValue: 8, }; +WithMoreFontSizes.parameters = { + docs: { + description: { + story: + 'When there are more than 5 font size options, the UI is no longer a toggle group.', + }, + }, +}; export const WithUnits = TwoFontSizePickersWithState.bind( {} ); WithUnits.args = { @@ -144,6 +160,14 @@ WithUnits.args = { } ) ), initialValue: '8px', }; +WithUnits.parameters = { + docs: { + description: { + story: + 'When units like `px` are specified explicitly, it will be shown as a label hint.', + }, + }, +}; export const WithComplexCSSValues = TwoFontSizePickersWithState.bind( {} ); WithComplexCSSValues.args = { @@ -183,3 +207,11 @@ WithComplexCSSValues.args = { ], initialValue: '1.125rem', }; +WithComplexCSSValues.parameters = { + docs: { + description: { + story: + 'The label hint will not be shown if it is a complex CSS value. Some examples of complex CSS values in this context are CSS functions like `calc()`, `clamp()`, and `var()`.', + }, + }, +}; From a2b660e6e0eb4633c83b481c8ca71c22b09df460 Mon Sep 17 00:00:00 2001 From: Lena Morita Date: Wed, 2 Mar 2022 19:40:18 +0900 Subject: [PATCH 7/7] Improve story description Co-authored-by: Marco Ciampini --- packages/components/src/font-size-picker/stories/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/font-size-picker/stories/index.js b/packages/components/src/font-size-picker/stories/index.js index 4ad792592e78a2..c124fb03acc559 100644 --- a/packages/components/src/font-size-picker/stories/index.js +++ b/packages/components/src/font-size-picker/stories/index.js @@ -100,7 +100,7 @@ WithCustomSizesDisabled.parameters = { docs: { description: { story: - 'When disabled, the user will only be able to pick one of the predefined sizes passed in `fontSizes`.', + 'With custom font sizes disabled via the `disableCustomFontSizes` prop, the user will only be able to pick one of the predefined sizes passed in `fontSizes`.', }, }, };