Skip to content

Offline Mode: Remove secondary actions from the editor - #22913

Merged
kean merged 1 commit into
feature/offline-mode-milesone-2from
task/remove-secondary-actions
Apr 5, 2024
Merged

Offline Mode: Remove secondary actions from the editor#22913
kean merged 1 commit into
feature/offline-mode-milesone-2from
task/remove-secondary-actions

Conversation

@kean

@kean kean commented Mar 27, 2024

Copy link
Copy Markdown
Contributor
  • Update the primary action for drafts to always be “Publish” (to match web and Android)
  • Remove secondary actions (“Save as Draft”) – you tap back now and it’s offline-first

This eliminates the scenario where you tap "Update" and need to force the sync engine to save the changes synchronously, so we no longer have to worry about it.

To test:

  • Verify that both new and existing drafts have a primary action "Publish" and no secondary action

The secondary action used to be here:

Screenshot 2024-03-27 at 9 14 24 AM

Regression Notes

  1. Potential unintended areas of impact: Post Editor
  2. What I did to test those areas of impact (or what existing automated tests I relied on): n/a
  3. What automated tests I added (or what prevented me from doing so): n/a

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • 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.txt if necessary.

Testing checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@wpmobilebot

wpmobilebot commented Mar 27, 2024

Copy link
Copy Markdown
Contributor
WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr22913-be54822
Version24.5
Bundle IDorg.wordpress.alpha
Commitbe54822
App Center BuildWPiOS - One-Offs #9340
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot

wpmobilebot commented Mar 27, 2024

Copy link
Copy Markdown
Contributor
Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr22913-be54822
Version24.5
Bundle IDcom.jetpack.alpha
Commitbe54822
App Center Buildjetpack-installable-builds #8384
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@kean
kean force-pushed the task/remove-secondary-actions branch from 38afcea to 2fe021f Compare March 27, 2024 15:34
@kean
kean marked this pull request as draft March 27, 2024 22:01
@kean

kean commented Mar 27, 2024

Copy link
Copy Markdown
Contributor Author

Found an issue where sometimes "Publish" for existing posts does not seem to be available.
Update: fixed.

import XCTest
@testable import WordPress

class PostEditorStateTests: XCTestCase {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests were covering what would be typically considered "configuration": the logic in the unit tests is as complex (or simple) as in the code itself, so I decided to remove these.

@kean
kean marked this pull request as ready for review March 28, 2024 01:17
@kean
kean force-pushed the task/remove-secondary-actions branch from 5be822c to 0c6ebaa Compare March 28, 2024 15:52
@kean kean mentioned this pull request Mar 29, 2024
15 tasks
@kean
kean force-pushed the task/sync-engine branch from bd49308 to 730f856 Compare April 1, 2024 17:49
@kean
kean requested a review from a team as a code owner April 1, 2024 17:49
@kean
kean force-pushed the task/sync-engine branch 2 times, most recently from f64287b to f41c02e Compare April 1, 2024 21:36
@kean
kean force-pushed the task/remove-secondary-actions branch from 0c6ebaa to 3f0ffd6 Compare April 1, 2024 21:40
@kean
kean force-pushed the task/sync-engine branch from f41c02e to 15f930e Compare April 1, 2024 22:21
@kean
kean force-pushed the task/remove-secondary-actions branch from 3f0ffd6 to be54822 Compare April 1, 2024 22:22

@momo-ozawa momo-ozawa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as described, but I found an issue accessing drafts as a contributor

fatalError("Not implemented (kahu-offline-mode)")
case .save, .continueFromHomepageEditing:
assertionFailure("No longer used and supported")
case .save, .saveAsDraft, .continueFromHomepageEditing:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I open a draft as a contributor, the app crashes at this assertion failure due to the post editor action being .save

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are a few issues with the contributor role, which I think is related to the sync engine changes:

  • If I create a new draft as a contributor, the post list cell activity indicator never disappears. When I re-open the editor, then tap the back button without making any changes, the app crashes because it fails the isOriginal() assertion for isNewDraft
  • As mentioned above, if I open a draft that's been submitted for review, the app crashes because the post editor action is .save (doesn't matter if you're logged in as a contributor or an admin)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll cover the contributor role and pending posts in a separate PR and according to the amend to the spec 👍
Working on it right now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the following PR: #22949

Base automatically changed from task/sync-engine to feature/offline-mode-milesone-2 April 3, 2024 00:14
@kean
kean requested a review from momo-ozawa April 3, 2024 21:57
@kean
kean merged commit 6ea1979 into feature/offline-mode-milesone-2 Apr 5, 2024
@kean
kean deleted the task/remove-secondary-actions branch April 5, 2024 12:16
@momo-ozawa momo-ozawa modified the milestones: 24.7, Pending, 24.8 Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants