Backport typography block supports for WP 6.1 (refresh) #3237
Conversation
|
@ironprogrammer Thank you! The style engine PR has now been merged; would you mind rebasing this PR? 😊 |
Replaced parent::set_up(); with parent::tear_down();.
New style engine does not emit spaces between attributes and values. See WordPress#3199.
ironprogrammer
force-pushed
the
add/backport-typography-block-supports-wp-6-1
branch
from
4184103 to
c16d09e
Compare
|
Hmmm, the test failures may be related to #3199, which removes spaces between generated CSS attributes and values. Looking into it. However, the tests in this PR are passing. |
@ironprogrammer I think in this case I'd lean toward updating those existing tests. As you mention. the style engine intentionally defaults to not prettifying output, so inline styles therefore don't have spaces. Given that there'll be a slight mismatch until all of the block supports are merged, I wonder if the |
Spaces are stripped between properties and values since WordPress#3199.
|
Thank for you for advancing this one!! 🙇 |
|
Currently testing and reviewing. When ready, will do the commit. |
consistency and readability
|
Prepping commit now. |
| $minimum_viewport_width = wp_get_typography_value_and_unit( | ||
| $minimum_viewport_width_raw, | ||
| array( | ||
| 'coerce_to' => $font_size_unit, | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Why is this needed twice? @ockham
There was a problem hiding this comment.
TBH I know next to nothing about this code -- I've only rebased it 😅
Speaking of rebase, maybe the duplicated code is a result of a slightly wonky rebase?
There was a problem hiding this comment.
Nope, it's the original Gutenberg PR WordPress/gutenberg#39529.
Hey @ramonjd is this duplicate code needed?
There was a problem hiding this comment.
Nope, it's the original Gutenberg PR WordPress/gutenberg#39529.
Ah, I hadn't noticed! Huh, weird.
(If it's blocking this from merging, I'd go out on a limb and say that we might remove it? 😅 )
There was a problem hiding this comment.
Hmm, it's been duplicated for a while: WordPress/gutenberg@eea3aeb#diff-2c54d3c8305590cf826f83511a9fd85d5b405470addd62edc3183ff9118177fbL302
There was a problem hiding this comment.
I'm going to remove it. If it's an issue, a Trac ticket can be opened to fix it during the beta cycle.
There was a problem hiding this comment.
Scratch that - my bad 🤦♀️ The 2nd time is for the minimum. Sorry for the confusion. Leaving it in (and smacking my forehead 🤣 ).
There was a problem hiding this comment.
Thanks for the ping, folks!
Yep, one is for $minimum_viewport_width and the other for $maximum_viewport_width.
If you think the vars could be named more clearly feel free to change. I can sync in Gutenberg today.
|
Committed via https://core.trac.wordpress.org/changeset/54260. Thank you everyone for contributions! |
Refresh of #3203.
From original PR:
Props @ramonjd.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.