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

Add shortName property to FontSize Preset to override t-shirt size label #46586

Closed
wants to merge 10 commits into from
Closed

Add shortName property to FontSize Preset to override t-shirt size label #46586

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c1f4d54
add `shortName` to FontSizes preset parameters in theme.json JSON schema
fabiankaegy Dec 15, 2022
2d35351
display fontSize short name in FontSizePicker if preset defines it
fabiankaegy Dec 15, 2022
1aa1593
add shortName to living theme.json documentation
fabiankaegy Dec 15, 2022
49ec766
Update packages/components/src/font-size-picker/font-size-picker-togg…
fabiankaegy Dec 15, 2022
01ee1ec
move hasValidShortName into utils folder
fabiankaegy Dec 15, 2022
dc9fdd4
Revert "Update packages/components/src/font-size-picker/font-size-pic…
fabiankaegy Dec 15, 2022
245644a
Update packages/components/src/font-size-picker/utils.ts
fabiankaegy Dec 16, 2022
6b32b74
rename font size picker constants for better consistency
fabiankaegy Dec 16, 2022
d6aa63a
fix description of shortName property in json schema and add min / ma…
fabiankaegy Dec 16, 2022
b33e8c4
fix remove unneccacary undefined type from hasValidShortName function
fabiankaegy Dec 16, 2022
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 docs/reference-guides/theme-json-reference/theme-json-living.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ Settings related to typography.
| textDecoration | boolean | true | |
| textTransform | boolean | true | |
| dropCap | boolean | true | |
| fontSizes | array | | fluid, name, size, slug |
| fontSizes | array | | fluid, name, shortName, size, slug |
| fontFamilies | array | | fontFace, fontFamily, name, slug |

---
Expand Down
24 changes: 23 additions & 1 deletion packages/components/src/font-size-picker/font-size-picker-toggle-group.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,23 @@ import {
import { T_SHIRT_ABBREVIATIONS, T_SHIRT_NAMES } from './constants';
import type { FontSizePickerToggleGroupProps } from './types';

/**
* hasValidShortName
*
* check that the font size short name is a string with at most
* 3 characters length (e.g. "XXS", "XS", "S", "M", "L", "XL", "XXL")
*
* @param shortName font size object
* @return boolean
*/
function hasValidShortName( shortName: string | undefined ): boolean {
return (
typeof shortName === 'string' &&
shortName.length >= 1 &&
shortName.length <= 3
);
}
fabiankaegy marked this conversation as resolved.
Show resolved Hide resolved

const FontSizePickerToggleGroup = ( props: FontSizePickerToggleGroupProps ) => {
const { fontSizes, value, __nextHasNoMarginBottom, size, onChange } = props;
return (
Expand All @@ -29,7 +46,12 @@ const FontSizePickerToggleGroup = ( props: FontSizePickerToggleGroupProps ) => {
<ToggleGroupControlOption
key={ fontSize.slug }
value={ fontSize.size }
label={ T_SHIRT_ABBREVIATIONS[ index ] }
label={
fontSize.shortName &&
hasValidShortName( fontSize.shortName )
Comment on lines +34 to +35
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fontSize.shortName &&
hasValidShortName( fontSize.shortName )
hasValidShortName( fontSize?.shortName )

Suggestion: The hasValidShortName will return false if short name is undefined.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Mamaduka the reason I added back in the duplicate check is that TypeScript is complaining about it for whatever reason... See https://github.com/WordPress/gutenberg/actions/runs/3706287491/jobs/6281315087

Copy link
Member

Choose a reason for hiding this comment

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

I just noticed it. Sorry about that. Let's revert for now, and maybe more TS-savvy people can help us here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reverted in dc9fdd4

Copy link
Member

Choose a reason for hiding this comment

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

It should work to remove this so long as hasValidShortName's argument type is string | undefined.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah for whatever reason that doesn't work for me. As per your later comment, I now removed the | undefined type from the function and kept the duplicate check inline.

? fontSize.shortName
: T_SHIRT_ABBREVIATIONS[ index ]
fabiankaegy marked this conversation as resolved.
Show resolved Hide resolved
}
aria-label={ fontSize.name || T_SHIRT_NAMES[ index ] }
showTooltip
/>
Expand Down
5 changes: 5 additions & 0 deletions packages/components/src/font-size-picker/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ export type FontSize = {
* size. Used for the class generation process.
*/
slug: string;
/**
* The `shortName` property is a string with a short label for that font
* size e.g.: `S`.
*/
shortName?: string;
};

export type FontSizePickerSelectProps = Pick<
Expand Down
4 changes: 4 additions & 0 deletions schemas/json/theme.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,10 @@
"description": "Kebab-case unique identifier for the font size preset.",
"type": "string"
},
"shortName": {
"description": "Short name of the font size preset. Gets used in the font size selector if fewer than 5 presets are defined.",
fabiankaegy marked this conversation as resolved.
Show resolved Hide resolved
"type": "string"
},
"size": {
"description": "CSS font-size value, including units.",
"type": "string"
Expand Down