-
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
Limit Classic editor access on Simple WP.com sites to existing Classic posts #15766
Limit Classic editor access on Simple WP.com sites to existing Classic posts #15766
Conversation
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
Build 41606
|
👋 We're freezing |
Remove the "Use block editor" setting from My Site → Settings on Simple WP.com sites. In light of the fact that the Classic editor is not available to users on on Simple WP.com sites (except via wp-admin), this PR brings the WordPress for iOS app closer to the web in terms of behavior.
Before this commit, when creating a new post the app would check the site's mobile editor preference (which is different to their web editor preference) and open the corresponding editor (Block editor or Classic editor). With this commit we now do not look at the preference when the user creates a new post and we will now always use the Block editor in this scenario).
Simple sites are WP.com-connected non-Atomic sites.
On Simple WP.com sites, we no longer show the option to switch to classic. This is part of the classic deprecation/removal project.
c4488f0
to
5f023ae
Compare
A follow-up to #15766 to add the dismiss alert call to the other Aztec test. Otherwise tests could break if `testBasicPostPublish` is not run before `testTextPostPublish`.
The [classic editor is deprecated](wordpress-mobile/WordPress-iOS#16008) and is [now being removed](wordpress-mobile/WordPress-iOS#15766) for Simple sites (except when editing existing posts with classic content or when using the iOS share extension). This means that on Simple sites, the site settings screen will no longer have a "Enable block editor" switch. WPiOS has a UI test suite, `EditorAztecTests`, that runs on a mock Simple site ("Tri-County Real Estate") and tests various features of the classic editor. Since the classic editor is going away for Simple sites, I think it makes sense to make `EditorAztecTests` run on an Atomic site instead of a Simple site. On WPiOS, this mock site is used for all other UI tests, so this has the side-effect of switching all WPiOS UI tests to run on a mock Atomic site instead of a mock Simple site. The same could be true for WPAndroid and the same considerations might apply.
Tested build 44258 🟢 (if you need help testing this, please let me know and I'd be more than happy to help!) |
Tested commit e9cd034 locally 🟢 |
5ae5a7d
to
82ecea5
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.
Verified test cases, and disabled Classic editor tests until we migrate them to testing existing classic posts
@illusaen I had to write some swift code, perhaps you can take a look so final review on this PR isn't just me approving my own code 🙏 . @mchowning any feedback on issues or plan for disabling tests until they are migrated to existing classic posts would be welcome here as well. 🙇 |
Definitely not a blocker, but I wonder whether a change along the lines of what I suggested on the WPAndroid PR would also make sense here. If you both (@illusaen and @cameronvoell ) think it makes sense, would you be willing to make that change for us @illusaen ? |
@@ -7,6 +7,10 @@ class EditorFlow { | |||
} | |||
} | |||
|
|||
static func goToMySiteScreen() -> MySiteScreen { |
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.
I do think it's a tad confusing that you changed the name from gotoMySiteScreen
to goToMySiteScreen
(see difference in capitalization in the first "T") but otherwise looks good!
…lization to match existing capitalization.
Updates look good, thanks @illusaen! |
The next step after notifying users that the classic editor is going away is to actually remove it, starting with Simple sites.
I think this change doesn't warrant inclusion in RELEASE-NOTES.txt since we already notified users about this change in 16.9, but if anyone thinks differently I'd love to hear. Perhaps we could include it in release notes but exclude it from the App Store version notes.
Addresses wordpress-mobile/gutenberg-mobile#3092 (for iOS)
| |
This PR changes two things on Simple sites only:
To reiterate, the above changes only affect Simple sites. They do not affect Atomic sites, Jetpack-connect self-hosted sites, regular self-hosted sites, etc. The classic editor will still be used in in the iOS share extension.
To test
Note: The toggle switch referred to below is the "Use block editor" toggle switch found in My Site → Settings. This is only available to site admins.
Regression notes (new ✨ )
I'm adding this section to try out @startuptester's proposal to improve code quality.
Potential unintended areas of impact (regressions)
This PR removes the classic editor on Simple sites. However, we're leaving it in place for scenarios where the block editor cannot be used instead. These scenarios are a) editing a classic post (i.e. a post that contains only classic content), and b) creating a post via the iOS share extension. We have to check to make sure these scenarios still work on Simple sites. We should also check to make sure all other site types (Atomic/Jetpack/self-hosted) are unaffected by this change.
What you did to manually test those/ what existing automated tests you relied on
I performed the following manual regression tests on a build made locally in Xcode from this branch. This is because one of the tests – the iOS sharing extension test – doesn't work on App Center builds (I get a "Sharing error" and I don't think it's related to the changes I've made in this PR, I've seen it before).
Regression test Simple sites
Regression test Atomic sites
Our UI tests will ensure that Atomic sites still show a toggle switch and that the switch works, i.e. it controls the default editor on mobile (feel free to run the
EditorAztecTests
andEditorGutenbergTests
UI tests locally in Xcode to check this). All that's left is to check that existing posts that contain classic content are opened in the classic editor:Regression test Jetpack-connected self-hosted sites
Regression test self-hosted sites
What automated tests you added or what prevented you from doing so
Our current UI test suite includes tests for both the classic editor (a.k.a. Aztec) and the block editor (a.k.a Gutenberg). These tests run on just one site type: Simple sites. Now that classic is no longer an option for new posts on Simple sites,
I updated the all UI tests to run on an Atomic site instead. This is was the easiest option to implement, but changes our entire UI test suite, so a less risky option could be to just update the classic editor tests to run on n Atomic site and leave the other UI tests to run on a Simple site as they currently do.We are disabling the Classic Editor tests until they can be migrated to executing on existing classic posts, instead of creating new ones. That work is tracked with this ticket here: #16218PR submission checklist
RELEASE-NOTES.txt
if necessary.