Block Support: Add font style and weight options with combined UI #26444

aaronrobertshaw · 2020-10-26T07:38:00Z

Description

After feedback and discussion around adding block support for font styles ( #26050 ), it was decided to add support for both font styles and font weights, then combine the UI controls for them.

Changes Included

Adds block support for font styles and font weights
Allows for preset font style/weight options and uses CSS variables
Updates navigation block to opt-in to font style support for easier testing

How has this been tested?

Manually tested using navigation block.

Test Instructions

Add a navigation block to a post and select it.
You should see a new "Appearance" option under the Inspector Controls > Typography section.
Select various font style/weight options and confirm the block updates visually as you'd expect.
Selecting a non-default option, inspect the block in dev tools and confirm that the block includes appropriate inline styles using var() and CSS variables relating to the selection made.
Save the post and view on the frontend.
The same font style and weight choices should be reflected on the frontend block's inline styles.

Screenshots

Types of changes

Enhancement

Next Steps

Update tests in class-block-supported-styles-test.php if needed after approach confirmed.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

aaronrobertshaw · 2020-10-26T07:51:30Z

@jasmussen Hope you don't mind I started a fresh PR for this. When you get the chance can you take a look at the new control combining font style and weight options?

I'm not sure if it needs separators between the groups of weight/style combos. Also, should it be 100% width or do you see it potentially fitting in alongside other options?

jasmussen · 2020-10-26T09:34:52Z

Nice. This is what I see:

The names are all-caps, due to this rule it appears:

It seems that rule was intended for menu subheadings, but is a little wide-reaching. Suffice to say, the fonts should not be all-caps, and should be the usual $gray-900 font color. I also think "Heavy Italic" is more readable than "Heavy - Italic", so no need for the dash.

Other than that, this is working well, and feels obvious. 👍 👍

aaronrobertshaw · 2020-10-27T02:01:14Z

I've updated the control's styling so the items:

are not all caps
use the $gray-900 color
do not include the hyphen in the label

Thanks for all the feedback. I think this is getting close now 🙂

aaronrobertshaw · 2020-10-30T07:40:45Z

I've rebased this to pull in the latest changes in approach to registering/applying block support. I think this just needs a review now to move forward.

jasmussen · 2020-10-30T07:59:04Z

This is cool:

I think you should sync up with @karmatosed per the opportunity outlined in #26572 (comment), it looks like you're both touching the same files, and the color, font size, all caps styles that are currently inherited, probably shouldn't be inherited at all.

aaronrobertshaw · 2020-10-30T08:03:38Z

I'm glad you're happy so far with the combined weight/style dropdown.

I think you should sync up with karmatosed per the opportunity outlined in #26572 (comment), it looks like you're both touching the same files, and the color, font size, all caps styles that are currently inherited, probably shouldn't be inherited at all.

Agreed. Those styles appear to be applied when they shouldn't be. I was hoping to be able to take a look at where they are actually used and tweak the CSS accordingly. I will probably do that via a separate PR though.

jasmussen · 2020-10-30T08:06:12Z

By the way, can you add a separator between the regular and italic weights? It's not super necessary but would be nice.

aaronrobertshaw · 2020-10-30T08:13:26Z

By the way, can you add a separator between the regular and italic weights? It's not super necessary but would be nice.

I initially thought it could do with a separator in there as well although I didn't find an obvious option to do that. Now, it's been brought up, I'll take another look at it next week.

My main issue was making sure that I wasn't adding another option that users could interact with or would impede accessibility. If anyone has any suggestions for doing this with a CustomSelectControl they'd be much appreciated 🙂

jasmussen · 2020-10-30T08:15:20Z

The separator should not block this PR.

apeatling

This is working great and I think we can merge and move ahead with this. Any more work could be done in followup PRs. 👍

aaronrobertshaw · 2020-11-04T01:00:38Z

I've rebased and resolved conflicts for this however it will need another round of rebasing and resolving conflicts after #26059 is merged.

Adds both font style and font weight block support options. The UI for both are combined into a single dropdown. The inline styles generated via this feature leverage CSS variables.

apeatling · 2020-11-06T00:22:20Z

React Native test failures here look unrelated to this PR, so moving ahead on merge as others have passed.

ceyhun · 2020-11-06T11:15:10Z

Hey there @apeatling 👋 How did you conclude that the React Native test failures are unrelated to this PR? Because they are indeed breaking React Native. You can see here that this PR is the first one breaking the React Native tests on master branch.

Do you think there is a way we can make it easier to spot that a PR will be breaking React Native in the future?

hypest · 2020-11-06T11:44:00Z

look unrelated to this PR

I'd like to denote that because of the high amount of code sharing between the web and mobile client (a good thing 🎉) it's not easy to tell if a change will affect the native client just by looking at the code, or seeing that there are no .native.js files touched. If native mobile tests fail but it's still not clear why, it's highly advised to collaborate with the native mobile folks to help figure out what's going on.

hypest · 2020-11-06T11:53:38Z

A fix PR is ~~in progress~~ merged #26768.

Fix PR: WordPress/gutenberg#26444 (comment)

jorgefilipecosta · 2020-11-06T15:02:35Z

lib/experimental-default-theme.json

@@ -161,6 +161,54 @@
 						"size": 42
 					}
 				],
+				"fontStyles": [


Previously @mtias referred to the intention of not having more presets for now, besides the one for the font family. Would you be able to confirm if that's still the case @mtias?
In the fontStyles and fontWeights I'm not sure if presets bring much value. I guess it may make sense that the themes define for a given font family the weights and styles that are available because depending on the font family one font-weight may be loaded or not, but that may be discussed as part of font loading API.

I guess for font-weight and style we can use normal inline styles without presets e.g: "font-weight: 900;".

jorgefilipecosta · 2020-11-06T15:04:49Z

packages/block-editor/src/components/font-appearance-control/index.js

+					name:
+						styleSlug === 'normal'
+							? weightName
+							: `${ weightName } ${ styleName }`,


I guess it may make sense to use sprintf and our translated function __ for i18n, depending on the locale it may make sense to change the order of weightName and styleName.

jorgefilipecosta · 2020-11-06T15:10:16Z

packages/block-library/src/navigation/block.json

@@ -50,6 +50,7 @@
 		"html": false,
 		"inserter": true,
 		"fontSize": true,
+		"__experimentalFontAppearance": true,


Font appearance is not something that is normally referred to. It was a UI decision to join them. But that decision may be changed in the future. For example, we may receive feedback to separate into weight and style using a different UI like buttons or to integrate additional typography settings into weight and style. I guess it would also make sense for a block to allow style selection without font-weight or the contrary. To me it feels like exposing the concept of font appearance as part of the block API is a restriction. What if we individually exposed fontWeight and fontStyle in block.json, the design may still join them in a single UI control. Depending on what is enabled, we would show some of the options.

jorgefilipecosta · 2020-11-06T15:12:23Z

packages/block-library/src/navigation/block.json

@@ -50,6 +50,7 @@
 		"html": false,
 		"inserter": true,
 		"fontSize": true,
+		"__experimentalFontAppearance": true,


Would it make sense to expand this to other blocks e.g: Site Title, Post Title, buttons?

jorgefilipecosta

This PR just adds the controls at the block level, to keep things consistent I will try to follow up with a PR that adds the UI at the global styles level.

apeatling · 2020-11-06T17:16:49Z

Hey there @apeatling 👋 How did you conclude that the React Native test failures are unrelated to this PR? Because they are indeed breaking React Native. You can see here that this PR is the first one breaking the React Native tests on master branch.

Sorry about this! We had fixed up a bunch of tests breaking, as well as rebased to fix some others that were breaking project wide. The breaking tests didn't seem related, at least from looking at the test result, but we may have been biased by all the previous breaking tests. What we can do in the future is ping someone specific to check first, if we know who the best person is.

bummytime · 2020-11-06T19:53:48Z

Thanks for the context @apeatling 🙇.

What we can do in the future is ping someone specific to check first, if we know who the best person is.

Going forward, if you encounter any React Native test failures feel free to ping @hypest or @Tug, and they can make sure the appropriate people are looped in.

jorgefilipecosta · 2020-11-09T17:07:28Z

packages/block-editor/src/components/font-appearance-control/index.js

+					// presetStyle are the actual typography styles that should be given to onChange.
+					presetStyle: {
+						fontStyle: `var:preset|font-style|${ styleSlug }`,
+						fontWeight: `var:preset|font-weight|${ weightSlug }`,


The other control components with presets that we have:
packages/block-editor/src/components/colors-gradients/control.js
packages/block-editor/src/components/font-family/index.js
packages/block-editor/src/components/font-sizes/font-size-picker.js

Don't handle the different ways a value should be stored, if a preset is used or not. They just receive a value, and a set of possible values (for UI purposes) and allow them to change the value. They are not aware of CSS vars etc..

jorgefilipecosta · 2020-11-10T21:41:24Z

lib/experimental-default-theme.json

+					},
+					{
+						"name": "Bold",
+						"slug": "700"


In the other presets we use different keys for slug and value. This approach is using the same keys for both. The idea of presets is we can change the value and the places using a given slug will automatically all use the new value. If we use the slug for the value the main objective of the preset is not accomplished as one can not change the value without changing the slug.
The inline style we store ends up containing the value anyway var:preset|font-weight|100. I guess removing presents from the fontWeight and fontStyle and just using values is the way to go.

In the other presets we use different keys for slug and value.

I can definitely update the presets to use both slug and value. It just seemed odd to me having both the slug and the value the same for the most part.

The inline style we store ends up containing the value anyway var:preset|font-weight|100. I guess removing presents from the fontWeight and fontStyle and just using values is the way to go.

The benefit I saw in using the presets that could be defined via the theme.json was that it also provided a means to customize the available styles or weights.

oandregal · 2020-11-11T12:33:28Z

@aaronrobertshaw @apeatling this PR landed without documentation. I've prepared a fix for it at #26891

aaronrobertshaw requested review from ajitbohra, ellatrix, Soean, spacedmonkey, talldan and TimothyBJacobs as code owners October 26, 2020 07:38

aaronrobertshaw mentioned this pull request Oct 26, 2020

Block Support: Add font styles support using CSS variables #26050

Closed

7 tasks

aaronrobertshaw mentioned this pull request Oct 26, 2020

Navigation Block: Add font style option to typography setttings #25234

Closed

aaronrobertshaw force-pushed the add/font-weight-style-block-support branch from ca28824 to 66805f0 Compare October 30, 2020 07:24

jasmussen mentioned this pull request Oct 30, 2020

Reduces padding on highlight for drop down #26572

Merged

apeatling self-requested a review November 3, 2020 22:45

apeatling approved these changes Nov 3, 2020

View reviewed changes

aaronrobertshaw force-pushed the add/font-weight-style-block-support branch from 66805f0 to 8633766 Compare November 4, 2020 00:58

aaronrobertshaw force-pushed the add/font-weight-style-block-support branch 2 times, most recently from 3d07a0a to 9066ca9 Compare November 5, 2020 23:34

aaronrobertshaw added 2 commits November 6, 2020 09:51

Add combined font style and weight block support

014d10b

Adds both font style and font weight block support options. The UI for both are combined into a single dropdown. The inline styles generated via this feature leverage CSS variables.

Update styling of font appearance list items

5eaf4fc

aaronrobertshaw force-pushed the add/font-weight-style-block-support branch from 9066ca9 to 5eaf4fc Compare November 5, 2020 23:54

apeatling merged commit 1d9c07a into WordPress:master Nov 6, 2020

github-actions bot added this to the Gutenberg 9.4 milestone Nov 6, 2020

hypest mentioned this pull request Nov 6, 2020

[RNMobile] Use Hermes bytecode #26732

Merged

6 tasks

hypest added a commit to wordpress-mobile/gutenberg-mobile that referenced this pull request Nov 6, 2020

Update GB hash to include broken native tests

150e2d3

Fix PR: WordPress/gutenberg#26444 (comment)

jorgefilipecosta reviewed Nov 6, 2020

View reviewed changes

jorgefilipecosta mentioned this pull request Nov 6, 2020

Add font weight picking mechanism #24978

Closed

jorgefilipecosta reviewed Nov 6, 2020

View reviewed changes

jorgefilipecosta reviewed Nov 9, 2020

View reviewed changes

apeatling assigned aaronrobertshaw Nov 9, 2020

jorgefilipecosta mentioned this pull request Nov 10, 2020

Add: Font appearance control on global styles #26868

Merged

jorgefilipecosta reviewed Nov 10, 2020

View reviewed changes

jorgefilipecosta mentioned this pull request Nov 10, 2020

Block Support: Separate opt in for font style and weight options #26844

Merged

6 tasks

This was referenced Nov 11, 2020

Update theme.json docs with new properties added #26891

Merged

Block Styles Breakdown #20331

Closed

aaronrobertshaw mentioned this pull request Dec 17, 2020

Allow borders on image, group, cover blocks #21540

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block Support: Add font style and weight options with combined UI #26444

Block Support: Add font style and weight options with combined UI #26444

aaronrobertshaw commented Oct 26, 2020

aaronrobertshaw commented Oct 26, 2020

jasmussen commented Oct 26, 2020

aaronrobertshaw commented Oct 27, 2020

aaronrobertshaw commented Oct 30, 2020

jasmussen commented Oct 30, 2020

aaronrobertshaw commented Oct 30, 2020

jasmussen commented Oct 30, 2020

aaronrobertshaw commented Oct 30, 2020

jasmussen commented Oct 30, 2020

apeatling left a comment

aaronrobertshaw commented Nov 4, 2020

apeatling commented Nov 6, 2020

ceyhun commented Nov 6, 2020

hypest commented Nov 6, 2020

hypest commented Nov 6, 2020 •

edited

Loading

jorgefilipecosta Nov 6, 2020

jorgefilipecosta Nov 6, 2020

jorgefilipecosta Nov 6, 2020 •

edited

Loading

jorgefilipecosta Nov 6, 2020

jorgefilipecosta left a comment

apeatling commented Nov 6, 2020 •

edited

Loading

bummytime commented Nov 6, 2020

jorgefilipecosta Nov 9, 2020

jorgefilipecosta Nov 10, 2020

aaronrobertshaw Nov 11, 2020

oandregal commented Nov 11, 2020

Block Support: Add font style and weight options with combined UI #26444

Block Support: Add font style and weight options with combined UI #26444

Conversation

aaronrobertshaw commented Oct 26, 2020

Description

Changes Included

How has this been tested?

Test Instructions

Screenshots

Types of changes

Next Steps

Checklist:

aaronrobertshaw commented Oct 26, 2020

jasmussen commented Oct 26, 2020

aaronrobertshaw commented Oct 27, 2020

aaronrobertshaw commented Oct 30, 2020

jasmussen commented Oct 30, 2020

aaronrobertshaw commented Oct 30, 2020

jasmussen commented Oct 30, 2020

aaronrobertshaw commented Oct 30, 2020

jasmussen commented Oct 30, 2020

apeatling left a comment

Choose a reason for hiding this comment

aaronrobertshaw commented Nov 4, 2020

apeatling commented Nov 6, 2020

ceyhun commented Nov 6, 2020

hypest commented Nov 6, 2020

hypest commented Nov 6, 2020 • edited Loading

jorgefilipecosta Nov 6, 2020

Choose a reason for hiding this comment

jorgefilipecosta Nov 6, 2020

Choose a reason for hiding this comment

jorgefilipecosta Nov 6, 2020 • edited Loading

Choose a reason for hiding this comment

jorgefilipecosta Nov 6, 2020

Choose a reason for hiding this comment

jorgefilipecosta left a comment

Choose a reason for hiding this comment

apeatling commented Nov 6, 2020 • edited Loading

bummytime commented Nov 6, 2020

jorgefilipecosta Nov 9, 2020

Choose a reason for hiding this comment

jorgefilipecosta Nov 10, 2020

Choose a reason for hiding this comment

aaronrobertshaw Nov 11, 2020

Choose a reason for hiding this comment

oandregal commented Nov 11, 2020

hypest commented Nov 6, 2020 •

edited

Loading

jorgefilipecosta Nov 6, 2020 •

edited

Loading

apeatling commented Nov 6, 2020 •

edited

Loading