From 770d4720ffd7d42185a7ea84604ce6474b74a5a8 Mon Sep 17 00:00:00 2001 From: Brian Litwin Date: Fri, 19 Oct 2018 11:58:29 -0400 Subject: [PATCH 1/4] Created DidTap Protocol for label section controllers - created a protocol to route tap events to a delegate. The idea is that the delgate will have a reference to GithubClient and we can limit the baggage of passing around GithubClient --- Classes/Issues/IssuesViewController.swift | 14 +++++++++++--- .../Labeled/IssueLabeledSectionController.swift | 17 +++++++++++++---- .../Labels/IssueLabelsSectionController.swift | 11 +++++++++-- Classes/Utility/UIViewController+Routing.swift | 1 - 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/Classes/Issues/IssuesViewController.swift b/Classes/Issues/IssuesViewController.swift index 2a60c1c83..c45e18341 100644 --- a/Classes/Issues/IssuesViewController.swift +++ b/Classes/Issues/IssuesViewController.swift @@ -34,7 +34,9 @@ final class IssuesViewController: MessageViewController, IssueCommentSectionControllerDelegate, IssueTextActionsViewSendDelegate, EmptyViewDelegate, - MessageTextViewListener { + MessageTextViewListener, + IssueLabelTapSectionControllerDelegate +{ private let client: GithubClient private let model: IssueDetailsModel @@ -451,7 +453,7 @@ final class IssuesViewController: MessageViewController, switch object { // header and metadata case is IssueTitleModel: return IssueTitleSectionController() - case is IssueLabelsModel: return IssueLabelsSectionController(issue: model) + case is IssueLabelsModel: return IssueLabelsSectionController(issue: model, tapDelegate: self) case is IssueAssigneesModel: return IssueAssigneesSectionController() case is Milestone: return IssueMilestoneSectionController(issueModel: model) case is IssueFileChangesModel: return IssueViewFilesSectionController(issueModel: model, client: client) @@ -465,7 +467,7 @@ final class IssuesViewController: MessageViewController, autocomplete: autocompleteController.autocomplete.copy, issueCommentDelegate: self ) - case is IssueLabeledModel: return IssueLabeledSectionController(issueModel: model) + case is IssueLabeledModel: return IssueLabeledSectionController(issueModel: model, tapDelegate: self) case is IssueStatusEventModel: return IssueStatusEventSectionController(issueModel: model) case is IssueReferencedModel: return IssueReferencedSectionController(client: client) case is IssueReferencedCommitModel: return IssueReferencedCommitSectionController() @@ -631,5 +633,11 @@ final class IssuesViewController: MessageViewController, func didChangeSelection(textView: MessageTextView) {} func willChangeRange(textView: MessageTextView, to range: NSRange) {} + + // MARK: IssueLabelsSectionControllerDelegate + + func didTapLabel(owner: String, repo: String, label: String) { + print("Label tapped! \(owner) \(repo) \(label)" ) + } } diff --git a/Classes/Issues/Labeled/IssueLabeledSectionController.swift b/Classes/Issues/Labeled/IssueLabeledSectionController.swift index 23a160eba..43f0a877e 100644 --- a/Classes/Issues/Labeled/IssueLabeledSectionController.swift +++ b/Classes/Issues/Labeled/IssueLabeledSectionController.swift @@ -9,13 +9,17 @@ import Foundation import IGListKit -final class IssueLabeledSectionController: ListGenericSectionController { + + +final class IssueLabeledSectionController: ListGenericSectionController, MarkdownStyledTextViewDelegate { private let issueModel: IssueDetailsModel + private weak var tapDelegate: IssueLabelTapSectionControllerDelegate? - init(issueModel: IssueDetailsModel) { + init(issueModel: IssueDetailsModel, tapDelegate: IssueLabelTapSectionControllerDelegate) { self.issueModel = issueModel super.init() + self.tapDelegate = tapDelegate } override func sizeForItem(at index: Int) -> CGSize { @@ -28,8 +32,13 @@ final class IssueLabeledSectionController: ListGenericSectionController, ListBindingSectionControllerDataSource, ListBindingSectionControllerSelectionDelegate { @@ -16,10 +21,12 @@ ListBindingSectionControllerSelectionDelegate { private let issue: IssueDetailsModel private var sizeCache = [String: CGSize]() private let lockedModel = Constants.Strings.locked + private weak var tapDelegate: IssueLabelTapSectionControllerDelegate? - init(issue: IssueDetailsModel) { + init(issue: IssueDetailsModel, tapDelegate: IssueLabelTapSectionControllerDelegate) { self.issue = issue super.init() + self.tapDelegate = tapDelegate minimumInteritemSpacing = Styles.Sizes.labelSpacing minimumLineSpacing = Styles.Sizes.labelSpacing let row = Styles.Sizes.rowSpacing @@ -97,7 +104,7 @@ ListBindingSectionControllerSelectionDelegate { func sectionController(_ sectionController: ListBindingSectionController, didSelectItemAt index: Int, viewModel: Any) { guard let viewModel = viewModel as? RepositoryLabel else { return } - viewController?.presentLabels(owner: issue.owner, repo: issue.repo, label: viewModel.name) + tapDelegate?.didTapLabel(owner: issue.owner, repo: issue.repo, label: viewModel.name) } } diff --git a/Classes/Utility/UIViewController+Routing.swift b/Classes/Utility/UIViewController+Routing.swift index cc69b0a6c..7f50aed3b 100644 --- a/Classes/Utility/UIViewController+Routing.swift +++ b/Classes/Utility/UIViewController+Routing.swift @@ -19,7 +19,6 @@ extension UIViewController { UIApplication.shared.open(url, options: [:], completionHandler: nil) } case .username(let username): presentProfile(login: username) - case .label(let label): presentLabels(owner: label.owner, repo: label.repo, label: label.label) case .commit(let commit): presentCommit(owner: commit.owner, repo: commit.repo, hash: commit.hash) default: return false } From 1f854b70b9900b0f57a673e0d35f0f605ab24ef7 Mon Sep 17 00:00:00 2001 From: Brian Litwin Date: Fri, 19 Oct 2018 12:58:21 -0400 Subject: [PATCH 2/4] Refactor Repo VC and SC to use name and owner as parameters - Will make it easier to present RepoVC without needing a ref to RepositoryDetails. The only parameters being used from RepositoryDetails were owner and name --- .../Repository/RepositoryIssuesViewController.swift | 12 +++++++----- .../RepositorySummarySectionController.swift | 8 +++++--- Classes/Repository/RepositoryViewController.swift | 4 ++-- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Classes/Repository/RepositoryIssuesViewController.swift b/Classes/Repository/RepositoryIssuesViewController.swift index bac4aa825..628dedc8d 100644 --- a/Classes/Repository/RepositoryIssuesViewController.swift +++ b/Classes/Repository/RepositoryIssuesViewController.swift @@ -19,16 +19,18 @@ BaseListViewControllerDataSource, SearchBarSectionControllerDelegate { private var models = [ListDiffable]() - private let repo: RepositoryDetails + private let owner: String + private let repo: String private let client: RepositoryClient private let type: RepositoryIssuesType private let searchKey: ListDiffable = "searchKey" as ListDiffable private let debouncer = Debouncer() private var previousSearchString = "is:open " - init(client: GithubClient, repo: RepositoryDetails, type: RepositoryIssuesType) { + init(client: GithubClient, owner: String, repo: String, type: RepositoryIssuesType) { + self.owner = owner self.repo = repo - self.client = RepositoryClient(githubClient: client, owner: repo.owner, name: repo.name) + self.client = RepositoryClient(githubClient: client, owner: owner, name: repo) self.type = type super.init( @@ -105,7 +107,7 @@ SearchBarSectionControllerDelegate { query: previousSearchString ) } - return RepositorySummarySectionController(client: client.githubClient, repo: repo) + return RepositorySummarySectionController(client: client.githubClient, owner: owner, repo: repo) } func emptySectionController(listAdapter: ListAdapter) -> ListSectionController { @@ -129,7 +131,7 @@ SearchBarSectionControllerDelegate { case .issues: typeQuery = "is:issue" case .pullRequests: typeQuery = "is:pr" } - return "repo:\(repo.owner)/\(repo.name) \(typeQuery) \(previousSearchString)" + return "repo:\(owner)/\(repo) \(typeQuery) \(previousSearchString)" } } diff --git a/Classes/Repository/RepositorySummarySectionController.swift b/Classes/Repository/RepositorySummarySectionController.swift index 831674d17..9cb87ad51 100644 --- a/Classes/Repository/RepositorySummarySectionController.swift +++ b/Classes/Repository/RepositorySummarySectionController.swift @@ -11,10 +11,12 @@ import IGListKit final class RepositorySummarySectionController: ListGenericSectionController { private let client: GithubClient - private let repo: RepositoryDetails + private let owner: String + private let repo: String - init(client: GithubClient, repo: RepositoryDetails) { + init(client: GithubClient, owner: String, repo: String) { self.client = client + self.owner = owner self.repo = repo super.init() } @@ -53,7 +55,7 @@ final class RepositorySummarySectionController: ListGenericSectionController Date: Fri, 19 Oct 2018 13:21:28 -0400 Subject: [PATCH 3/4] Added presentLabels extension to UIViewController --- Classes/Issues/IssuesViewController.swift | 4 +-- .../IssueLabeledSectionController.swift | 2 +- .../Labels/IssueLabelsSectionController.swift | 4 +-- .../RepositoryIssuesViewController.swift | 18 ++++++++---- .../UIViewController+PresentLabels.swift | 28 +++++++++++++++++++ .../UIViewController+Safari.swift | 5 ---- Freetime.xcodeproj/project.pbxproj | 4 +++ 7 files changed, 50 insertions(+), 15 deletions(-) create mode 100644 Classes/View Controllers/UIViewController+PresentLabels.swift diff --git a/Classes/Issues/IssuesViewController.swift b/Classes/Issues/IssuesViewController.swift index c45e18341..e163294a8 100644 --- a/Classes/Issues/IssuesViewController.swift +++ b/Classes/Issues/IssuesViewController.swift @@ -636,8 +636,8 @@ final class IssuesViewController: MessageViewController, // MARK: IssueLabelsSectionControllerDelegate - func didTapLabel(owner: String, repo: String, label: String) { - print("Label tapped! \(owner) \(repo) \(label)" ) + func didTapIssueLabel(owner: String, repo: String, label: String) { + presentLabels(client: client, owner: owner, repo: repo, label: label) } } diff --git a/Classes/Issues/Labeled/IssueLabeledSectionController.swift b/Classes/Issues/Labeled/IssueLabeledSectionController.swift index 43f0a877e..a1807d5b1 100644 --- a/Classes/Issues/Labeled/IssueLabeledSectionController.swift +++ b/Classes/Issues/Labeled/IssueLabeledSectionController.swift @@ -38,7 +38,7 @@ final class IssueLabeledSectionController: ListGenericSectionController, @@ -104,7 +104,7 @@ ListBindingSectionControllerSelectionDelegate { func sectionController(_ sectionController: ListBindingSectionController, didSelectItemAt index: Int, viewModel: Any) { guard let viewModel = viewModel as? RepositoryLabel else { return } - tapDelegate?.didTapLabel(owner: issue.owner, repo: issue.repo, label: viewModel.name) + tapDelegate?.didTapIssueLabel(owner: issue.owner, repo: issue.repo, label: viewModel.name) } } diff --git a/Classes/Repository/RepositoryIssuesViewController.swift b/Classes/Repository/RepositoryIssuesViewController.swift index 628dedc8d..3a34fe552 100644 --- a/Classes/Repository/RepositoryIssuesViewController.swift +++ b/Classes/Repository/RepositoryIssuesViewController.swift @@ -26,12 +26,17 @@ SearchBarSectionControllerDelegate { private let searchKey: ListDiffable = "searchKey" as ListDiffable private let debouncer = Debouncer() private var previousSearchString = "is:open " + private var label: String? - init(client: GithubClient, owner: String, repo: String, type: RepositoryIssuesType) { + init(client: GithubClient, owner: String, repo: String, type: RepositoryIssuesType, label: String? = nil) { self.owner = owner self.repo = repo self.client = RepositoryClient(githubClient: client, owner: owner, name: repo) self.type = type + self.label = label + if let label = label { + previousSearchString += "label:\(label) " + } super.init( emptyErrorMessage: NSLocalizedString("Cannot load issues.", comment: "") @@ -53,10 +58,13 @@ SearchBarSectionControllerDelegate { super.viewDidLoad() makeBackBarItemEmpty() - - // set the frame in -viewDidLoad is required when working with TabMan - feed.collectionView.frame = view.bounds - feed.collectionView.contentInsetAdjustmentBehavior = .never + + let presentingInTabMan = label == nil + if presentingInTabMan { + // set the frame in -viewDidLoad is required when working with TabMan + feed.collectionView.frame = view.bounds + feed.collectionView.contentInsetAdjustmentBehavior = .never + } } // MARK: Overrides diff --git a/Classes/View Controllers/UIViewController+PresentLabels.swift b/Classes/View Controllers/UIViewController+PresentLabels.swift new file mode 100644 index 000000000..41141fe65 --- /dev/null +++ b/Classes/View Controllers/UIViewController+PresentLabels.swift @@ -0,0 +1,28 @@ +// +// UIViewController+PresentLabels.swift +// Freetime +// +// Created by B_Litwin on 10/19/18. +// Copyright © 2018 Ryan Nystrom. All rights reserved. +// + +import UIKit +import GitHubAPI + +extension UIViewController { + func presentLabels(client: GithubClient, owner: String, repo: String, label: String) { + let repositoryIssuesViewController = + RepositoryIssuesViewController( + client: client, + owner: owner, + repo: repo, + type: .issues, + label: label + ) + + navigationController?.pushViewController( + repositoryIssuesViewController, + animated: true + ) + } +} diff --git a/Classes/View Controllers/UIViewController+Safari.swift b/Classes/View Controllers/UIViewController+Safari.swift index 0bff58c7b..5142f991e 100644 --- a/Classes/View Controllers/UIViewController+Safari.swift +++ b/Classes/View Controllers/UIViewController+Safari.swift @@ -30,11 +30,6 @@ extension UIViewController { presentSafari(url: url) } - func presentLabels(owner: String, repo: String, label: String) { - guard let url = URL(string: "https://github.com/\(owner)/\(repo)/labels/\(label)") else { return } - presentSafari(url: url) - } - func presentMilestone(owner: String, repo: String, milestone: Int) { guard let url = URL(string: "https://github.com/\(owner)/\(repo)/milestone/\(milestone)") else { return } presentSafari(url: url) diff --git a/Freetime.xcodeproj/project.pbxproj b/Freetime.xcodeproj/project.pbxproj index e14e29225..d2ba8d408 100644 --- a/Freetime.xcodeproj/project.pbxproj +++ b/Freetime.xcodeproj/project.pbxproj @@ -449,6 +449,7 @@ 98F9F4011F9CCFFE005A0266 /* ImgurClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98F9F3FE1F9CCFFE005A0266 /* ImgurClient.swift */; }; 98F9F4031F9CD006005A0266 /* Image+Base64.swift in Sources */ = {isa = PBXBuildFile; fileRef = 98F9F4021F9CD006005A0266 /* Image+Base64.swift */; }; BD3761B0209E032500401DFB /* BookmarkNavigationItemTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BD3761AF209E032500401DFB /* BookmarkNavigationItemTests.swift */; }; + BD67495C217A47BC00E8E4FD /* UIViewController+PresentLabels.swift in Sources */ = {isa = PBXBuildFile; fileRef = BD67495B217A47BC00E8E4FD /* UIViewController+PresentLabels.swift */; }; BD89007E20B8844B0026013F /* NetworkingURLPathTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BD89007D20B8844B0026013F /* NetworkingURLPathTests.swift */; }; BDB6AA66215FBC35009BB73C /* RepositoryBranchesViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = BDB6AA5F215FBC35009BB73C /* RepositoryBranchesViewModel.swift */; }; BDB6AA67215FBC35009BB73C /* RepositoryBranchUpdatable.swift in Sources */ = {isa = PBXBuildFile; fileRef = BDB6AA60215FBC35009BB73C /* RepositoryBranchUpdatable.swift */; }; @@ -999,6 +1000,7 @@ ACAE4A11E9671879046F0CE7 /* Pods-FreetimeWatch.testflight.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-FreetimeWatch.testflight.xcconfig"; path = "Pods/Target Support Files/Pods-FreetimeWatch/Pods-FreetimeWatch.testflight.xcconfig"; sourceTree = ""; }; B3C439BE890EECD7C0C692C5 /* Pods-Freetime.testflight.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Freetime.testflight.xcconfig"; path = "Pods/Target Support Files/Pods-Freetime/Pods-Freetime.testflight.xcconfig"; sourceTree = ""; }; BD3761AF209E032500401DFB /* BookmarkNavigationItemTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkNavigationItemTests.swift; sourceTree = ""; }; + BD67495B217A47BC00E8E4FD /* UIViewController+PresentLabels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIViewController+PresentLabels.swift"; sourceTree = ""; }; BD89007D20B8844B0026013F /* NetworkingURLPathTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkingURLPathTests.swift; sourceTree = ""; }; BDB6AA5F215FBC35009BB73C /* RepositoryBranchesViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RepositoryBranchesViewModel.swift; sourceTree = ""; }; BDB6AA60215FBC35009BB73C /* RepositoryBranchUpdatable.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RepositoryBranchUpdatable.swift; sourceTree = ""; }; @@ -1743,6 +1745,7 @@ 290056F2210028B20046EAE5 /* UIViewController+MenuDone.swift */, 29792B181FFB10A3007A0C57 /* UIViewController+MessageAutocompleteControllerLayoutDelegate.swift */, 292CD3D11F0DBEC000D3D57B /* UIViewController+Safari.swift */, + BD67495B217A47BC00E8E4FD /* UIViewController+PresentLabels.swift */, 290D2A3C1F044CB20082E6CC /* UIViewController+SmartDeselection.swift */, 299F63E3205E1CAB0015D901 /* UIViewController+StyledTextViewCellDelegate.swift */, 4920F1A71F72E27200131E9D /* UIViewController+UserActivity.swift */, @@ -2762,6 +2765,7 @@ 29459A6F1FE61E0500034A04 /* MarkdownCheckboxModel.swift in Sources */, 299997302031227E00995FFD /* IssueMergeButtonModel.swift in Sources */, 29CEA5CD1F84DB1B009827DB /* BaseListViewController.swift in Sources */, + BD67495C217A47BC00E8E4FD /* UIViewController+PresentLabels.swift in Sources */, 299F63E0205DDF6B0015D901 /* UIViewController+Routing.swift in Sources */, 98835BD41F1A17EE005BA24F /* Bundle+Version.swift in Sources */, 29316DB51ECC7DEB007CAE3F /* ButtonCell.swift in Sources */, From 37a311b099733235686f3a2892470e432485a2d9 Mon Sep 17 00:00:00 2001 From: Brian Litwin Date: Fri, 19 Oct 2018 22:04:07 -0400 Subject: [PATCH 4/4] requested changes --- .../Issues/Labeled/IssueLabeledSectionController.swift | 9 ++++++--- Classes/Issues/Labels/IssueLabelsSectionController.swift | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Classes/Issues/Labeled/IssueLabeledSectionController.swift b/Classes/Issues/Labeled/IssueLabeledSectionController.swift index a1807d5b1..f56fd5798 100644 --- a/Classes/Issues/Labeled/IssueLabeledSectionController.swift +++ b/Classes/Issues/Labeled/IssueLabeledSectionController.swift @@ -18,8 +18,8 @@ final class IssueLabeledSectionController: ListGenericSectionController CGSize { @@ -37,8 +37,11 @@ final class IssueLabeledSectionController: ListGenericSectionController