Inline font-sizes to fix themes that don't enqueue the preset classes #22668
Conversation
|
Note that I've prepared this on top of #22356 which should be merged first. |
| if ( | ||
| ! hasBlockSupport( blockName, FONT_SIZE_SUPPORT_KEY ) || | ||
| style?.typography?.fontSize || | ||
| ! fontSize |
There was a problem hiding this comment.
We shouldn't return early if the font size is not defined because it remounts the whole block when you select aa font size closing the inspector panels.
There was a problem hiding this comment.
Maybe because the typography panel is open by default. if we change it to be closed by default you'll notice the issue. I had the issue with the color panel.
There was a problem hiding this comment.
I still think we need to remove style?.typography?.fontSize || ! fontSize from here and only check the support here before merging.
There was a problem hiding this comment.
@youknowriad Do you know what style?.typography is? That's not a CSS prop?
|
I've set |
|
Size Change: +106 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
youknowriad
force-pushed
the
fix/themes-not-loading-palette
branch
from
99d6a8a
to
84f06b1
Compare
ellatrix
force-pushed
the
fix/themes-not-loading-palette
branch
from
84f06b1
to
bb0ae85
Compare
ellatrix
force-pushed
the
fix/themes-not-loading-font-sizes
branch
from
3b91591
to
ba0e7ca
Compare
ellatrix
force-pushed
the
fix/themes-not-loading-font-sizes
branch
from
ba0e7ca
to
0cb918b
Compare
|
We should address this before merging too #22668 (comment) |
| style?.typography?.fontSize | ||
| ).size; | ||
|
|
||
| props.wrapperProps = { |
There was a problem hiding this comment.
I don't like mutating arguments personally. Ideally, we'd have a lint rule for that as it can potentially create weird issues. (Not certain it's the case here though)
There was a problem hiding this comment.
It's spread a few lines below, so it should be ok
There was a problem hiding this comment.
It should be spread before making the change. The problem here is that you're changing the value of an object defined elsewhere (the caller) which means potentially unexpected side effects there.
There was a problem hiding this comment.
This breaks throwing:
Uncaught TypeError: can't define property "wrapperProps": Object is not extensible
Depends on #22356
I've set
fix/themes-not-loading-paletteas the base to easy review. This should be changed before merging the PR.This PR follows suit on what is proposed at #22356
It adds the font-size attribute as an inline style for the editor, regardless of the font value being a preset or a custom value. This is to fix themes that don't enqueue the preset classes. See context.
How to test