Toolkit plugin: Always enable line-height in Gutenberg settings #44772
Conversation
noahtallen
added
[Type] Enhancement
[Goal] Gutenberg
matticbot
added
the
[Status] Needs Review
|
Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com D47821-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2 |
|
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
|
note that @fullofcaffeine noticed that this throws a fatal error on atomic: |
|
Just an FYI that I don't have the space to continue working on this. So if we still would like this solution to be in place, someone else will have to pick it up :) |
fullofcaffeine
force-pushed
the
add/line-height-fix-to-gutenberg
branch
from
ea2aa48
to
887c332
Compare
I found out the root of the issue and fixed it. PHP couldn't find the function and was (silently) failing with the following warning: Passing the function path with the correct parent namespace prefix solves the issue 👍🏻 I tested this version with a fresh ephemeral AT site and the setting appears and works fine:
I do see some errors when the Editing Toolkit Plugin is activated though and seems related to the premium blocks... see the console in the screenshot above. Here's a more detailed screenshot with the stacktrace:
While I think we can ignore these errors for the purposes of this PR, I'm pinging @kwight from Serenity, since I know they have been taking care of premium blocks lately. Kirk, do you have any insights about the error shown in the screenshot above? cc @noahtallen Also, @noahtallen, since I've modified the PR, could you be the reviewer now 🥺? Thanks! |
|
Ah, my bad about the namespace! :) I can also test this out in a bit and give a check, but I can't technically approve since I'm the author ;) |
PHP couldn't find the function and was (silently) failing with the following warning: ``` [14-Aug-2020 23:15:08 UTC] PHP Warning: call_user_func_array() expects parameter 1 to be a valid callback, function 'wpcom_gutenberg_enable_custom_line_height' not found or invalid function name in __wp__/wp-includes/class-wp-hook.php on line 289 ``` Passing the function path with the correct parent namespace prefix solves the issue.
noahtallen
force-pushed
the
add/line-height-fix-to-gutenberg
branch
from
9431d4a
to
6a48c6e
Compare
|
@noahtallen I can merge if you sanity-check this :) I also added a TODO item in the description to remind us to revert the changes we made to |
There was a problem hiding this comment.
After removing the existing code on wpcom, I verified that the setting stopped showing up. Then, I synced this build to my sandbox and verified that the setting showed up again.
I then transferred my test site to Atomic, installed a custom plugin. After making sure the line-height setting stopped showing up, I then uploaded the build of the toolkit plugin associated with this PR to my atomic site. I then verified the line-height setting started showing up on my Atomic site
matticbot
removed
the
[Status] Needs Review
|
WP.com revert diff: D49932-code |
Note: this patch was applied directly to wpcom in our wpcom gutenberg loader files in D47818-code. This additionally makes the patch available for atomic sites. Once deployed, we can remove the existing wpcom patch.
Changes proposed in this Pull Request
Always enable the custom line-height setting. This overrides the default Gutenberg setting, which is to rely on theme support. We need this so that existing users on our wpcom simple and atomic infrastructure do not loose functionality when upgrading to the latest Gutenberg version.
Testing instructions
TODO