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

Add: Support for other units and CSS properties in Font Size presets #26475

Conversation

jorgefilipecosta
Copy link
Member

This PR makes sure themes can use font size values on font size presets that are not in pixels.
Some examples are "2rem", "1em", or "clamp(12px, 5vw, 100px)". Any valid CSS property for font size is accepted on the preset.

The font size preset was the only preset whose value was not a valid CSS property e.g: it used "13" as a value and "13" is an invalid property while "13px" is a valid property. This causes problems on #26319 because if we try to reference a variable containing the font size preset the variable is not valid. So this PR includes functionality that if the font size preset is a number automatically adds the pixel unit e.g: "13" -> "13px". This is only done on existing font size presets added with add_theme_supports, for the font sizes passed via theme.json we assume they pass valid CSS properties.

When a font size preset is selected that is not in pixels the font size component shows the custom field as empty and correctly shows the selected preset. I guess the UI of the component should be improved to deal with other units besides pixes and to allow a free form input version where the user can type something like "clamp(12px, 5vw, 100px)" or change a little bit the advanced preset. But than can be done in a follow up PR. cc: @ItsJonQ as someone that is involved in UI work and worked with unit selection before.

Description

How has this been tested?

I tested 2019 and 2020 themes and verified the font size presets were still applied and font size selection still worked as expected.
I tested the global styles theme with the following typography settings:

			"typography": {
				"customLineHeight": true,
				"fontSizes": [
					{
					  "name": "x-small",
					  "slug": "x-small",
					  "size": "1rem"
					},
					{
					  "name": "small",
					  "slug": "small",
					  "size": "2em"
					},
					{
						"name": "normal",
						"slug": "normal",
						"size": "15px"
					},
					{
						"name": "Fluid",
						"slug": "fluid",
						"size": "clamp(12px, 5vw, 100px)"
					}
				]
			}

I verified all the presets worked as expected.

@github-actions
Copy link

github-actions bot commented Oct 26, 2020

Size Change: +70 B (0%)

Total Size: 1.21 MB

Filename Size Change
build/components/index.js 172 kB +70 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.78 kB 0 B
build/api-fetch/index.js 3.45 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 8.72 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 130 kB 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.98 kB 0 B
build/block-library/editor.css 8.98 kB 0 B
build/block-library/index.js 146 kB 0 B
build/block-library/style-rtl.css 7.83 kB 0 B
build/block-library/style.css 7.84 kB 0 B
build/block-library/theme-rtl.css 837 B 0 B
build/block-library/theme.css 838 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/style-rtl.css 15.2 kB 0 B
build/components/style.css 15.2 kB 0 B
build/compose/index.js 9.81 kB 0 B
build/core-data/index.js 12.3 kB 0 B
build/data-controls/index.js 772 B 0 B
build/data/index.js 8.77 kB 0 B
build/date/index.js 31.8 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.46 kB 0 B
build/edit-navigation/index.js 11.2 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.38 kB 0 B
build/edit-post/style.css 6.37 kB 0 B
build/edit-site/index.js 22.1 kB 0 B
build/edit-site/style-rtl.css 3.86 kB 0 B
build/edit-site/style.css 3.85 kB 0 B
build/edit-widgets/index.js 26.4 kB 0 B
build/edit-widgets/style-rtl.css 3.1 kB 0 B
build/edit-widgets/style.css 3.1 kB 0 B
build/editor/editor-styles-rtl.css 480 B 0 B
build/editor/editor-styles.css 482 B 0 B
build/editor/index.js 43.1 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 7.7 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 712 B 0 B
build/keyboard-shortcuts/index.js 2.52 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.11 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.34 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/reusable-blocks/index.js 3.06 kB 0 B
build/rich-text/index.js 13.2 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@ItsJonQ
Copy link

ItsJonQ commented Oct 28, 2020

@jorgefilipecosta Haiii! Wow, thanks for working on this. I took it for a spin locally. There are some UI issues when creating super mega sized fonts, in particular with the preview in the font-size picker:

Screen Shot 2020-10-28 at 1 12 44 PM

However, it's not an issue caused by your update.

In the meantime.. maybe we can add a CSS style rule for the FontSize picker to clamp() the font-size to a max value, so that it doesn't explode the menu 😂 .


✅ CSS Props Validation

Your update touches upon something that I've been attempting to streamline recently.. and that is Input based controls being able to handle any valid CSS property/value, not just units. Your calc example is a good one.

ItsJonQ/g2#66

(See the "CSS Prop Validation" section)

The current FontSizePicker is able to tap into just the number value of a given font-size. However, I don't think future Input fields have this luxury if we're expanding style values to more CSS properties via Global Styles and Full Site Editing.

@jorgefilipecosta
Copy link
Member Author

jorgefilipecosta commented Oct 28, 2020

Hi @ItsJonQ,

There are some UI issues when creating super mega sized fonts, in particular with the preview in the font-size picker:

Yes, I guess we can try a CSS min function to try to have a maximum value for font-size. But I guess these tries should not be done in this PR as we already had this problem e.g: if we set font size for a big value like 90px.

Your update touches upon something that I've been attempting to streamline recently.. and that is Input based controls being able to handle any valid CSS property/value, not just units. Your calc example is a good one.

Yah these updates basically allows anything and if we can parse the value we show it otherwise we show nothing in the custom value. It seems like a good first step towards allowing everything that CSS has to offer. Having nonpixel values and calcs editable by the user would be the next step and your components looks good. Would you be able to propose a PR after this one is merged with the new component for size shown here ItsJonQ/g2#66 that allows the user to control any value and select units etc?

Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

This looks great!
Tested with an FSE theme that already uses non-px values and it works as advertised. Also tested with FSE themes that use integer values and nothing breaks.

There are some UI issues when creating super mega sized fonts, in particular with the preview in the font-size picker

There's already another ticket for that, see #25862 👍

I think this one is good to merge! ❤️

@jorgefilipecosta jorgefilipecosta force-pushed the add/support-for-other-units-and-advanced-css-properties-on-font-size-presets branch from 16864c2 to 1f3514e Compare October 29, 2020 11:44
@jorgefilipecosta jorgefilipecosta merged commit 5d7ebd1 into master Oct 29, 2020
@jorgefilipecosta jorgefilipecosta deleted the add/support-for-other-units-and-advanced-css-properties-on-font-size-presets branch October 29, 2020 14:32
@github-actions github-actions bot added this to the Gutenberg 9.3 milestone Oct 29, 2020
@oandregal
Copy link
Member

Hi @jorgefilipecosta I'm experiencing some errors when using this code in the site editor. This is what I see:

2020-10-30-font-size-units

Although the UI doesn't reflect the change, the data can be saved to the CPT but with a wrong format:

{
  "global": {
    "styles": { 
      "typography": { 
        "fontSize": "60pxpx"
      }
    }
  }
}

I'm using https://github.com/nosolosw/global-styles-theme/ as a test demo.

@talldan talldan added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Feb 15, 2021
@talldan
Copy link
Contributor

talldan commented Feb 15, 2021

Labelling as an PR to backport for 5.6.2 as per #28738 (comment)

desrosj added a commit that referenced this pull request Feb 16, 2021
@tellthemachines tellthemachines mentioned this pull request Feb 17, 2021
7 tasks
tellthemachines added a commit that referenced this pull request Feb 17, 2021
* Backport #26475 for the 5.6 branch.

* Backport #28604.

* Revert "Backport #28604."

This reverts commit 32784b6.

* Backport #26583 to the 5.6 branch.

* Commit lock file updates.

* Committing lock file differences after updating `browserslist`.

* Pin the version of NodeJS in the Compressed Size workflow.

* Memoize getEntityRecords to prevent infinite re-renders (#26447)

* Fix issue where gallery block requests all attachments when empty (#28621)

* Return early from building attachments for galleries

* Improve code clarity

* Fix link control styles to prevent scrollbar (#27777)

* Update package-lock

* Update package-lock again

Co-authored-by: Jonathan Desrosiers <[email protected]>
Co-authored-by: Kai Hao <[email protected]>
Co-authored-by: Daniel Richards <[email protected]>
Co-authored-by: Aaron Robertshaw <[email protected]>
@noisysocks noisysocks removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants