-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Block Supports: Allow skipping serialization of typography attributes #30880
Block Supports: Allow skipping serialization of typography attributes #30880
Conversation
Size Change: +40 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
481fd01
to
6005d8c
Compare
3156515
to
0ce473d
Compare
Rebased. |
Not sure if this is the right issue, but in the GIF above, is it going to be possible for themes to set line-height classes instead of a number? Many companies who develop WP themes prefer to use style guidelines and not allow the client to make random choices. Having options in GB for line height would be preferable rather than setting a style attribute and letting user select any number. theme.json example "line-heights": [
{
"name": "Loose",
"slug": "loose",
"value": 2
},
{
"name": "Normal",
"slug": "normal",
"value": 1.5
},
{
"name": "Tight",
"slug": "tight",
"value": 1.25
},
{
"name": "Compact",
"slug": "compact",
"value": 1
}
] |
I think that makes sense (and we have precedent for something like that with font size classes), but would you mind filing a dedicated issue for this, @coreyworrell? This PR modifies the wider set of typography features (which just happen to include line height) so that block authors can define how to persist related block style attributes as part of the block markup; adding line-height classes is a rather separate concern and should be discussed (and implemented) separately 🙂 |
@coreyworrell A good place to surface that convo would be this issue #27100 |
I've tested this with dynamic (site title) and static (paragraph) blocks using text decoration, transform, and line-height. It looks like text decoration and transform don't work in the editor (they still are serialized). |
I think I've addressed all feedback -- should be ready for another look 🙂 |
65238e8
to
44be700
Compare
44be700
to
bed5ad8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed that CI hadn't run for this, so rebased it to push and force a re-run. Also pushed bed5ad8 to add some simplifications to the logic.
Tested with a static block (paragraph) and a dynamic one (site title). Everything is working fine for me. We can land after the tests pass.
911d8cf
to
ca1c29b
Compare
Failed to re-run the CI actions, so I've tried rebase this from |
I'm a bit puzzled by some of the failures, especially the PHP unit tests: I can't repro locally and I don't understand why the changes here would break those tests in any scenario. |
ca1c29b
to
330fbe5
Compare
Description
Part of #28913. Counterpart to #30879, #30035, or #29142+#29253.
Find affected blocks by grepping any of the following typography related feature flags in
block.json
files:__experimentalFontFamily
__experimentalFontStyle
__experimentalFontWeight
lineHeight
__experimentalTextDecoration
__experimentalTextTransform
Note that
fontSize
is handled separately (see #30879). (The main difference to this PR is that in addition tostyle
attributes,fontSize
also addsclassName
s.)Since
lineHeight
is no longer experimental, it's fairly widespread; it's e.g. present in the Heading and Paragraph blocks.Examples of the other features include the Navigation block. (A number of other blocks has the
__experimentalFontFamily
flag set totrue
, but this feature doesn't seem to be visible in the TT1 Blocks theme.)How has this been tested?
(Based on #30879.)
"__experimentalSkipTypographySerialization": true
as a new key-value pair within thesupports
object for any block that also has one of the aforementioned typography flags set. In the following, we'll be using the Heading block.Here's a patch for your convenience:
The expected result is that:
typography:{ lineHeight: ... } }
attribute, e.g.<!-- wp:heading {"style":{"typography":{"lineHeight":"1.7"}}} -->
-- but not as inlined markup (HTMLstyle="line-height:1.7"
attribute on the<h2 />
tag).style="font-size:...
attribute on the<h2 />
tag).Screencasts
Before
After
Note
For a discussion of the nomenclature, syntax, and granularity of the typrography-related
__experimental...
flag(s), see #30879 (both PR desc and comments), and #28913 (comment) (and below).