From 5e7f1f60066a0ca44e0eb5febcd8a7cb86e325c4 Mon Sep 17 00:00:00 2001 From: Julien Bodet Date: Wed, 6 Feb 2019 22:42:54 -0500 Subject: [PATCH 1/2] Show discard or keep alert after tapping outside of the PR reviewers, assignees, milestones, labels modal --- .../IssueManagingContextController.swift | 62 +++++++++++++++++-- Classes/Labels/LabelsViewController.swift | 7 +++ .../Milestones/MilestonesViewController.swift | 7 +++ Classes/People/PeopleViewController.swift | 7 +++ Classes/Utility/AlertAction.swift | 4 ++ 5 files changed, 81 insertions(+), 6 deletions(-) diff --git a/Classes/Issues/IssueManagingContextController.swift b/Classes/Issues/IssueManagingContextController.swift index 437c2d34f..f74fcfbb1 100644 --- a/Classes/Issues/IssueManagingContextController.swift +++ b/Classes/Issues/IssueManagingContextController.swift @@ -304,10 +304,22 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate { Haptic.triggerNotification(.success) } - func didDismiss(selected labels: [RepositoryLabel]) { + func didDismiss(controller: LabelsViewController) { guard let previous = result, - previous.labels.labels != labels + previous.labels.labels != controller.selected else { return } + + if controller.wasDismissedByDone { + self.update(selected: controller.selected, previous: previous) + } else { + // Ask for confirmation + self.showCancelAlert { [weak self] in + self?.update(selected: controller.selected, previous: previous) + } + } + } + + private func update(selected labels: [RepositoryLabel], previous: IssueResult) { delegate?.willMutateModel(from: self) client.mutateLabels( previous: previous, @@ -323,10 +335,22 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate { let selected = controller.selected guard controller.selectionChanged(newValues: selected) else { return } + + if controller.wasDismissedByDone { + self.update(selected: selected, type: controller.type, previous: previous) + } else { + // Ask for confirmation + self.showCancelAlert { [weak self] in + self?.update(selected: selected, type: controller.type, previous: previous) + } + } + } + + private func update(selected assignees: [IssueAssigneeViewModel], type: PeopleViewController.PeopleType, previous: IssueResult) { delegate?.willMutateModel(from: self) let mutationType: V3AddPeopleRequest.PeopleType - switch controller.type { + switch type { case .assignee: mutationType = .assignees case .reviewer: mutationType = .reviewers } @@ -337,7 +361,7 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate { owner: model.owner, repo: model.repo, number: model.number, - people: controller.selected + people: assignees ) } @@ -345,13 +369,25 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate { guard let previous = result, previous.milestone != controller.selected else { return } + + if controller.wasDismissedByDone { + self.update(selected: controller.selected, previous: previous) + } else { + // Ask for confirmation + self.showCancelAlert { [weak self] in + self?.update(selected: controller.selected, previous: previous) + } + } + } + + private func update(selected milestone: Milestone?, previous: IssueResult) { delegate?.willMutateModel(from: self) client.setMilestone( previous: previous, owner: model.owner, repo: model.repo, number: model.number, - milestone: controller.selected + milestone: milestone ) } @@ -363,10 +399,24 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate { } else if let people = viewController as? PeopleViewController { didDismiss(controller: people) } else if let labels = viewController as? LabelsViewController { - didDismiss(selected: labels.selected) + didDismiss(controller: labels) } } func contextMenuDidDismiss(viewController: UIViewController, animated: Bool) {} + func showCancelAlert(onKeep: @escaping () -> Void) { + guard let viewController = self.viewController else { return } + + let title = NSLocalizedString("Are you sure?", comment: "") + let message = NSLocalizedString("Some changes have been made. Are you sure to discard them?", comment: "") + let alert = UIAlertController.configured(title: title, message: message, preferredStyle: .alert) + + alert.addActions([ + AlertAction.keep { _ in onKeep() }, + AlertAction.discard() + ]) + + viewController.present(alert, animated: trueUnlessReduceMotionEnabled) + } } diff --git a/Classes/Labels/LabelsViewController.swift b/Classes/Labels/LabelsViewController.swift index 0b6c0a0f6..d51d6d844 100644 --- a/Classes/Labels/LabelsViewController.swift +++ b/Classes/Labels/LabelsViewController.swift @@ -19,6 +19,8 @@ LabelSectionControllerDelegate { private let client: GithubClient private let request: RepositoryLabelsQuery + var wasDismissedByDone: Bool = false + init( selected: [RepositoryLabel], client: GithubClient, @@ -102,6 +104,11 @@ LabelSectionControllerDelegate { }) } + override func onMenuDone() { + self.wasDismissedByDone = true + super.onMenuDone() + } + // MARK: BaseListViewControllerDataSource func models(adapter: ListSwiftAdapter) -> [ListSwiftPair] { diff --git a/Classes/Milestones/MilestonesViewController.swift b/Classes/Milestones/MilestonesViewController.swift index b621998ee..7ae5dd42a 100644 --- a/Classes/Milestones/MilestonesViewController.swift +++ b/Classes/Milestones/MilestonesViewController.swift @@ -26,6 +26,8 @@ MilestoneSectionControllerDelegate { private let feedRefresh = FeedRefresh() private var milestones = [Milestone]() + var wasDismissedByDone: Bool = false + private let dateFormatter: DateFormatter = { let formatter = DateFormatter() formatter.dateStyle = .long @@ -96,6 +98,11 @@ MilestoneSectionControllerDelegate { } } + override func onMenuDone() { + self.wasDismissedByDone = true + super.onMenuDone() + } + // MARK: BaseListViewControllerDataSource func models(adapter: ListSwiftAdapter) -> [ListSwiftPair] { diff --git a/Classes/People/PeopleViewController.swift b/Classes/People/PeopleViewController.swift index 7c56b81bc..5830faf24 100644 --- a/Classes/People/PeopleViewController.swift +++ b/Classes/People/PeopleViewController.swift @@ -30,6 +30,8 @@ PeopleSectionControllerDelegate { private var owner: String private var repo: String + var wasDismissedByDone: Bool = false + init( selections: [String], exclusions: [String], @@ -165,6 +167,11 @@ PeopleSectionControllerDelegate { } } + @objc override func onMenuDone() { + self.wasDismissedByDone = true + super.onMenuDone() + } + // MARK: BaseListViewControllerDataSource func models(adapter: ListSwiftAdapter) -> [ListSwiftPair] { diff --git a/Classes/Utility/AlertAction.swift b/Classes/Utility/AlertAction.swift index c483b84a6..85c92f541 100644 --- a/Classes/Utility/AlertAction.swift +++ b/Classes/Utility/AlertAction.swift @@ -113,6 +113,10 @@ struct AlertAction { return UIAlertAction(title: NSLocalizedString("Discard", comment: ""), style: .destructive, handler: handler) } + static func keep(_ handler: AlertActionBlock? = nil) -> UIAlertAction { + return UIAlertAction(title: NSLocalizedString("Keep", comment: ""), style: .default, handler: handler) + } + static func delete(_ handler: AlertActionBlock? = nil) -> UIAlertAction { return UIAlertAction(title: NSLocalizedString("Delete", comment: ""), style: .destructive, handler: handler) } From 554c8472b0fa4b009855a9c6e718333f71356d83 Mon Sep 17 00:00:00 2001 From: Julien Bodet Date: Sat, 16 Mar 2019 18:00:34 -0400 Subject: [PATCH 2/2] add a completion for the alert, inferred Bool, update text --- .../Issues/IssueManagingContextController.swift | 17 ++++++++++------- Classes/Labels/LabelsViewController.swift | 2 +- .../Milestones/MilestonesViewController.swift | 2 +- Classes/People/PeopleViewController.swift | 2 +- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/Classes/Issues/IssueManagingContextController.swift b/Classes/Issues/IssueManagingContextController.swift index f74fcfbb1..2f9ea4eba 100644 --- a/Classes/Issues/IssueManagingContextController.swift +++ b/Classes/Issues/IssueManagingContextController.swift @@ -313,7 +313,8 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate { self.update(selected: controller.selected, previous: previous) } else { // Ask for confirmation - self.showCancelAlert { [weak self] in + self.showCancelAlert { [weak self] keepChanges in + guard keepChanges else { return } self?.update(selected: controller.selected, previous: previous) } } @@ -340,7 +341,8 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate { self.update(selected: selected, type: controller.type, previous: previous) } else { // Ask for confirmation - self.showCancelAlert { [weak self] in + self.showCancelAlert { [weak self] keepChanges in + guard keepChanges else { return } self?.update(selected: selected, type: controller.type, previous: previous) } } @@ -374,7 +376,8 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate { self.update(selected: controller.selected, previous: previous) } else { // Ask for confirmation - self.showCancelAlert { [weak self] in + self.showCancelAlert { [weak self] keepChanges in + guard keepChanges else { return } self?.update(selected: controller.selected, previous: previous) } } @@ -405,16 +408,16 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate { func contextMenuDidDismiss(viewController: UIViewController, animated: Bool) {} - func showCancelAlert(onKeep: @escaping () -> Void) { + private func showCancelAlert(completion: @escaping (_ keepChanges: Bool) -> Void) { guard let viewController = self.viewController else { return } let title = NSLocalizedString("Are you sure?", comment: "") - let message = NSLocalizedString("Some changes have been made. Are you sure to discard them?", comment: "") + let message = NSLocalizedString("Some changes have been made. Are you sure you want to discard them?", comment: "") let alert = UIAlertController.configured(title: title, message: message, preferredStyle: .alert) alert.addActions([ - AlertAction.keep { _ in onKeep() }, - AlertAction.discard() + AlertAction.keep { _ in completion(true) }, + AlertAction.discard { _ in completion(false) } ]) viewController.present(alert, animated: trueUnlessReduceMotionEnabled) diff --git a/Classes/Labels/LabelsViewController.swift b/Classes/Labels/LabelsViewController.swift index d51d6d844..4c3ef7fbf 100644 --- a/Classes/Labels/LabelsViewController.swift +++ b/Classes/Labels/LabelsViewController.swift @@ -19,7 +19,7 @@ LabelSectionControllerDelegate { private let client: GithubClient private let request: RepositoryLabelsQuery - var wasDismissedByDone: Bool = false + var wasDismissedByDone = false init( selected: [RepositoryLabel], diff --git a/Classes/Milestones/MilestonesViewController.swift b/Classes/Milestones/MilestonesViewController.swift index 7ae5dd42a..3ea587115 100644 --- a/Classes/Milestones/MilestonesViewController.swift +++ b/Classes/Milestones/MilestonesViewController.swift @@ -26,7 +26,7 @@ MilestoneSectionControllerDelegate { private let feedRefresh = FeedRefresh() private var milestones = [Milestone]() - var wasDismissedByDone: Bool = false + var wasDismissedByDone = false private let dateFormatter: DateFormatter = { let formatter = DateFormatter() diff --git a/Classes/People/PeopleViewController.swift b/Classes/People/PeopleViewController.swift index 5830faf24..538a118c8 100644 --- a/Classes/People/PeopleViewController.swift +++ b/Classes/People/PeopleViewController.swift @@ -30,7 +30,7 @@ PeopleSectionControllerDelegate { private var owner: String private var repo: String - var wasDismissedByDone: Bool = false + var wasDismissedByDone = false init( selections: [String],