Conversation
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
| boolean unsupportedBlockEditorSwitch = !mIsJetpackSsoEnabled && "gutenberg".equals(mSite.getWebEditor()); | ||
| return new GutenbergPropsBuilder( | ||
| enableMentions, | ||
| isUnsupportedBlockEditorEnabled, | ||
| unsupportedBlockEditorSwitch, |
There was a problem hiding this comment.
unsupportedBlockEditorSwitch is true if the site is a Jetpack site. Meaning that the site can switch the SSO settings. Just Jetpack sites have this capability.
Naming is not the best, but we are trying to make its usage generic, and not refer to SSO or Jetpack directly on gutenberg.
|
You can test the changes on this Pull Request by downloading the APK here. |
guarani
left a comment
There was a problem hiding this comment.
All three scenarios are working as expected 🎉. I've left some comments on the gutenberg and gutenberg-mobile PRs.
| private void setupToolbar() { | ||
| setTitle(getResources().getText(R.string.jetpack_security_setting_title)); | ||
|
|
||
| Toolbar toolbar = findViewById(org.wordpress.mobile.ReactNativeGutenbergBridge.R.id.toolbar); |
There was a problem hiding this comment.
Do we need the full package name here for the String?
There was a problem hiding this comment.
Ah no, thanks!
Fixed.
| } else if (preference == mJpUseTwoFactorPref) { | ||
| mJpUseTwoFactorPref.setChecked((Boolean) newValue); | ||
| mSiteSettings.enableJetpackSsoTwoFactor((Boolean) newValue); | ||
| } |
There was a problem hiding this comment.
Not a big deal, but I think it would help readability a bit if we extracted out the Boolean cast so that we don't have to do the cast repeatedly in the if-else block:
Boolean prefBool = (Boolean) newValue;
if (preference == mJpMonitorActivePref) {
mJpMonitorActivePref.setChecked(prefBool);
mSiteSettings.enableJetpackMonitor(prefBool);
} else if (preference == mJpMonitorEmailNotesPref) {
mJpMonitorEmailNotesPref.setChecked(prefBool);
mSiteSettings.enableJetpackMonitorEmailNotifications(prefBool);
} else if ...
If we had written this file in Kotlin, we could have also used a when statement to make this a bit more readable:
when (preference) {
mJpMonitorActivePref -> {
mJpMonitorActivePref.setChecked(prefBool);
mSiteSettings.enableJetpackMonitor(prefBool);
}
mJpMonitorEmailNotesPref -> {
mJpMonitorEmailNotesPref.setChecked(prefBool);
mSiteSettings.enableJetpackMonitorEmailNotifications(prefBool);
}
}
...
There was a problem hiding this comment.
You are right. Thanks! Fixed.
Thanks a lot for extra effort! |
Many thanks!!! |
…-to-enable-jetpack-sso
…kEditorSwitch to canEnableUnsupportedBlockEditor
Gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#2610
This PR implements changes from wordpress-mobile/gutenberg-mobile#2610 to present the Jetpack Security settings modally over Gutenberg, and refresh the editor capabilities to enable the Unsupported Block Editor.
NOTE: The action button text for UBE Enabled has been updated to
Edit using web editor.To test:
Jetpack connected sites (Self-Hosted and Atomic)
Jetpack - UBE Disabled.Open Jetpack security settings.Allow WordPress.com loginand press Done.Jetpack - UBE Enabledfrom the image.Edit using web editor.Self-Hosted sites.
Self-Hosted - Disabled.WPCom Simple sites.
On a WPCom Simple site:
Open a post with an unsupported block in it.
Press the (?) button on the missing block.
WPCom Simple - Enabled.Press the action button
Edit using web editor.I have considered adding unit tests where possible.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to
RELEASE-NOTES.txtif necessary.