Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -28,6 +28,7 @@
- `UnitControl`: Remove outer wrapper to normalize className placement ([#41860](https://github.com/WordPress/gutenberg/pull/41860)).
- `ColorPalette`: Fix transparent checkered background pattern ([#45295](https://github.com/WordPress/gutenberg/pull/45295)).
- `ToggleGroupControl`: Add `isDeselectable` prop to allow deselecting the selected option ([#45123](https://github.com/WordPress/gutenberg/pull/45123)).
- `FontSizePicker`: Improve hint text shown next to 'Font size' label ([#44966](https://github.com/WordPress/gutenberg/pull/44966)).

### Bug Fix

Expand Down
7 changes: 7 additions & 0 deletions packages/components/src/custom-select-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export default function CustomSelectControl( props ) {
value: _selectedItem,
onMouseOver,
onMouseOut,
__experimentalShowSelectedHint = false,
Copy link
Member Author

@noisysocks noisysocks Nov 2, 2022

Choose a reason for hiding this comment

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

I obviously don't love adding yet another prop 😅

We've talked before about introducing renderItem or something like that which should negate the need for this. I figure let's add the dumb prop for now and come back to this if/when we pick #41726 back up. CustomSelectControl is in need of a lot of love!

Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a reasonable compromise that we can remove once we're done reworking CustomSelectControl. It should be ok to remove this prop in the near future before it makes its way into WordPress core.

CustomSelectControl is in need of a lot of love!

We are well aware of it! Unfortunately our low capacity and some unforeseen high priority tasks (i.e. fixing the high amount of regressions introduced by the Popover refactor) caused some delays. We should be able to resume work on it very soon (maybe we can even start collaborating on it next week, if we find some time)

Copy link
Contributor

Choose a reason for hiding this comment

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

Just a quick request — can we add its associated control in Storybook?

diff --git a/packages/components/src/custom-select-control/stories/index.js b/packages/components/src/custom-select-control/stories/index.js
index 585c380923..4891bcf210 100644
--- a/packages/components/src/custom-select-control/stories/index.js
+++ b/packages/components/src/custom-select-control/stories/index.js
@@ -8,6 +8,7 @@ export default {
 	component: CustomSelectControl,
 	argTypes: {
 		__next36pxDefaultSize: { control: { type: 'boolean' } },
+		__experimentalShowSelectedHint: { control: { type: 'boolean' } },
 		size: {
 			control: {
 				type: 'radio',

Copy link
Member Author

Choose a reason for hiding this comment

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

(maybe we can even start collaborating on it next week, if we find some time)

Let's!

} = props;

const {
Expand Down Expand Up @@ -194,6 +195,12 @@ export default function CustomSelectControl( props ) {
} ) }
>
{ itemToString( selectedItem ) }
{ __experimentalShowSelectedHint &&
selectedItem.__experimentalHint && (
<span className="components-custom-select-control__hint">
{ selectedItem.__experimentalHint }
</span>
) }
</SelectControlSelect>
</InputBaseWithBackCompatMinWidth>
{ /* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */ }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default {
component: CustomSelectControl,
argTypes: {
__next36pxDefaultSize: { control: { type: 'boolean' } },
__experimentalShowSelectedHint: { control: { type: 'boolean' } },
size: {
control: {
type: 'radio',
Expand Down
7 changes: 6 additions & 1 deletion packages/components/src/custom-select-control/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
outline: 0; // focus ring is handled elsewhere
}

.components-custom-select-control__hint {
color: $gray-600;
margin-left: 10px;
}

.components-custom-select-control__menu {
// Hide when collapsed.
&[aria-hidden="true"] {
Expand Down Expand Up @@ -50,7 +55,7 @@
background: $gray-300;
}
.components-custom-select-control__item-hint {
color: $gray-700;
color: $gray-600;
text-align: right;
padding-right: $grid-unit-05;
}
Expand Down
39 changes: 39 additions & 0 deletions packages/components/src/custom-select-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,43 @@ describe( 'CustomSelectControl', () => {

expect( onKeyDown ).toHaveBeenCalledTimes( 0 );
} );

it( 'does not show selected hint by default', () => {
render(
<CustomSelectControl
label="Custom select"
options={ [
{
key: 'one',
name: 'One',
__experimentalHint: 'Hint',
},
] }
__nextUnconstrainedWidth
/>
);
expect(
screen.getByRole( 'button', { name: 'Custom select' } )
).not.toHaveTextContent( 'Hint' );
} );

it( 'shows selected hint when __experimentalShowSelectedHint is set', () => {
render(
<CustomSelectControl
label="Custom select"
options={ [
{
key: 'one',
name: 'One',
__experimentalHint: 'Hint',
},
] }
__experimentalShowSelectedHint
__nextUnconstrainedWidth
/>
);
expect(
screen.getByRole( 'button', { name: 'Custom select' } )
).toHaveTextContent( 'Hint' );
} );
} );
37 changes: 37 additions & 0 deletions packages/components/src/font-size-picker/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* List of T-shirt abbreviations.
*
* When there are 5 font sizes or fewer, we assume that the font sizes are
* ordered by size and show T-shirt labels.
*/
export const T_SHIRT_ABBREVIATIONS = [
/* translators: S stands for 'small' and is a size label. */
__( 'S' ),
/* translators: M stands for 'medium' and is a size label. */
__( 'M' ),
/* translators: L stands for 'large' and is a size label. */
__( 'L' ),
/* translators: XL stands for 'extra large' and is a size label. */
__( 'XL' ),
/* translators: XXL stands for 'extra extra large' and is a size label. */
__( 'XXL' ),
];

/**
* List of T-shirt names.
*
* When there are 5 font sizes or fewer, we assume that the font sizes are
* ordered by size and show T-shirt labels.
*/
export const T_SHIRT_NAMES = [
__( 'Small' ),
__( 'Medium' ),
__( 'Large' ),
__( 'Extra Large' ),
__( 'Extra Extra Large' ),
];
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
FontSizePickerSelectProps,
FontSizePickerSelectOption,
} from './types';
import { getCommonSizeUnit, isSimpleCssValue } from './utils';

const DEFAULT_OPTION: FontSizePickerSelectOption = {
key: 'default',
Expand All @@ -34,18 +35,27 @@ const FontSizePickerSelect = ( props: FontSizePickerSelectProps ) => {
onSelectCustom,
} = props;

const areAllSizesSameUnit = !! getCommonSizeUnit( fontSizes );

const options: FontSizePickerSelectOption[] = [
DEFAULT_OPTION,
...fontSizes.map( ( fontSize ) => {
const [ quantity ] = parseQuantityAndUnitFromRawValue(
fontSize.size
);
let hint;
if ( areAllSizesSameUnit ) {
const [ quantity ] = parseQuantityAndUnitFromRawValue(
fontSize.size
);
if ( quantity !== undefined ) {
hint = String( quantity );
}
} else if ( isSimpleCssValue( fontSize.size ) ) {
hint = String( fontSize.size );
}
return {
key: fontSize.slug,
name: fontSize.name || fontSize.slug,
value: fontSize.size,
__experimentalHint:
quantity !== undefined ? String( quantity ) : undefined,
__experimentalHint: hint,
};
} ),
...( disableCustomFontSizes ? [] : [ CUSTOM_OPTION ] ),
Expand All @@ -68,6 +78,7 @@ const FontSizePickerSelect = ( props: FontSizePickerSelectProps ) => {
) }
options={ options }
value={ selectedOption }
__experimentalShowSelectedHint
onChange={ ( {
selectedItem,
}: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,9 @@ import {
ToggleGroupControl,
ToggleGroupControlOption,
} from '../toggle-group-control';
import { T_SHIRT_ABBREVIATIONS, T_SHIRT_NAMES } from './constants';
import type { FontSizePickerToggleGroupProps } from './types';

/**
* In case we have at most five font sizes, show a `T-shirt size` alias as a
* label of the font size. The label assumes that the font sizes are ordered
* accordingly - from smallest to largest.
*/
const FONT_SIZES_ALIASES = [
/* translators: S stands for 'small' and is a size label. */
__( 'S' ),
/* translators: M stands for 'medium' and is a size label. */
__( 'M' ),
/* translators: L stands for 'large' and is a size label. */
__( 'L' ),
/* translators: XL stands for 'extra large' and is a size label. */
__( 'XL' ),
/* translators: XXL stands for 'extra extra large' and is a size label. */
__( 'XXL' ),
];

const FontSizePickerToggleGroup = ( props: FontSizePickerToggleGroupProps ) => {
const { fontSizes, value, __nextHasNoMarginBottom, size, onChange } = props;
return (
Expand All @@ -46,8 +29,8 @@ const FontSizePickerToggleGroup = ( props: FontSizePickerToggleGroupProps ) => {
<ToggleGroupControlOption
key={ fontSize.slug }
value={ fontSize.size }
label={ FONT_SIZES_ALIASES[ index ] }
aria-label={ fontSize.name || FONT_SIZES_ALIASES[ index ] }
label={ T_SHIRT_ABBREVIATIONS[ index ] }
aria-label={ fontSize.name || T_SHIRT_NAMES[ index ] }
showTooltip
/>
) ) }
Expand Down
55 changes: 16 additions & 39 deletions packages/components/src/font-size-picker/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
useCustomUnits,
} from '../unit-control';
import { VisuallyHidden } from '../visually-hidden';
import { isSimpleCssValue } from './utils';
import { getCommonSizeUnit } from './utils';
import { HStack } from '../h-stack';
import type { FontSizePickerProps } from './types';
import {
Expand All @@ -36,6 +36,7 @@ import {
import { Spacer } from '../spacer';
import FontSizePickerSelect from './font-size-picker-select';
import FontSizePickerToggleGroup from './font-size-picker-toggle-group';
import { T_SHIRT_NAMES } from './constants';

const UnforwardedFontSizePicker = (
props: FontSizePickerProps,
Expand Down Expand Up @@ -78,53 +79,29 @@ const UnforwardedFontSizePicker = (

const headerHint = useMemo( () => {
if ( showCustomValueControl ) {
return `(${ __( 'Custom' ) })`;
return __( 'Custom' );
}

// If we have a custom value that is not available in the font sizes,
// show it as a hint as long as it's a simple CSS value.
if ( isCustomValue ) {
return (
value !== undefined &&
isSimpleCssValue( value ) &&
`(${ value })`
);
if ( ! shouldUseSelectControl ) {
if ( selectedFontSize ) {
return (
selectedFontSize.name ||
T_SHIRT_NAMES[ fontSizes.indexOf( selectedFontSize ) ]
);
}
return '';
}

if ( shouldUseSelectControl ) {
return (
selectedFontSize?.size !== undefined &&
isSimpleCssValue( selectedFontSize?.size ) &&
`(${ selectedFontSize?.size })`
);
const commonUnit = getCommonSizeUnit( fontSizes );
if ( commonUnit ) {
return `(${ commonUnit })`;
}

if ( ! selectedFontSize ) {
return __( 'Default' );
}

// Calculate the `hint` for toggle group control.
let hint = selectedFontSize.name || selectedFontSize.slug;
const fontSizesContainComplexValues = fontSizes.some(
( fontSize ) => ! isSimpleCssValue( fontSize.size )
);
if (
! fontSizesContainComplexValues &&
typeof selectedFontSize.size === 'string'
) {
const [ , unit ] = parseQuantityAndUnitFromRawValue(
selectedFontSize.size,
units
);
hint += `(${ unit })`;
}
return hint;
return '';
}, [
showCustomValueControl,
isCustomValue,
selectedFontSize,
value,
shouldUseSelectControl,
selectedFontSize,
fontSizes,
] );

Expand Down
Loading