Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Show discard or keep alert after tapping outside of the PR reviewers,… #2633

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Show discard or keep alert after tapping outside of the PR reviewers,… #2633

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e7f1f6
Show discard or keep alert after tapping outside of the PR reviewers,…
julienbodet Feb 7, 2019
554c847
add a completion for the alert, inferred Bool, update text
julienbodet Mar 16, 2019
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
62 changes: 56 additions & 6 deletions Classes/Issues/IssueManagingContextController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -323,10 +335,22 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate {

let selected = controller.selected
guard controller.selectionChanged(newValues: selected) else { return }

if controller.wasDismissedByDone {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we put this logic in a function, as it seems to be repeated from lines 312-320?

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
}
Expand All @@ -337,21 +361,33 @@ final class IssueManagingContextController: NSObject, ContextMenuDelegate {
owner: model.owner,
repo: model.repo,
number: model.number,
people: controller.selected
people: assignees
)
}

func didDismiss(controller: MilestonesViewController) {
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
)
}

Expand All @@ -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) {
julienbodet marked this conversation as resolved.
Show resolved Hide resolved
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: "")
julienbodet marked this conversation as resolved.
Show resolved Hide resolved
let alert = UIAlertController.configured(title: title, message: message, preferredStyle: .alert)

alert.addActions([
AlertAction.keep { _ in onKeep() },
AlertAction.discard()
])

viewController.present(alert, animated: trueUnlessReduceMotionEnabled)
}
}
7 changes: 7 additions & 0 deletions Classes/Labels/LabelsViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ LabelSectionControllerDelegate {
private let client: GithubClient
private let request: RepositoryLabelsQuery

var wasDismissedByDone: Bool = false
julienbodet marked this conversation as resolved.
Show resolved Hide resolved

init(
selected: [RepositoryLabel],
client: GithubClient,
Expand Down Expand Up @@ -102,6 +104,11 @@ LabelSectionControllerDelegate {
})
}

override func onMenuDone() {
self.wasDismissedByDone = true
super.onMenuDone()
}

// MARK: BaseListViewControllerDataSource

func models(adapter: ListSwiftAdapter) -> [ListSwiftPair] {
Expand Down
7 changes: 7 additions & 0 deletions Classes/Milestones/MilestonesViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -96,6 +98,11 @@ MilestoneSectionControllerDelegate {
}
}

override func onMenuDone() {
self.wasDismissedByDone = true
super.onMenuDone()
}

// MARK: BaseListViewControllerDataSource

func models(adapter: ListSwiftAdapter) -> [ListSwiftPair] {
Expand Down
7 changes: 7 additions & 0 deletions Classes/People/PeopleViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ PeopleSectionControllerDelegate {
private var owner: String
private var repo: String

var wasDismissedByDone: Bool = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this variable be included in the delegate maybe? What do you think?

Copy link
Author

@julienbodet julienbodet Feb 23, 2019 •
edited by BasThomas
Loading

Choose a reason for hiding this comment

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

Are you talking about ContextMenuDelegate? Maybe we can modify its functions signatures to add this flag:

public protocol ContextMenuDelegate: class {
    func contextMenuWillDismiss(viewController: UIViewController, animated: Bool, doneTapped: Bool)
    func contextMenuDidDismiss(viewController: UIViewController, animated: Bool, doneTapped: Bool)
}

Copy link
Collaborator

@BasThomas BasThomas Feb 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, that sounds good! Would maybe make it didTapDone (or maybe even viaButton: Button or similar, do be able to differentiate?)

A bit tricky as the ContextMenu is a separate pod and will probably not always contain a Done.


init(
selections: [String],
exclusions: [String],
Expand Down Expand Up @@ -165,6 +167,11 @@ PeopleSectionControllerDelegate {
}
}

@objc override func onMenuDone() {
self.wasDismissedByDone = true
super.onMenuDone()
}

// MARK: BaseListViewControllerDataSource

func models(adapter: ListSwiftAdapter) -> [ListSwiftPair] {
Expand Down
4 changes: 4 additions & 0 deletions Classes/Utility/AlertAction.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down