-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[Gutenberg] Disable editor Jetpack-powered features based on Jetpack Features Removal phases #19691
Conversation
You can test the changes in WordPress from this Pull Request by:
|
You can test the changes in Jetpack from this Pull Request by:
|
e47c3d8
to
e6479b8
Compare
e6479b8
to
677e26c
Compare
Nice work @fluiddot. I've added my testing experiences -- they are all green, with the exception of the "Get Support" section appearing in the WordPress app when Jetpack-powered features are disabled. I'm curious if this matches your testing experiences. 1 - Editor Jetpack-powered features ENABLED in WordPress app:
2 - Editor Jetpack-powered features DISABLED in WordPress app:
3- Editor Jetpack-powered features ENABLED in Jetpack app:
|
Thank you @derekblank for testing the changes 🙇 ! I went through the testing instructions again to check the steps that failed for you:
Good catch! It also failed when I tested the installable builds, however, succeeds when testing locally. This made me realize that I completely forgot to update the bundle in the Gutenberg Mobile PR 🤦, which basically means that the Gutenberg changes aren't included in the build. I'll generate a new RN bundle and create new installable builds.
It's interesting that this step succeeded in your case. Following the above comment, due to the fact that the RN bundle wasn't updated, the Jetpack blocks shouldn't have been displayed as unsupported since this was a new change 🤔. |
Bundle updated in e461d84. @derekblank let me know if could perform another review once the installable builds are generated, thanks! |
@@ -1191,6 +1191,30 @@ extension GutenbergViewController: GutenbergBridgeDataSource { | |||
func gutenbergCapabilities() -> [Capabilities: Bool] { | |||
let isFreeWPCom = post.blog.isHostedAtWPcom && !post.blog.hasPaidPlan | |||
let isWPComSite = post.blog.isHostedAtWPcom || post.blog.isAtomic() | |||
|
|||
// Disable Jetpack-powered editor features in WordPress app based on Features Removal coordination | |||
if JetpackFeaturesRemovalCoordinator.generalPhase() == .four { |
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.
There are two phases where Jetpack features should be removed:
four
newUsers
I suggest adding a helper to identify whether features should be removed or not. Something along the lines of shouldRemoveJetpackFeatures
. This helper should return true if the general phase is four
or newUsers
. What do you think?
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.
Ah, true, thanks @hassaanelgarem for the call-up about the two phases 🙇 .
I suggest adding a helper to identify whether features should be removed or not. Something along the lines of
shouldRemoveJetpackFeatures
. This helper should return true if the general phase isfour
ornewUsers
. What do you think?
Good idea, I'll work on adding a function to determine when the Jetpack features should be removed. Similarly, I'll replicate this suggestion in Android 👍 .
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.
Suggestion applied in fb13609.
Actually, this particular case makes sense -- I was running both local and installable builds when testing. When running locally I was pointing at the Gutenberg Mobile branch, so I was able to pick up the Gutenberg Mobile changes, not realizing they could have different results. However, now I am wondering why the other step, |
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.
After retesting the installable builds with the updated bundle, I was able to succeed on every step of the testing criteria. 🟢
LGTM! 🚀
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.
LGTM! 🚀
a38f060
to
74d38a6
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.
All tests passed correctly ✅
onlyCoreBlocks
capability is enabled gutenberg-mobile#5293To test
The following features in the editor will be disabled when reaching the phase of disabling Jetpack-powered features:
1 - Editor Jetpack-powered features ENABLED in WordPress app
Preparation
Jetpack Features Removal Phase Four
andJetpack Features Removal Phase For New Users
flags off.Check Jetpack blocks
Contact Info
,Story
,Layout Grid
blocks).Check @-mentions and X-posts features
+
character and observe that thex-post
sheet is displayed.NOTE:
x-post
feature only works in sites with the O2 plugin enabled like P2-based sites. In order to test this, please use a P2-based site.@
character and observe that the@-mentions
sheet is displayed.@
format button is displayed.Check the help's support section
Support
, tap on it.Check Reusable blocks
Check Unsupported Block Editor (UBE)
2 - Editor Jetpack-powered features DISABLED in WordPress app
Preparation
Jetpack Features Removal Phase Four
flag on.NOTE: It's recommended that the following test cases are also checked by turning the
Jetpack Features Removal Phase For New Users
flag on andJetpack Features Removal Phase Four
flag off in Debug settings.Check Jetpack blocks
Check @-mentions and X-posts features
+
character and observe that thex-post
sheet is NOT displayed.NOTE:
x-post
feature only works in sites with the O2 plugin enabled like P2-based sites. In order to test this, please use a P2-based site.@
character and observe that the@-mentions
sheet is NOT displayed.@
format button is NOT displayed.Check the help's support section
Support
, tap on it.Check Reusable blocks
Check Unsupported Block Editor (UBE)
3- Editor Jetpack-powered features ENABLED in Jetpack app
Jetpack Features Removal Phase Four
flag on. This flag should be omitted by the Jetpack app.Regression Notes
Potential unintended areas of impact
Only the editor should be affected.
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A
What automated tests I added (or what prevented me from doing so)
N/A
PR submission checklist:
RELEASE-NOTES.txt
if necessary.