-
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
Editor - Move Undo/Redo to the header #20930
Conversation
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr20930-cd4bc84 | |
Version | 22.8 | |
Bundle ID | org.wordpress.alpha | |
Commit | cd4bc84 | |
App Center Build | WPiOS - One-Offs #6299 |
|
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr20930-cd4bc84 | |
Version | 22.8 | |
Bundle ID | com.jetpack.alpha | |
Commit | cd4bc84 | |
App Center Build | jetpack-installable-builds #5324 |
bcc1f6a
to
1777067
Compare
a0772b4
to
9221a3e
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.
Hey @geriux, let me start by saying that I only reviewed the UI Tests related changes, so a review from other engineer is still needed.
I've left a few comments and suggestions, being the one around avoiding duplication, and the naming convention for assertion steps, the ones that require changes. [Don't simply trust the suggestions, I haven't tested them and some do impact others. 😬 ]
Thanks for adding the UI Tests! 🙇♂️
WordPress/UITestsFoundation/Screens/Editor/BlockEditorScreen.swift
Outdated
Show resolved
Hide resolved
WordPress/UITestsFoundation/Screens/Editor/BlockEditorScreen.swift
Outdated
Show resolved
Hide resolved
WordPress/UITestsFoundation/Screens/Editor/BlockEditorScreen.swift
Outdated
Show resolved
Hide resolved
WordPress/UITestsFoundation/Screens/Editor/BlockEditorScreen.swift
Outdated
Show resolved
Hide resolved
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.
Works as described! Left one q
WordPress/Classes/ViewRelated/Post/PostEditorNavigationBarManager.swift
Outdated
Show resolved
Hide resolved
…or both Aztec and Gutenberg, for Gutenberg it will include new undo/redo buttons, for Aztec it will keep the current ones. It also updates the "Generating preview" text that used to be shown in the header title, now it will show in a SVProgressHUD. Removes the reloadBlogTitleView code since it is no longer needed.
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!
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.
Thanks for considering the suggestions! 🙇
Everything looks good from a UI Tests perspective.
# Conflicts: # Gutenberg/version.rb # Podfile.lock # WordPress/UITestsFoundation/Screens/Editor/BlockEditorScreen.swift
…om the editor design changes
# Conflicts: # Podfile.lock
Related PRs:
This PR adds new undo/redo buttons in the navigation bar when the editor is opened, it also updates the existing menu icon.
Screenshots
To test
Bonus testing: follow these test cases.
Regression Notes
Potential unintended areas of impact
Post publishing
What I did to test those areas of impact (or what existing automated tests I relied on)
Rely on current tests and manual testing
What automated tests I added (or what prevented me from doing so)
Added new tests for testing the new undo/redo buttons in the navigation bar.
PR submission checklist:
RELEASE-NOTES.txt
if necessary.UI Changes testing checklist: