Skip to content

Offline Mode: Remove secondary actions from the editor #22913

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.

Sign up for GitHub

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

Merged
kean merged 1 commit into from
Apr 5, 2024
Merged

Offline Mode: Remove secondary actions from the editor #22913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 5 additions & 14 deletions WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ extension PublishingEditor {
performEditorAction(action, analyticsStat: postEditorStateContext.publishActionAnalyticsStat)
}

/// - note: deprecated (kahu-offline-mode)
func handleSecondaryActionButtonTap() {
guard let action = self.postEditorStateContext.secondaryPublishButtonAction else {
// If the user tapped on the secondary publish action button, it means we should have a secondary publish action.
Expand All @@ -110,11 +111,7 @@ extension PublishingEditor {
let secondaryStat = self.postEditorStateContext.secondaryPublishActionAnalyticsStat

let publishPostClosure = { [unowned self] in
guard RemoteFeatureFlag.syncPublishing.enabled() else {
publishPost(action: action, dismissWhenDone: action.dismissesEditor, analyticsStat: secondaryStat)
return
}
performEditorAction(action, analyticsStat: secondaryStat)
publishPost(action: action, dismissWhenDone: action.dismissesEditor, analyticsStat: secondaryStat)
}

if presentedViewController != nil {
Expand All @@ -134,20 +131,14 @@ extension PublishingEditor {
switch action {
case .schedule, .publish:
showPrepublishingSheet(for: action, analyticsStat: analyticsStat)
case .saveAsDraft:
performSaveDraftAction()
case .update:
if post.original().status == .draft {
performSaveDraftAction()
} else {
performUpdatePostAction()
}
performUpdatePostAction()
case .submitForReview:
// TODO: Show Prepublishing VC
fatalError("Not implemented (kahu-offline-mode)")
case .save, .continueFromHomepageEditing:
assertionFailure("No longer used and supported")
case .save, .saveAsDraft, .continueFromHomepageEditing:

@momo-ozawa momo-ozawa Apr 2, 2024

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

@momo-ozawa momo-ozawa Apr 2, 2024

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)

@kean kean Apr 3, 2024

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.

@kean kean Apr 3, 2024

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

assertionFailure("No longer used and supported")
break
}
}

Expand Down
6 changes: 5 additions & 1 deletion WordPress/Classes/ViewRelated/Post/PostEditor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ extension PostEditor {
}

var editorHasChanges: Bool {
return post.hasUnsavedChanges()
if RemoteFeatureFlag.syncPublishing.enabled() {
return !post.changes.isEmpty
} else {
return post.hasUnsavedChanges()
}
}

func editorContentWasUpdated() {
Expand Down
37 changes: 28 additions & 9 deletions WordPress/Classes/ViewRelated/Post/PostEditorState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import WordPressShared
public enum PostEditorAction {
/// - note: Deprecated (kahu-offline-mode)
case save
/// - note: Deprecated (kahu-offline-mode)
case saveAsDraft
case schedule
case publish
Expand Down Expand Up @@ -114,6 +115,7 @@ public enum PostEditorAction {
}
}

/// - note: deprecated (kahu-offline-mode)
fileprivate var secondaryPublishAction: PostEditorAction? {
switch self {
case .publish:
Expand Down Expand Up @@ -260,7 +262,11 @@ public class PostEditorStateContext {
case .draft where originalPostStatus == nil:
return publishAction(userCanPublish: userCanPublish)
case .draft:
return .update
if RemoteFeatureFlag.syncPublishing.enabled() {
return publishAction(userCanPublish: userCanPublish)
} else {
return .update
}
case .pending:
return .save
case .publish where isNewOrDraft(originalPostStatus):
Expand Down Expand Up @@ -358,8 +364,7 @@ public class PostEditorStateContext {
return action.publishActionAnalyticsStat
}

/// Returns whether the secondary publish button should be displayed, or not
///
/// - note: deprecated (kahu-offline-mode)
var isSecondaryPublishButtonShown: Bool {
guard hasContent else {
return false
Expand All @@ -376,11 +381,14 @@ public class PostEditorStateContext {
return false
}

guard !RemoteFeatureFlag.syncPublishing.enabled() else {
return false
}

return action.secondaryPublishAction != nil
}

/// Returns the secondary publish action
///
/// - note: deprecated (kahu-offline-mode)
var secondaryPublishButtonAction: PostEditorAction? {
guard isSecondaryPublishButtonShown else {
return nil
Expand All @@ -389,8 +397,7 @@ public class PostEditorStateContext {
return action.secondaryPublishAction
}

/// Returns the secondary publish button text
///
/// - note: deprecated (kahu-offline-mode)
var secondaryPublishButtonText: String? {
guard isSecondaryPublishButtonShown else {
return nil
Expand All @@ -399,7 +406,7 @@ public class PostEditorStateContext {
return action.secondaryPublishAction?.publishActionLabel
}

/// Returns the WPAnalyticsStat enum to be tracked when this post is published with the secondary action
/// - note: deprecated (kahu-offline-mode)
var secondaryPublishActionAnalyticsStat: WPAnalyticsStat? {
guard isSecondaryPublishButtonShown else {
return nil
Expand All @@ -411,7 +418,19 @@ public class PostEditorStateContext {
/// Indicates whether the Publish Action should be allowed, or not
///
private func updatePublishActionAllowed() {
publishActionAllowed = hasContent && hasChanges && !isBeingPublished && (action.isAsync || !isUploadingMedia)
if RemoteFeatureFlag.syncPublishing.enabled() {
switch action {
case .schedule, .publish, .submitForReview:
publishActionAllowed = hasContent
case .update:
publishActionAllowed = hasContent && hasChanges && !isBeingPublished
case .save, .saveAsDraft, .continueFromHomepageEditing:
assertionFailure("No longer used")
break
}
} else {
publishActionAllowed = hasContent && hasChanges && !isBeingPublished && (action.isAsync || !isUploadingMedia)
}
}
}

Expand Down
4 changes: 0 additions & 4 deletions WordPress/WordPress.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2445,7 +2445,6 @@
937250EE267A492D0086075F /* StatsPeriodStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 937250ED267A492D0086075F /* StatsPeriodStoreTests.swift */; };
937D9A0F19F83812007B9D5F /* WordPress-22-23.xcmappingmodel in Sources */ = {isa = PBXBuildFile; fileRef = 937D9A0E19F83812007B9D5F /* WordPress-22-23.xcmappingmodel */; };
937D9A1119F838C2007B9D5F /* AccountToAccount22to23.swift in Sources */ = {isa = PBXBuildFile; fileRef = 937D9A1019F838C2007B9D5F /* AccountToAccount22to23.swift */; };
937E3AB61E3EBE1600CDA01A /* PostEditorStateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 937E3AB51E3EBE1600CDA01A /* PostEditorStateTests.swift */; };
937F3E321AD6FDA7006BA498 /* WPAnalyticsTrackerAutomatticTracks.m in Sources */ = {isa = PBXBuildFile; fileRef = 937F3E311AD6FDA7006BA498 /* WPAnalyticsTrackerAutomatticTracks.m */; };
938466B92683CA0E00A538DC /* ReferrerDetailsViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 938466B82683CA0E00A538DC /* ReferrerDetailsViewModelTests.swift */; };
938CF3DC1EF1BE6800AF838E /* CocoaLumberjack.swift in Sources */ = {isa = PBXBuildFile; fileRef = 938CF3DB1EF1BE6800AF838E /* CocoaLumberjack.swift */; };
Expand Down Expand Up @@ -7827,7 +7826,6 @@
937D9A0C19F83744007B9D5F /* WordPress 22.xcdatamodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcdatamodel; path = "WordPress 22.xcdatamodel"; sourceTree = "<group>"; };
937D9A0E19F83812007B9D5F /* WordPress-22-23.xcmappingmodel */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcmappingmodel; path = "WordPress-22-23.xcmappingmodel"; sourceTree = "<group>"; };
937D9A1019F838C2007B9D5F /* AccountToAccount22to23.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AccountToAccount22to23.swift; sourceTree = "<group>"; };
937E3AB51E3EBE1600CDA01A /* PostEditorStateTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PostEditorStateTests.swift; sourceTree = "<group>"; };
937F3E301AD6FDA7006BA498 /* WPAnalyticsTrackerAutomatticTracks.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; lineEnding = 0; path = WPAnalyticsTrackerAutomatticTracks.h; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.objcpp; };
937F3E311AD6FDA7006BA498 /* WPAnalyticsTrackerAutomatticTracks.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = WPAnalyticsTrackerAutomatticTracks.m; sourceTree = "<group>"; };
938466B82683CA0E00A538DC /* ReferrerDetailsViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReferrerDetailsViewModelTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -14375,7 +14373,6 @@
57E3C98223835A57004741DB /* Controllers */,
57DF04BF2314895E00CC93D6 /* Views */,
5749984522FA0EB900CE86ED /* Utils */,
937E3AB51E3EBE1600CDA01A /* PostEditorStateTests.swift */,
E10F3DA01E5C2CE0008FAADA /* PostListFilterTests.swift */,
57D6C83D22945A10003DDC7E /* PostCompactCellTests.swift */,
575E126222973EBB0041B3EB /* PostCompactCellGhostableTests.swift */,
Expand Down Expand Up @@ -23898,7 +23895,6 @@
D821C817210036D9002ED995 /* ActivityContentFactoryTests.swift in Sources */,
931D26F719ED7F7500114F17 /* ReaderPostServiceTest.m in Sources */,
B5772AC61C9C84900031F97E /* GravatarServiceTests.swift in Sources */,
937E3AB61E3EBE1600CDA01A /* PostEditorStateTests.swift in Sources */,
FF9A6E7121F9361700D36D14 /* MediaUploadHashTests.swift in Sources */,
B532ACCF1DC3AB8E00FFFA57 /* NotificationSyncMediatorTests.swift in Sources */,
7E4A772F20F7FDF8001C706D /* ActivityLogTestData.swift in Sources */,
Expand Down
188 changes: 0 additions & 188 deletions WordPress/WordPressTest/PostEditorStateTests.swift
View file
Open in desktop

This file was deleted.