-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Editing Toolkit: Update to 2.8.5 #47442
Conversation
Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com D52773-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. |
Known issue with the php unit tests running in CI. They pass when run locally. |
I'll start on testing this. :) |
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.
@p-jackson Per Atomic testing instructions I have checked that the Editor loads with this ETK build, both with and without Gutenberg plugin. Success! ✅
@p-jackson @apeatling while testing this release I can see a visual issue with the Payments block, which I think could be related to #47245 (probably not caused by the PR, but revealed by it, if that makes sense!) — Update: actually it's probably not that PR, I just noticed that it depends on Automattic/jetpack#17702 which isn't merged 🤔 With this release patched there's a stray Without this release patched, it renders a Subscribe block, but I don't see the stray 0 character: Does this happen for anyone else? (Tested on a simple site on the Free plan) |
TEST FOR: Gutenboarding: plans grid requests data in the correct locale (#47227) Note: my launch flow went to /start/launch-site TEST FOR: Gutenboarding i18n: i18n for domain picker in the launch flow (#47178)** |
TEST FOR: Add block preview for Premium Content block (#47200) For Business Plan Site:
For Free Site: |
@autumnfjeld looks like the Update on my testing of the Premium Content block: it's working much like in Autumn's screenshots for me now, turns out I had an issue in my sandbox with my test site, so now testing on a fresh Free site it's looking a fair bit better. There is still the stray zero character rendering during the loading state, but it's appearing both with and without sandboxing this version change, so appears to be unrelated to the recent PRs in this ETK plugin release: |
Starting a review of our PRs in here this morning 👍 |
Just confirming that this issue is fixed by Automattic/jetpack#17696: I will look at merging that change on dotcom early today because it's a rough visual issue and I'd prefer not to wait on the Jetpack release. |
If anyone is curious, here's the issue for the failing PHP unit spec #47255. |
Created a diff to remove the outdated ET build D52840-code whenever this update is merged |
5b86c01
to
069b2c4
Compare
Rebased to include #47450 |
TEST FOR: use get_iso_639_locale() for coming soon page links (#47307) Did a very basic smoke test and just double-checked that the login button works. @roo2 Would love to have you smoke test this version bump as well if possible 🙂
TEST FOR: [eslint] Adds rule one-var and autofixes all related errors (#47310) ✅ Tested atomic and simple sites with the one editing toolkit block that was refactored (event countdown block) |
Deployed in r216900-wpcom |
Changes proposed in this Pull Request
Changes to Editing Toolkit since 2.8.4:
one-var
and autofixes all related errors ([eslint] Adds ruleone-var
and autofixes all related errors #47310)[ ] Persistent Launch Button: enable in all environments (Persistent Launch Button: enable in all environments #46817)(was just a code comment change)Testing instructions