Set font size slider to 1 as default, em units

[notnownikki]

Description

• Allows units to be displayed next to a slider control
• Sets the units used for font size in paragraph blocks to 'em'
• Sets the default value to 1
• If the font size has been set to a value over 8, assume it was set using the old pixel based slider and divide by 16, to get the value in em units

How Has This Been Tested?

Started a new post, typed some text, used the slider to resize it, checking the default value of 1 was set first.

Opened saved posts, checked that it did not cause warnings for blocks where the size had not been set.

For posts where the size had been set, check that the size was correctly converted to em

Types of changes

New feature, potential change to existing posts that have pixel sizes set.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows has proper inline documentation.

[notnownikki]

Addresses #2716

[notnownikki]

Requesting feedback on how the units are displayed next to the slider.

Currently looks like this:

[codecov]

Codecov Report

Merging #3090 into master will decrease coverage by 1.24%.
The diff coverage is 72.22%.

@@            Coverage Diff            @@
##           master   #3090      +/-   ##
=========================================
- Coverage   32.25%     31%   -1.25%     
=========================================
  Files         216     222       +6     
  Lines        6148    6818     +670     
  Branches     1080    1265     +185     
=========================================
+ Hits         1983    2114     +131     
- Misses       3515    3907     +392     
- Partials      650     797     +147

Impacted Files	Coverage Δ
blocks/inspector-controls/range-control/index.js	75% <ø> (ø)	⬆️
blocks/api/source.js	96.55% <100%> (+1.09%)	⬆️
blocks/library/paragraph/index.js	40% <54.54%> (+6.66%)	⬆️
editor/utils/dom.js	21.91% <0%> (-17.62%)	⬇️
editor/sidebar/post-sticky/index.js	40% <0%> (-14.55%)	⬇️
blocks/library/code/index.js	50% <0%> (-7.15%)	⬇️
editor/block-settings-menu/block-mode-toggle.js	33.33% <0%> (-4.17%)	⬇️
blocks/editable/format-toolbar/index.js	5.19% <0%> (-1.19%)	⬇️
blocks/library/freeform/old-editor.js	1.49% <0%> (-0.03%)	⬇️
editor/selectors.js	95.63% <0%> (-0.02%)	⬇️
... and 16 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3316c3c...228620c. Read the comment docs.

[ellatrix]

What happens if the theme sets a font size on the paragraph in a stylesheet? Then it would base the font size on the parent node, right? https://developer.mozilla.org/en-US/docs/Web/CSS/font-size#Ems

[notnownikki]

Yeah, that's why I thought that em was the right choice. The theme should be setting the base font size, and the font size slider is there to make it bigger or smaller. Or am I misunderstanding the issue?

[ellatrix]

Yes, em works in that way, but ideally it would be relative to the normal font size of that element, rather than the font size of the parent element. See #2610 (comment). Many themes target the paragraph directly, e.g. .content p { font-size: 16px }. In this case, I think it would break?

[notnownikki]

I'm not sure... from what I can see, it would be fine. Here's how we currently set the font size on paragraphs in the editor:

.editor-visual-editor p {
    font-family: "Noto Serif", serif;
    font-size: 16px;
    line-height: 1.8;
}

Applying a font-size in the paragraph's style attribute seems to accept 16px as the same as 1em, and it works as I expected. (I compared an unstyled paragraph to one with font-size set to 1em, they were identical) Perhaps I'm missing some detail though?

[notnownikki]

I was missing a detail. It was coincidence it worked. Thanks for putting me on the right track here.

[jorgefilipecosta]

Hi @notnownikki, nice work here, this generally works as advertised. During my tests I found some regressions that should be corrected.
After changing the font size in a paragraph, I'm not able to add new content it automatically gets deleted:

After saving a post wih a custom font-size when reloading, we are not recognizing its marktup and we show an error:
.

I think both of this problems are related with the addition of the span element. The span element was added to make the font-size relative to the default size of a paragraph.
Would it be enougth to make the font-size relative to the parent of the paragraph (in most cases entry-content) ? Many themes set the font size of content and than use relative units in children html elements (headers, paragraph etc...). This way we would avoid a wrapper span and the additional effort required to make it work as expected.
I think the plan is to remove inline styles, so what about instead of directly setting the em units in paragraph the slider would show descritive sizes like: xx-small, x-small, small..., xx-large. This sizes would not map to the corresponding absolute size keywork but to a class e.g "xxSmall" that would by default in gutenberg styles set a relative size e.g: "font-size: 0.6 em;" Theme authors would easily be able to overwrite and use their own sizes if needed. c.c: @mtias, @iseulde, @jasmussen.
Given that you already have work on font-size choosing I would love to know you thougths on this alternative approach that also adresses the inline style problem.

[notnownikki]

As I remember, there were discussions that we shouldn't take this approach, and let themes register font sizes that the slider uses.

[mtias]

Yes, we need to revise this a bit more generally, and probably go with a stepped slider that covers classes like is-text-small, is-text-default, is-text-larger, that themes can customizer at will and handle responsively.

The current implementation in master would either be a fallback or a "custom" option.

[jorgefilipecosta]

This approach where we use a slider that just set classes is-text-small, is-text-default, is-text-larger that themes are able to change looks good to me. We provide styles for this classes that should be interpreted as a fallback when themes are not setting them.

[notnownikki]

Yes, and that allows us to just apply a class to the p and avoids inline styles, and extra spans, which also means no existing blocks have to be migrated.

[jorgefilipecosta]

@notnownikki, I can try to create a PR with that approach so we can look at how the approach looks, would that be ok? Or given that you already started you would prefer to continue the work on this and update this PR / create a new one?

[youknowriad]

From the comments above, this PR should be closed, am I right?

[notnownikki]

Oh, yes, sorry. I missed the last message on here. This should be closed in favor of theme supplied sizes.