From f17a278349ec1df5af2aac8e1e92e5496c14b486 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 16 Oct 2019 08:30:37 -0500 Subject: [PATCH 01/13] Fix randomly failing Feedbin test --- .../AccountTests/AccountFeedSyncTest.swift | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/Frameworks/Account/AccountTests/AccountFeedSyncTest.swift b/Frameworks/Account/AccountTests/AccountFeedSyncTest.swift index ecb649750..4aeb1560e 100644 --- a/Frameworks/Account/AccountTests/AccountFeedSyncTest.swift +++ b/Frameworks/Account/AccountTests/AccountFeedSyncTest.swift @@ -27,8 +27,13 @@ class AccountFeedSyncTest: XCTestCase { // Test initial folders let initialExpection = self.expectation(description: "Initial feeds") - account.refreshAll() { _ in - initialExpection.fulfill() + account.refreshAll() { result in + switch result { + case .success: + initialExpection.fulfill() + case .failure(let error): + XCTFail(error.localizedDescription) + } } waitForExpectations(timeout: 5, handler: nil) @@ -41,11 +46,16 @@ class AccountFeedSyncTest: XCTestCase { XCTAssertEqual("https://favicons.feedbinusercontent.com/6ac/6acc098f35ed2bcc0915ca89d50a97e5793eda45.png", daringFireball!.faviconURL) // Test Adding a Feed - testTransport.testFiles["https://api.feedbin.com/v2/subscriptions.json"] = "subscriptions_add.json" + testTransport.testFiles["subscriptions.json"] = "subscriptions_add.json" let addExpection = self.expectation(description: "Add feeds") - account.refreshAll() { _ in - addExpection.fulfill() + account.refreshAll() { result in + switch result { + case .success: + addExpection.fulfill() + case .failure(let error): + XCTFail(error.localizedDescription) + } } waitForExpectations(timeout: 5, handler: nil) From acbbee870e21b4b6052c86704e3015a0e57f8c56 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 16 Oct 2019 08:36:21 -0500 Subject: [PATCH 02/13] Rename Feedbin specific tests to reflect the specificity and move them to a Feedbin folder --- .../Account/Account.xcodeproj/project.pbxproj | 32 ++++++++++++------- ...ccountFeedbinFolderContentsSyncTest.swift} | 4 +-- .../AccountFeedbinFolderSyncTest.swift} | 4 +-- .../AccountFeedbinSyncTest.swift} | 4 +-- 4 files changed, 26 insertions(+), 18 deletions(-) rename Frameworks/Account/AccountTests/{AccountFolderContentsSyncTest.swift => Feedbin/AccountFeedbinFolderContentsSyncTest.swift} (95%) rename Frameworks/Account/AccountTests/{AccountFolderSyncTest.swift => Feedbin/AccountFeedbinFolderSyncTest.swift} (96%) rename Frameworks/Account/AccountTests/{AccountFeedSyncTest.swift => Feedbin/AccountFeedbinSyncTest.swift} (96%) diff --git a/Frameworks/Account/Account.xcodeproj/project.pbxproj b/Frameworks/Account/Account.xcodeproj/project.pbxproj index 6c9139eec..0f04f4032 100644 --- a/Frameworks/Account/Account.xcodeproj/project.pbxproj +++ b/Frameworks/Account/Account.xcodeproj/project.pbxproj @@ -12,7 +12,7 @@ 5107A09D227DE77700C7C3C5 /* TestTransport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5107A09C227DE77700C7C3C5 /* TestTransport.swift */; }; 510BD111232C3801002692E4 /* AccountMetadataFile.swift in Sources */ = {isa = PBXBuildFile; fileRef = 510BD110232C3801002692E4 /* AccountMetadataFile.swift */; }; 510BD113232C3E9D002692E4 /* FeedMetadataFile.swift in Sources */ = {isa = PBXBuildFile; fileRef = 510BD112232C3E9D002692E4 /* FeedMetadataFile.swift */; }; - 513323082281070D00C30F19 /* AccountFeedSyncTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 513323072281070C00C30F19 /* AccountFeedSyncTest.swift */; }; + 513323082281070D00C30F19 /* AccountFeedbinSyncTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 513323072281070C00C30F19 /* AccountFeedbinSyncTest.swift */; }; 5133230A2281082F00C30F19 /* subscriptions_initial.json in Resources */ = {isa = PBXBuildFile; fileRef = 513323092281082F00C30F19 /* subscriptions_initial.json */; }; 5133230C2281088A00C30F19 /* subscriptions_add.json in Resources */ = {isa = PBXBuildFile; fileRef = 5133230B2281088A00C30F19 /* subscriptions_add.json */; }; 5133230E2281089500C30F19 /* icons.json in Resources */ = {isa = PBXBuildFile; fileRef = 5133230D2281089500C30F19 /* icons.json */; }; @@ -23,7 +23,7 @@ 515E4EB52324FF8C0057B0E7 /* CredentialsManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 515E4EB22324FF8C0057B0E7 /* CredentialsManager.swift */; }; 515E4EB62324FF8C0057B0E7 /* URLRequest+RSWeb.swift in Sources */ = {isa = PBXBuildFile; fileRef = 515E4EB32324FF8C0057B0E7 /* URLRequest+RSWeb.swift */; }; 515E4EB72324FF8C0057B0E7 /* Credentials.swift in Sources */ = {isa = PBXBuildFile; fileRef = 515E4EB42324FF8C0057B0E7 /* Credentials.swift */; }; - 5165D7122282080C00D9D53D /* AccountFolderContentsSyncTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5165D7112282080C00D9D53D /* AccountFolderContentsSyncTest.swift */; }; + 5165D7122282080C00D9D53D /* AccountFeedbinFolderContentsSyncTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5165D7112282080C00D9D53D /* AccountFeedbinFolderContentsSyncTest.swift */; }; 5165D71622821C2400D9D53D /* taggings_delete.json in Resources */ = {isa = PBXBuildFile; fileRef = 5165D71322821C2400D9D53D /* taggings_delete.json */; }; 5165D71722821C2400D9D53D /* taggings_add.json in Resources */ = {isa = PBXBuildFile; fileRef = 5165D71422821C2400D9D53D /* taggings_add.json */; }; 5165D71822821C2400D9D53D /* taggings_initial.json in Resources */ = {isa = PBXBuildFile; fileRef = 5165D71522821C2400D9D53D /* taggings_initial.json */; }; @@ -37,7 +37,7 @@ 51D5875A227F630B00900287 /* tags_delete.json in Resources */ = {isa = PBXBuildFile; fileRef = 51D58757227F630B00900287 /* tags_delete.json */; }; 51D5875B227F630B00900287 /* tags_add.json in Resources */ = {isa = PBXBuildFile; fileRef = 51D58758227F630B00900287 /* tags_add.json */; }; 51D5875C227F630B00900287 /* tags_initial.json in Resources */ = {isa = PBXBuildFile; fileRef = 51D58759227F630B00900287 /* tags_initial.json */; }; - 51D5875E227F643C00900287 /* AccountFolderSyncTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51D5875D227F643C00900287 /* AccountFolderSyncTest.swift */; }; + 51D5875E227F643C00900287 /* AccountFeedbinFolderSyncTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51D5875D227F643C00900287 /* AccountFeedbinFolderSyncTest.swift */; }; 51E148EC234B8FFC0004F7A5 /* SyncDatabase.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 51E148EB234B8FFC0004F7A5 /* SyncDatabase.framework */; }; 51E148ED234B8FFC0004F7A5 /* SyncDatabase.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 51E148EB234B8FFC0004F7A5 /* SyncDatabase.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; 51E3EB41229AF61B00645299 /* AccountError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51E3EB40229AF61B00645299 /* AccountError.swift */; }; @@ -190,7 +190,7 @@ 5107A09C227DE77700C7C3C5 /* TestTransport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestTransport.swift; sourceTree = ""; }; 510BD110232C3801002692E4 /* AccountMetadataFile.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountMetadataFile.swift; sourceTree = ""; }; 510BD112232C3E9D002692E4 /* FeedMetadataFile.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedMetadataFile.swift; sourceTree = ""; }; - 513323072281070C00C30F19 /* AccountFeedSyncTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountFeedSyncTest.swift; sourceTree = ""; }; + 513323072281070C00C30F19 /* AccountFeedbinSyncTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountFeedbinSyncTest.swift; sourceTree = ""; }; 513323092281082F00C30F19 /* subscriptions_initial.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = subscriptions_initial.json; sourceTree = ""; }; 5133230B2281088A00C30F19 /* subscriptions_add.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = subscriptions_add.json; sourceTree = ""; }; 5133230D2281089500C30F19 /* icons.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = icons.json; sourceTree = ""; }; @@ -201,7 +201,7 @@ 515E4EB22324FF8C0057B0E7 /* CredentialsManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CredentialsManager.swift; sourceTree = ""; }; 515E4EB32324FF8C0057B0E7 /* URLRequest+RSWeb.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "URLRequest+RSWeb.swift"; sourceTree = ""; }; 515E4EB42324FF8C0057B0E7 /* Credentials.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Credentials.swift; sourceTree = ""; }; - 5165D7112282080C00D9D53D /* AccountFolderContentsSyncTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountFolderContentsSyncTest.swift; sourceTree = ""; }; + 5165D7112282080C00D9D53D /* AccountFeedbinFolderContentsSyncTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountFeedbinFolderContentsSyncTest.swift; sourceTree = ""; }; 5165D71322821C2400D9D53D /* taggings_delete.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = taggings_delete.json; sourceTree = ""; }; 5165D71422821C2400D9D53D /* taggings_add.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = taggings_add.json; sourceTree = ""; }; 5165D71522821C2400D9D53D /* taggings_initial.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = taggings_initial.json; sourceTree = ""; }; @@ -216,7 +216,7 @@ 51D58757227F630B00900287 /* tags_delete.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = tags_delete.json; sourceTree = ""; }; 51D58758227F630B00900287 /* tags_add.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = tags_add.json; sourceTree = ""; }; 51D58759227F630B00900287 /* tags_initial.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = tags_initial.json; sourceTree = ""; }; - 51D5875D227F643C00900287 /* AccountFolderSyncTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountFolderSyncTest.swift; sourceTree = ""; }; + 51D5875D227F643C00900287 /* AccountFeedbinFolderSyncTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountFeedbinFolderSyncTest.swift; sourceTree = ""; }; 51E148EB234B8FFC0004F7A5 /* SyncDatabase.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = SyncDatabase.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 51E3EB40229AF61B00645299 /* AccountError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountError.swift; sourceTree = ""; }; 51E490352288C37100C791F0 /* FeedbinDate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedbinDate.swift; sourceTree = ""; }; @@ -337,6 +337,16 @@ /* End PBXFrameworksBuildPhase section */ /* Begin PBXGroup section */ + 5111D71C2357534700737D45 /* Feedbin */ = { + isa = PBXGroup; + children = ( + 513323072281070C00C30F19 /* AccountFeedbinSyncTest.swift */, + 5165D7112282080C00D9D53D /* AccountFeedbinFolderContentsSyncTest.swift */, + 51D5875D227F643C00900287 /* AccountFeedbinFolderSyncTest.swift */, + ); + path = Feedbin; + sourceTree = ""; + }; 515E4EB12324FF7D0057B0E7 /* Credentials */ = { isa = PBXGroup; children = ( @@ -498,11 +508,9 @@ isa = PBXGroup; children = ( 5107A098227DE42E00C7C3C5 /* AccountCredentialsTest.swift */, - 51D5875D227F643C00900287 /* AccountFolderSyncTest.swift */, - 513323072281070C00C30F19 /* AccountFeedSyncTest.swift */, - 5165D7112282080C00D9D53D /* AccountFolderContentsSyncTest.swift */, 5107A09C227DE77700C7C3C5 /* TestTransport.swift */, 5107A09A227DE49500C7C3C5 /* TestAccountManager.swift */, + 5111D71C2357534700737D45 /* Feedbin */, 9E7F15082341E97100F860D1 /* Feedly */, 51D58756227F62E300900287 /* JSON */, 848935061F62485000CEBD24 /* Info.plist */, @@ -911,10 +919,10 @@ buildActionMask = 2147483647; files = ( 9E7F15072341E96700F860D1 /* AccountFeedlySyncTest.swift in Sources */, - 5165D7122282080C00D9D53D /* AccountFolderContentsSyncTest.swift in Sources */, - 51D5875E227F643C00900287 /* AccountFolderSyncTest.swift in Sources */, + 5165D7122282080C00D9D53D /* AccountFeedbinFolderContentsSyncTest.swift in Sources */, + 51D5875E227F643C00900287 /* AccountFeedbinFolderSyncTest.swift in Sources */, 5107A09B227DE49500C7C3C5 /* TestAccountManager.swift in Sources */, - 513323082281070D00C30F19 /* AccountFeedSyncTest.swift in Sources */, + 513323082281070D00C30F19 /* AccountFeedbinSyncTest.swift in Sources */, 5107A09D227DE77700C7C3C5 /* TestTransport.swift in Sources */, 5107A099227DE42E00C7C3C5 /* AccountCredentialsTest.swift in Sources */, 9E1773DB234593CF0056A5A8 /* FeedlyResourceIdTests.swift in Sources */, diff --git a/Frameworks/Account/AccountTests/AccountFolderContentsSyncTest.swift b/Frameworks/Account/AccountTests/Feedbin/AccountFeedbinFolderContentsSyncTest.swift similarity index 95% rename from Frameworks/Account/AccountTests/AccountFolderContentsSyncTest.swift rename to Frameworks/Account/AccountTests/Feedbin/AccountFeedbinFolderContentsSyncTest.swift index c3ca12b3c..75210b272 100644 --- a/Frameworks/Account/AccountTests/AccountFolderContentsSyncTest.swift +++ b/Frameworks/Account/AccountTests/Feedbin/AccountFeedbinFolderContentsSyncTest.swift @@ -1,5 +1,5 @@ // -// AccountFolderContentsSyncTest.swift +// AccountFeedbinFolderContentsSyncTest.swift // AccountTests // // Created by Maurice Parker on 5/7/19. @@ -9,7 +9,7 @@ import XCTest @testable import Account -class AccountFolderContentsSyncTest: XCTestCase { +class AccountFeedbinFolderContentsSyncTest: XCTestCase { override func setUp() { } diff --git a/Frameworks/Account/AccountTests/AccountFolderSyncTest.swift b/Frameworks/Account/AccountTests/Feedbin/AccountFeedbinFolderSyncTest.swift similarity index 96% rename from Frameworks/Account/AccountTests/AccountFolderSyncTest.swift rename to Frameworks/Account/AccountTests/Feedbin/AccountFeedbinFolderSyncTest.swift index 09831a937..c7e2a983f 100644 --- a/Frameworks/Account/AccountTests/AccountFolderSyncTest.swift +++ b/Frameworks/Account/AccountTests/Feedbin/AccountFeedbinFolderSyncTest.swift @@ -1,5 +1,5 @@ // -// AccountFolderSyncTest.swift +// AccountFeedbinFolderSyncTest.swift // AccountTests // // Created by Maurice Parker on 5/5/19. @@ -9,7 +9,7 @@ import XCTest @testable import Account -class AccountFolderSyncTest: XCTestCase { +class AccountFeedbinFolderSyncTest: XCTestCase { override func setUp() { } diff --git a/Frameworks/Account/AccountTests/AccountFeedSyncTest.swift b/Frameworks/Account/AccountTests/Feedbin/AccountFeedbinSyncTest.swift similarity index 96% rename from Frameworks/Account/AccountTests/AccountFeedSyncTest.swift rename to Frameworks/Account/AccountTests/Feedbin/AccountFeedbinSyncTest.swift index 4aeb1560e..664f6182c 100644 --- a/Frameworks/Account/AccountTests/AccountFeedSyncTest.swift +++ b/Frameworks/Account/AccountTests/Feedbin/AccountFeedbinSyncTest.swift @@ -1,5 +1,5 @@ // -// AccountFullSyncTest.swift +// AccountFeedbinSyncTest.swift // AccountTests // // Created by Maurice Parker on 5/6/19. @@ -9,7 +9,7 @@ import XCTest @testable import Account -class AccountFeedSyncTest: XCTestCase { +class AccountFeedbinSyncTest: XCTestCase { override func setUp() { } From 0182fb72969aaef6fed300972f4536deee1b1ea1 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 16 Oct 2019 11:31:20 -0500 Subject: [PATCH 03/13] Use a wrapper class to prevent a circular reference between the web view and the article controller --- iOS/Article/ArticleViewController.swift | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/iOS/Article/ArticleViewController.swift b/iOS/Article/ArticleViewController.swift index 8dd1fcc0e..63f92ad6c 100644 --- a/iOS/Article/ArticleViewController.swift +++ b/iOS/Article/ArticleViewController.swift @@ -112,7 +112,7 @@ class ArticleViewController: UIViewController { webView.uiDelegate = self webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasClicked) - webView.configuration.userContentController.add(self, name: MessageName.imageWasClicked) + webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked) // Even though page.html should be loaded into this webview, we have to do it again // to work around this bug: http://www.openradar.me/22855188 @@ -378,6 +378,21 @@ extension ArticleViewController: WKScriptMessageHandler { } +class WrapperScriptMessageHandler: NSObject, WKScriptMessageHandler { + + // We need to wrap a message handler to prevent a circlular reference + private weak var handler: WKScriptMessageHandler? + + init(_ handler: WKScriptMessageHandler) { + self.handler = handler + } + + func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) { + handler?.userContentController(userContentController, didReceive: message) + } + +} + // MARK: UIViewControllerTransitioningDelegate extension ArticleViewController: UIViewControllerTransitioningDelegate { From 1ebb0e60c3402d22e27fb6899ed8e3a8482b21a9 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 16 Oct 2019 11:34:18 -0500 Subject: [PATCH 04/13] Simplify transition handoff --- iOS/Article/ArticleViewController.swift | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/iOS/Article/ArticleViewController.swift b/iOS/Article/ArticleViewController.swift index 63f92ad6c..cd2e181d4 100644 --- a/iOS/Article/ArticleViewController.swift +++ b/iOS/Article/ArticleViewController.swift @@ -67,9 +67,6 @@ class ArticleViewController: UIViewController { } } - var clickedImage: UIImage? - var clickedImageFrame: CGRect? - var articleExtractorButtonState: ArticleExtractorButtonState { get { return articleExtractorButton.buttonState @@ -363,8 +360,8 @@ extension ArticleViewController: WKScriptMessageHandler { if let imageData = Data(base64Encoded: base64Image), let image = UIImage(data: imageData) { let rect = CGRect(x: CGFloat(clickMessage.x), y: CGFloat(clickMessage.y), width: CGFloat(clickMessage.width), height: CGFloat(clickMessage.height)) - clickedImageFrame = webView.convert(rect, to: nil) - clickedImage = image + transition.originFrame = webView.convert(rect, to: nil) + transition.originImage = image let imageVC = UIStoryboard.main.instantiateController(ofType: ImageViewController.self) imageVC.image = image @@ -398,9 +395,6 @@ class WrapperScriptMessageHandler: NSObject, WKScriptMessageHandler { extension ArticleViewController: UIViewControllerTransitioningDelegate { func animationController(forPresented presented: UIViewController, presenting: UIViewController, source: UIViewController) -> UIViewControllerAnimatedTransitioning? { - guard let frame = clickedImageFrame, let image = clickedImage else { return nil } - transition.originFrame = frame - transition.originImage = image transition.presenting = true return transition } From 3ddd14d8563c943a5f2491bc296e4d66d7523cdf Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 16 Oct 2019 11:55:08 -0500 Subject: [PATCH 05/13] Change zoom out animation to make the view controller appear faster --- iOS/Article/ImageTransition.swift | 53 +++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/iOS/Article/ImageTransition.swift b/iOS/Article/ImageTransition.swift index 35d0372eb..f76ab92b8 100644 --- a/iOS/Article/ImageTransition.swift +++ b/iOS/Article/ImageTransition.swift @@ -10,7 +10,7 @@ import UIKit class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { - let duration = 0.3 + let duration = 0.4 var presenting = true var originFrame: CGRect! var originImage: UIImage! @@ -40,22 +40,43 @@ class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { let fromView = transitionContext.view(forKey: .from)! fromView.removeFromSuperview() - transitionContext.containerView.backgroundColor = UIColor.systemBackground - transitionContext.containerView.addSubview(imageView) + if presenting { + + transitionContext.containerView.backgroundColor = UIColor.systemBackground + transitionContext.containerView.addSubview(imageView) + + UIView.animate( + withDuration: duration, + delay:0.0, + usingSpringWithDamping: 0.8, + initialSpringVelocity: 0.2, + animations: { + imageView.frame = targetFrame + }, completion: { _ in + imageView.removeFromSuperview() + let toView = transitionContext.view(forKey: .to)! + transitionContext.containerView.addSubview(toView) + transitionContext.completeTransition(true) + }) + + } else { - UIView.animate( - withDuration: duration, - delay:0.0, - usingSpringWithDamping: 0.8, - initialSpringVelocity: 0.2, - animations: { - imageView.frame = targetFrame - }, completion: { _ in - imageView.removeFromSuperview() - let toView = transitionContext.view(forKey: .to)! - transitionContext.containerView.addSubview(toView) - transitionContext.completeTransition(true) - }) + let toView = transitionContext.view(forKey: .to)! + transitionContext.containerView.addSubview(toView) + transitionContext.containerView.addSubview(imageView) + + UIView.animate( + withDuration: duration, + delay:0.0, + animations: { + imageView.frame = targetFrame + imageView.alpha = 0 + }, completion: { _ in + imageView.removeFromSuperview() + transitionContext.completeTransition(true) + }) + + } } From cb6490222f1b97d738c573c3db6cce400e473687 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 16 Oct 2019 16:40:49 -0500 Subject: [PATCH 06/13] Change image zoom animation to be a solid picture zooming in and out --- iOS/Article/ArticleViewController.swift | 63 ++++++++----- iOS/Article/ImageTransition.swift | 112 +++++++++++++----------- iOS/Resources/main_ios.js | 18 +++- 3 files changed, 120 insertions(+), 73 deletions(-) diff --git a/iOS/Article/ArticleViewController.swift b/iOS/Article/ArticleViewController.swift index cd2e181d4..ae623cefb 100644 --- a/iOS/Article/ArticleViewController.swift +++ b/iOS/Article/ArticleViewController.swift @@ -24,6 +24,7 @@ class ArticleViewController: UIViewController { private struct MessageName { static let imageWasClicked = "imageWasClicked" + static let imageWasShown = "imageWasShown" } @IBOutlet private weak var nextUnreadBarButtonItem: UIBarButtonItem! @@ -43,7 +44,8 @@ class ArticleViewController: UIViewController { }() private var webView: WKWebView! - private var transition = ImageTransition() + private lazy var transition = ImageTransition(controller: self) + private var clickedImageCompletion: (() -> Void)? weak var coordinator: SceneCoordinator! @@ -109,7 +111,9 @@ class ArticleViewController: UIViewController { webView.uiDelegate = self webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasClicked) + webView.configuration.userContentController.removeScriptMessageHandler(forName: MessageName.imageWasShown) webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasClicked) + webView.configuration.userContentController.add(WrapperScriptMessageHandler(self), name: MessageName.imageWasShown) // Even though page.html should be loaded into this webview, we have to do it again // to work around this bug: http://www.openradar.me/22855188 @@ -296,6 +300,15 @@ class ArticleViewController: UIViewController { webView.scrollView.setContentOffset(scrollToPoint, animated: true) } + func hideClickedImage() { + webView?.evaluateJavaScript("hideClickedImage();") + } + + func showClickedImage(completion: @escaping () -> Void) { + clickedImageCompletion = completion + webView?.evaluateJavaScript("showClickedImage();") + } + } // MARK: WKNavigationDelegate @@ -350,26 +363,13 @@ extension ArticleViewController: WKUIDelegate { extension ArticleViewController: WKScriptMessageHandler { func userContentController(_ userContentController: WKUserContentController, didReceive message: WKScriptMessage) { - if message.name == MessageName.imageWasClicked, - let body = message.body as? String, - let data = body.data(using: .utf8), - let clickMessage = try? JSONDecoder().decode(ImageClickMessage.self, from: data), - let range = clickMessage.imageURL.range(of: ";base64,") { - - let base64Image = String(clickMessage.imageURL.suffix(from: range.upperBound)) - if let imageData = Data(base64Encoded: base64Image), let image = UIImage(data: imageData) { - - let rect = CGRect(x: CGFloat(clickMessage.x), y: CGFloat(clickMessage.y), width: CGFloat(clickMessage.width), height: CGFloat(clickMessage.height)) - transition.originFrame = webView.convert(rect, to: nil) - transition.originImage = image - - let imageVC = UIStoryboard.main.instantiateController(ofType: ImageViewController.self) - imageVC.image = image - imageVC.modalPresentationStyle = .fullScreen - imageVC.transitioningDelegate = self - present(imageVC, animated: true) - - } + switch message.name { + case MessageName.imageWasShown: + clickedImageCompletion?() + case MessageName.imageWasClicked: + imageWasClicked(body: message.body as? String) + default: + return } } @@ -430,4 +430,25 @@ private extension ArticleViewController { } } + func imageWasClicked(body: String?) { + guard let body = body, + let data = body.data(using: .utf8), + let clickMessage = try? JSONDecoder().decode(ImageClickMessage.self, from: data), + let range = clickMessage.imageURL.range(of: ";base64,") + else { return } + + let base64Image = String(clickMessage.imageURL.suffix(from: range.upperBound)) + if let imageData = Data(base64Encoded: base64Image), let image = UIImage(data: imageData) { + let rect = CGRect(x: CGFloat(clickMessage.x), y: CGFloat(clickMessage.y), width: CGFloat(clickMessage.width), height: CGFloat(clickMessage.height)) + transition.originFrame = webView.convert(rect, to: nil) + transition.originImage = image + + let imageVC = UIStoryboard.main.instantiateController(ofType: ImageViewController.self) + imageVC.image = image + imageVC.modalPresentationStyle = .fullScreen + imageVC.transitioningDelegate = self + present(imageVC, animated: true) + } + } + } diff --git a/iOS/Article/ImageTransition.swift b/iOS/Article/ImageTransition.swift index f76ab92b8..e3268c184 100644 --- a/iOS/Article/ImageTransition.swift +++ b/iOS/Article/ImageTransition.swift @@ -10,74 +10,84 @@ import UIKit class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { - let duration = 0.4 + private weak var articleController: ArticleViewController? + private let duration = 0.4 var presenting = true var originFrame: CGRect! var originImage: UIImage! + init(controller: ArticleViewController) { + self.articleController = controller + } + func transitionDuration(using transitionContext: UIViewControllerContextTransitioning?) -> TimeInterval { return duration } func animateTransition(using transitionContext: UIViewControllerContextTransitioning) { - - let destFrame: CGRect = { - if presenting { - let imageController = transitionContext.viewController(forKey: .to) as! ImageViewController - return imageController.zoomedFrame - } else { - let imageController = transitionContext.viewController(forKey: .from) as! ImageViewController - return imageController.zoomedFrame - } - }() + if presenting { + animateTransitionPresenting(using: transitionContext) + } else { + animateTransitionReturning(using: transitionContext) + } + } - let initialFrame = presenting ? originFrame! : destFrame - let targetFrame = presenting ? destFrame : originFrame! + private func animateTransitionPresenting(using transitionContext: UIViewControllerContextTransitioning) { let imageView = UIImageView(image: originImage) - imageView.frame = initialFrame + imageView.frame = originFrame + + let fromView = transitionContext.view(forKey: .from)! + fromView.removeFromSuperview() + + transitionContext.containerView.backgroundColor = UIColor.systemBackground + transitionContext.containerView.addSubview(imageView) + + articleController?.hideClickedImage() + + UIView.animate( + withDuration: duration, + delay:0.0, + usingSpringWithDamping: 0.8, + initialSpringVelocity: 0.2, + animations: { + let imageController = transitionContext.viewController(forKey: .to) as! ImageViewController + imageView.frame = imageController.zoomedFrame + }, completion: { _ in + imageView.removeFromSuperview() + let toView = transitionContext.view(forKey: .to)! + transitionContext.containerView.addSubview(toView) + transitionContext.completeTransition(true) + }) + } + + private func animateTransitionReturning(using transitionContext: UIViewControllerContextTransitioning) { + let imageController = transitionContext.viewController(forKey: .from) as! ImageViewController + let imageView = UIImageView(image: originImage) + imageView.frame = imageController.zoomedFrame let fromView = transitionContext.view(forKey: .from)! fromView.removeFromSuperview() - if presenting { - - transitionContext.containerView.backgroundColor = UIColor.systemBackground - transitionContext.containerView.addSubview(imageView) - - UIView.animate( - withDuration: duration, - delay:0.0, - usingSpringWithDamping: 0.8, - initialSpringVelocity: 0.2, - animations: { - imageView.frame = targetFrame - }, completion: { _ in - imageView.removeFromSuperview() - let toView = transitionContext.view(forKey: .to)! - transitionContext.containerView.addSubview(toView) - transitionContext.completeTransition(true) - }) - - } else { + let toView = transitionContext.view(forKey: .to)! + transitionContext.containerView.addSubview(toView) + transitionContext.containerView.addSubview(imageView) - let toView = transitionContext.view(forKey: .to)! - transitionContext.containerView.addSubview(toView) - transitionContext.containerView.addSubview(imageView) - - UIView.animate( - withDuration: duration, - delay:0.0, - animations: { - imageView.frame = targetFrame - imageView.alpha = 0 - }, completion: { _ in - imageView.removeFromSuperview() - transitionContext.completeTransition(true) - }) - - } - + UIView.animate( + withDuration: duration, + delay:0.0, + usingSpringWithDamping: 0.8, + initialSpringVelocity: 0.2, + animations: { + imageView.frame = self.originFrame + }, completion: { _ in + self.articleController?.showClickedImage() { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + imageView.removeFromSuperview() + transitionContext.completeTransition(true) + } + } + }) } } diff --git a/iOS/Resources/main_ios.js b/iOS/Resources/main_ios.js index 0e248a0d7..21d6bdb83 100644 --- a/iOS/Resources/main_ios.js +++ b/iOS/Resources/main_ios.js @@ -1,5 +1,7 @@ // Used to pop a resizable image view async function imageWasClicked(img) { + img.classList.add("nnwClicked"); + const rect = img.getBoundingClientRect(); var message = { @@ -31,7 +33,21 @@ async function imageWasClicked(img) { } -// Add the click listeners for images +// Used to animate the transition to a fullscreen image +function hideClickedImage() { + var img = document.querySelector('.nnwClicked') + img.style.opacity = 0 +} + +// Used to animate the transition from a fullscreen image +function showClickedImage() { + var img = document.querySelector('.nnwClicked') + img.classList.remove("nnwClicked"); + img.style.opacity = 1 + window.webkit.messageHandlers.imageWasShown.postMessage(""); +} + +// Add the click listener for images function imageClicks() { window.onclick = function(event) { if (event.target.matches('img')) { From 6c562f93b51af2368a14f8e905cc7925effd7834 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 16 Oct 2019 19:32:33 -0500 Subject: [PATCH 07/13] Add a masking subview so that the image appears to slide under the nav and toolbars --- iOS/Article/ArticleViewController.swift | 1 + iOS/Article/ImageTransition.swift | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/iOS/Article/ArticleViewController.swift b/iOS/Article/ArticleViewController.swift index ae623cefb..352dd422d 100644 --- a/iOS/Article/ArticleViewController.swift +++ b/iOS/Article/ArticleViewController.swift @@ -441,6 +441,7 @@ private extension ArticleViewController { if let imageData = Data(base64Encoded: base64Image), let image = UIImage(data: imageData) { let rect = CGRect(x: CGFloat(clickMessage.x), y: CGFloat(clickMessage.y), width: CGFloat(clickMessage.width), height: CGFloat(clickMessage.height)) transition.originFrame = webView.convert(rect, to: nil) + transition.maskFrame = webView.convert(webView.frame, to: nil) transition.originImage = image let imageVC = UIStoryboard.main.instantiateController(ofType: ImageViewController.self) diff --git a/iOS/Article/ImageTransition.swift b/iOS/Article/ImageTransition.swift index e3268c184..17a14c926 100644 --- a/iOS/Article/ImageTransition.swift +++ b/iOS/Article/ImageTransition.swift @@ -14,6 +14,7 @@ class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { private let duration = 0.4 var presenting = true var originFrame: CGRect! + var maskFrame: CGRect! var originImage: UIImage! init(controller: ArticleViewController) { @@ -67,11 +68,22 @@ class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { imageView.frame = imageController.zoomedFrame let fromView = transitionContext.view(forKey: .from)! + let windowFrame = fromView.window!.frame fromView.removeFromSuperview() let toView = transitionContext.view(forKey: .to)! transitionContext.containerView.addSubview(toView) - transitionContext.containerView.addSubview(imageView) + + let maskingView = UIView() + + let fullMaskFrame = CGRect(x: windowFrame.minX, y: maskFrame.minY, width: windowFrame.width, height: maskFrame.height) + let path = UIBezierPath(rect: fullMaskFrame) + let maskLayer = CAShapeLayer() + maskLayer.path = path.cgPath + maskingView.layer.mask = maskLayer + + maskingView.addSubview(imageView) + transitionContext.containerView.addSubview(maskingView) UIView.animate( withDuration: duration, From eca5f8259a085e4a366ec629cbc596dc81a312f4 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 16 Oct 2019 20:20:36 -0500 Subject: [PATCH 08/13] Enhance full image view support to work with 3 panel mode and split window views --- iOS/Article/ArticleViewController.swift | 6 +- iOS/Article/ImageScrollView.swift | 653 ++++++++++++------------ iOS/Article/ImageViewController.swift | 10 +- iOS/RootSplitViewController.swift | 1 + iOS/SceneCoordinator.swift | 8 + 5 files changed, 337 insertions(+), 341 deletions(-) diff --git a/iOS/Article/ArticleViewController.swift b/iOS/Article/ArticleViewController.swift index 352dd422d..89396ac74 100644 --- a/iOS/Article/ArticleViewController.swift +++ b/iOS/Article/ArticleViewController.swift @@ -444,11 +444,7 @@ private extension ArticleViewController { transition.maskFrame = webView.convert(webView.frame, to: nil) transition.originImage = image - let imageVC = UIStoryboard.main.instantiateController(ofType: ImageViewController.self) - imageVC.image = image - imageVC.modalPresentationStyle = .fullScreen - imageVC.transitioningDelegate = self - present(imageVC, animated: true) + coordinator.showFullScreenImage(image: image, transitioningDelegate: self) } } diff --git a/iOS/Article/ImageScrollView.swift b/iOS/Article/ImageScrollView.swift index 93292a06b..8439d7ce0 100644 --- a/iOS/Article/ImageScrollView.swift +++ b/iOS/Article/ImageScrollView.swift @@ -9,365 +9,352 @@ import UIKit @objc public protocol ImageScrollViewDelegate: UIScrollViewDelegate { - func imageScrollViewDidChangeOrientation(imageScrollView: ImageScrollView) func imageScrollViewDidGestureSwipeUp(imageScrollView: ImageScrollView) func imageScrollViewDidGestureSwipeDown(imageScrollView: ImageScrollView) } open class ImageScrollView: UIScrollView { - - @objc public enum ScaleMode: Int { - case aspectFill - case aspectFit - case widthFill - case heightFill - } - - @objc public enum Offset: Int { - case begining - case center - } - - static let kZoomInFactorFromMinWhenDoubleTap: CGFloat = 2 - - @objc open var imageContentMode: ScaleMode = .widthFill - @objc open var initialOffset: Offset = .begining - - @objc public private(set) var zoomView: UIImageView? = nil - - @objc open weak var imageScrollViewDelegate: ImageScrollViewDelegate? - - var imageSize: CGSize = CGSize.zero - private var pointToCenterAfterResize: CGPoint = CGPoint.zero - private var scaleToRestoreAfterResize: CGFloat = 1.0 - var maxScaleFromMinScale: CGFloat = 3.0 - + + @objc public enum ScaleMode: Int { + case aspectFill + case aspectFit + case widthFill + case heightFill + } + + @objc public enum Offset: Int { + case begining + case center + } + + static let kZoomInFactorFromMinWhenDoubleTap: CGFloat = 2 + + @objc open var imageContentMode: ScaleMode = .widthFill + @objc open var initialOffset: Offset = .begining + + @objc public private(set) var zoomView: UIImageView? = nil + + @objc open weak var imageScrollViewDelegate: ImageScrollViewDelegate? + + var imageSize: CGSize = CGSize.zero + private var pointToCenterAfterResize: CGPoint = CGPoint.zero + private var scaleToRestoreAfterResize: CGFloat = 1.0 + var maxScaleFromMinScale: CGFloat = 3.0 + var zoomedFrame: CGRect { return zoomView?.frame ?? CGRect.zero } - override open var frame: CGRect { - willSet { - if frame.equalTo(newValue) == false && newValue.equalTo(CGRect.zero) == false && imageSize.equalTo(CGSize.zero) == false { - prepareToResize() - } - } - - didSet { - if frame.equalTo(oldValue) == false && frame.equalTo(CGRect.zero) == false && imageSize.equalTo(CGSize.zero) == false { - recoverFromResizing() - } - } - } - - override public init(frame: CGRect) { - super.init(frame: frame) - - initialize() - } - - required public init?(coder aDecoder: NSCoder) { - super.init(coder: aDecoder) - - initialize() - } - - deinit { - NotificationCenter.default.removeObserver(self) - } - - private func initialize() { - showsVerticalScrollIndicator = false - showsHorizontalScrollIndicator = false - bouncesZoom = true - decelerationRate = UIScrollView.DecelerationRate.fast - delegate = self - - NotificationCenter.default.addObserver(self, selector: #selector(ImageScrollView.changeOrientationNotification), name: UIDevice.orientationDidChangeNotification, object: nil) - } - - @objc public func adjustFrameToCenter() { - - guard let unwrappedZoomView = zoomView else { - return - } - - var frameToCenter = unwrappedZoomView.frame - - // center horizontally - if frameToCenter.size.width < bounds.width { - frameToCenter.origin.x = (bounds.width - frameToCenter.size.width) / 2 - } else { - frameToCenter.origin.x = 0 - } - - // center vertically - if frameToCenter.size.height < bounds.height { - frameToCenter.origin.y = (bounds.height - frameToCenter.size.height) / 2 - } else { - frameToCenter.origin.y = 0 - } - - unwrappedZoomView.frame = frameToCenter - } - - private func prepareToResize() { - let boundsCenter = CGPoint(x: bounds.midX, y: bounds.midY) - pointToCenterAfterResize = convert(boundsCenter, to: zoomView) - - scaleToRestoreAfterResize = zoomScale - - // If we're at the minimum zoom scale, preserve that by returning 0, which will be converted to the minimum - // allowable scale when the scale is restored. - if scaleToRestoreAfterResize <= minimumZoomScale + CGFloat(Float.ulpOfOne) { - scaleToRestoreAfterResize = 0 - } - } - - private func recoverFromResizing() { - setMaxMinZoomScalesForCurrentBounds() - - // restore zoom scale, first making sure it is within the allowable range. - let maxZoomScale = max(minimumZoomScale, scaleToRestoreAfterResize) - zoomScale = min(maximumZoomScale, maxZoomScale) - - // restore center point, first making sure it is within the allowable range. - - // convert our desired center point back to our own coordinate space - let boundsCenter = convert(pointToCenterAfterResize, to: zoomView) - - // calculate the content offset that would yield that center point - var offset = CGPoint(x: boundsCenter.x - bounds.size.width/2.0, y: boundsCenter.y - bounds.size.height/2.0) - - // restore offset, adjusted to be within the allowable range - let maxOffset = maximumContentOffset() - let minOffset = minimumContentOffset() - - var realMaxOffset = min(maxOffset.x, offset.x) - offset.x = max(minOffset.x, realMaxOffset) - - realMaxOffset = min(maxOffset.y, offset.y) - offset.y = max(minOffset.y, realMaxOffset) - - contentOffset = offset - } - - private func maximumContentOffset() -> CGPoint { - return CGPoint(x: contentSize.width - bounds.width,y:contentSize.height - bounds.height) - } - - private func minimumContentOffset() -> CGPoint { - return CGPoint.zero - } - - // MARK: - Set up - - open func setup() { - var topSupperView = superview - - while topSupperView?.superview != nil { - topSupperView = topSupperView?.superview - } - - // Make sure views have already layout with precise frame - topSupperView?.layoutIfNeeded() - } - - // MARK: - Display image - - @objc open func display(image: UIImage) { - - if let zoomView = zoomView { - zoomView.removeFromSuperview() - } - - zoomView = UIImageView(image: image) - zoomView!.isUserInteractionEnabled = true - addSubview(zoomView!) - - let tapGesture = UITapGestureRecognizer(target: self, action: #selector(doubleTapGestureRecognizer(_:))) - tapGesture.numberOfTapsRequired = 2 - zoomView!.addGestureRecognizer(tapGesture) - + override open var frame: CGRect { + willSet { + if frame.equalTo(newValue) == false && newValue.equalTo(CGRect.zero) == false && imageSize.equalTo(CGSize.zero) == false { + prepareToResize() + } + } + + didSet { + if frame.equalTo(oldValue) == false && frame.equalTo(CGRect.zero) == false && imageSize.equalTo(CGSize.zero) == false { + recoverFromResizing() + } + } + } + + override public init(frame: CGRect) { + super.init(frame: frame) + + initialize() + } + + required public init?(coder aDecoder: NSCoder) { + super.init(coder: aDecoder) + + initialize() + } + + private func initialize() { + showsVerticalScrollIndicator = false + showsHorizontalScrollIndicator = false + bouncesZoom = true + decelerationRate = UIScrollView.DecelerationRate.fast + delegate = self + } + + @objc public func adjustFrameToCenter() { + + guard let unwrappedZoomView = zoomView else { + return + } + + var frameToCenter = unwrappedZoomView.frame + + // center horizontally + if frameToCenter.size.width < bounds.width { + frameToCenter.origin.x = (bounds.width - frameToCenter.size.width) / 2 + } else { + frameToCenter.origin.x = 0 + } + + // center vertically + if frameToCenter.size.height < bounds.height { + frameToCenter.origin.y = (bounds.height - frameToCenter.size.height) / 2 + } else { + frameToCenter.origin.y = 0 + } + + unwrappedZoomView.frame = frameToCenter + } + + private func prepareToResize() { + let boundsCenter = CGPoint(x: bounds.midX, y: bounds.midY) + pointToCenterAfterResize = convert(boundsCenter, to: zoomView) + + scaleToRestoreAfterResize = zoomScale + + // If we're at the minimum zoom scale, preserve that by returning 0, which will be converted to the minimum + // allowable scale when the scale is restored. + if scaleToRestoreAfterResize <= minimumZoomScale + CGFloat(Float.ulpOfOne) { + scaleToRestoreAfterResize = 0 + } + } + + private func recoverFromResizing() { + setMaxMinZoomScalesForCurrentBounds() + + // restore zoom scale, first making sure it is within the allowable range. + let maxZoomScale = max(minimumZoomScale, scaleToRestoreAfterResize) + zoomScale = min(maximumZoomScale, maxZoomScale) + + // restore center point, first making sure it is within the allowable range. + + // convert our desired center point back to our own coordinate space + let boundsCenter = convert(pointToCenterAfterResize, to: zoomView) + + // calculate the content offset that would yield that center point + var offset = CGPoint(x: boundsCenter.x - bounds.size.width/2.0, y: boundsCenter.y - bounds.size.height/2.0) + + // restore offset, adjusted to be within the allowable range + let maxOffset = maximumContentOffset() + let minOffset = minimumContentOffset() + + var realMaxOffset = min(maxOffset.x, offset.x) + offset.x = max(minOffset.x, realMaxOffset) + + realMaxOffset = min(maxOffset.y, offset.y) + offset.y = max(minOffset.y, realMaxOffset) + + contentOffset = offset + } + + private func maximumContentOffset() -> CGPoint { + return CGPoint(x: contentSize.width - bounds.width,y:contentSize.height - bounds.height) + } + + private func minimumContentOffset() -> CGPoint { + return CGPoint.zero + } + + // MARK: - Set up + + open func setup() { + var topSupperView = superview + + while topSupperView?.superview != nil { + topSupperView = topSupperView?.superview + } + + // Make sure views have already layout with precise frame + topSupperView?.layoutIfNeeded() + } + + // MARK: - Display image + + @objc open func display(image: UIImage) { + + if let zoomView = zoomView { + zoomView.removeFromSuperview() + } + + zoomView = UIImageView(image: image) + zoomView!.isUserInteractionEnabled = true + addSubview(zoomView!) + + let tapGesture = UITapGestureRecognizer(target: self, action: #selector(doubleTapGestureRecognizer(_:))) + tapGesture.numberOfTapsRequired = 2 + zoomView!.addGestureRecognizer(tapGesture) + let downSwipeGesture = UISwipeGestureRecognizer(target: self, action: #selector(swipeUpGestureRecognizer(_:))) downSwipeGesture.direction = .down zoomView!.addGestureRecognizer(downSwipeGesture) - + let upSwipeGesture = UISwipeGestureRecognizer(target: self, action: #selector(swipeDownGestureRecognizer(_:))) upSwipeGesture.direction = .up zoomView!.addGestureRecognizer(upSwipeGesture) - configureImageForSize(image.size) - } - - private func configureImageForSize(_ size: CGSize) { - imageSize = size - contentSize = imageSize - setMaxMinZoomScalesForCurrentBounds() - zoomScale = minimumZoomScale - - switch initialOffset { - case .begining: - contentOffset = CGPoint.zero - case .center: - let xOffset = contentSize.width < bounds.width ? 0 : (contentSize.width - bounds.width)/2 - let yOffset = contentSize.height < bounds.height ? 0 : (contentSize.height - bounds.height)/2 - - switch imageContentMode { - case .aspectFit: - contentOffset = CGPoint.zero - case .aspectFill: - contentOffset = CGPoint(x: xOffset, y: yOffset) - case .heightFill: - contentOffset = CGPoint(x: xOffset, y: 0) - case .widthFill: - contentOffset = CGPoint(x: 0, y: yOffset) - } - } - } - - private func setMaxMinZoomScalesForCurrentBounds() { - // calculate min/max zoomscale - let xScale = bounds.width / imageSize.width // the scale needed to perfectly fit the image width-wise - let yScale = bounds.height / imageSize.height // the scale needed to perfectly fit the image height-wise - - var minScale: CGFloat = 1 - - switch imageContentMode { - case .aspectFill: - minScale = max(xScale, yScale) - case .aspectFit: - minScale = min(xScale, yScale) - case .widthFill: - minScale = xScale - case .heightFill: - minScale = yScale - } - - - let maxScale = maxScaleFromMinScale*minScale - - // don't let minScale exceed maxScale. (If the image is smaller than the screen, we don't want to force it to be zoomed.) - if minScale > maxScale { - minScale = maxScale - } - - maximumZoomScale = maxScale - minimumZoomScale = minScale * 0.999 // the multiply factor to prevent user cannot scroll page while they use this control in UIPageViewController - } - - // MARK: - Gesture - - @objc func doubleTapGestureRecognizer(_ gestureRecognizer: UIGestureRecognizer) { - // zoom out if it bigger than middle scale point. Else, zoom in - if zoomScale >= maximumZoomScale / 2.0 { - setZoomScale(minimumZoomScale, animated: true) - } else { - let center = gestureRecognizer.location(in: gestureRecognizer.view) - let zoomRect = zoomRectForScale(ImageScrollView.kZoomInFactorFromMinWhenDoubleTap * minimumZoomScale, center: center) - zoom(to: zoomRect, animated: true) - } - } - - @objc func swipeUpGestureRecognizer(_ gestureRecognizer: UIGestureRecognizer) { + configureImageForSize(image.size) + } + + private func configureImageForSize(_ size: CGSize) { + imageSize = size + contentSize = imageSize + setMaxMinZoomScalesForCurrentBounds() + zoomScale = minimumZoomScale + + switch initialOffset { + case .begining: + contentOffset = CGPoint.zero + case .center: + let xOffset = contentSize.width < bounds.width ? 0 : (contentSize.width - bounds.width)/2 + let yOffset = contentSize.height < bounds.height ? 0 : (contentSize.height - bounds.height)/2 + + switch imageContentMode { + case .aspectFit: + contentOffset = CGPoint.zero + case .aspectFill: + contentOffset = CGPoint(x: xOffset, y: yOffset) + case .heightFill: + contentOffset = CGPoint(x: xOffset, y: 0) + case .widthFill: + contentOffset = CGPoint(x: 0, y: yOffset) + } + } + } + + private func setMaxMinZoomScalesForCurrentBounds() { + // calculate min/max zoomscale + let xScale = bounds.width / imageSize.width // the scale needed to perfectly fit the image width-wise + let yScale = bounds.height / imageSize.height // the scale needed to perfectly fit the image height-wise + + var minScale: CGFloat = 1 + + switch imageContentMode { + case .aspectFill: + minScale = max(xScale, yScale) + case .aspectFit: + minScale = min(xScale, yScale) + case .widthFill: + minScale = xScale + case .heightFill: + minScale = yScale + } + + + let maxScale = maxScaleFromMinScale*minScale + + // don't let minScale exceed maxScale. (If the image is smaller than the screen, we don't want to force it to be zoomed.) + if minScale > maxScale { + minScale = maxScale + } + + maximumZoomScale = maxScale + minimumZoomScale = minScale * 0.999 // the multiply factor to prevent user cannot scroll page while they use this control in UIPageViewController + } + + // MARK: - Gesture + + @objc func doubleTapGestureRecognizer(_ gestureRecognizer: UIGestureRecognizer) { + // zoom out if it bigger than middle scale point. Else, zoom in + if zoomScale >= maximumZoomScale / 2.0 { + setZoomScale(minimumZoomScale, animated: true) + } else { + let center = gestureRecognizer.location(in: gestureRecognizer.view) + let zoomRect = zoomRectForScale(ImageScrollView.kZoomInFactorFromMinWhenDoubleTap * minimumZoomScale, center: center) + zoom(to: zoomRect, animated: true) + } + } + + @objc func swipeUpGestureRecognizer(_ gestureRecognizer: UIGestureRecognizer) { if gestureRecognizer.state == .ended { imageScrollViewDelegate?.imageScrollViewDidGestureSwipeUp(imageScrollView: self) } } - - @objc func swipeDownGestureRecognizer(_ gestureRecognizer: UIGestureRecognizer) { + + @objc func swipeDownGestureRecognizer(_ gestureRecognizer: UIGestureRecognizer) { if gestureRecognizer.state == .ended { imageScrollViewDelegate?.imageScrollViewDidGestureSwipeDown(imageScrollView: self) } } - + private func zoomRectForScale(_ scale: CGFloat, center: CGPoint) -> CGRect { - var zoomRect = CGRect.zero - - // the zoom rect is in the content view's coordinates. - // at a zoom scale of 1.0, it would be the size of the imageScrollView's bounds. - // as the zoom scale decreases, so more content is visible, the size of the rect grows. - zoomRect.size.height = frame.size.height / scale - zoomRect.size.width = frame.size.width / scale - - // choose an origin so as to get the right center. - zoomRect.origin.x = center.x - (zoomRect.size.width / 2.0) - zoomRect.origin.y = center.y - (zoomRect.size.height / 2.0) - - return zoomRect - } - - open func refresh() { - if let image = zoomView?.image { - display(image: image) - } - } - - // MARK: - Actions - - @objc func changeOrientationNotification() { - // A weird bug that frames are not update right after orientation changed. Need delay a little bit with async. - DispatchQueue.main.async { - self.configureImageForSize(self.imageSize) - self.imageScrollViewDelegate?.imageScrollViewDidChangeOrientation(imageScrollView: self) - } - } + var zoomRect = CGRect.zero + + // the zoom rect is in the content view's coordinates. + // at a zoom scale of 1.0, it would be the size of the imageScrollView's bounds. + // as the zoom scale decreases, so more content is visible, the size of the rect grows. + zoomRect.size.height = frame.size.height / scale + zoomRect.size.width = frame.size.width / scale + + // choose an origin so as to get the right center. + zoomRect.origin.x = center.x - (zoomRect.size.width / 2.0) + zoomRect.origin.y = center.y - (zoomRect.size.height / 2.0) + + return zoomRect + } + + open func refresh() { + if let image = zoomView?.image { + display(image: image) + } + } + + open func resize() { + self.configureImageForSize(self.imageSize) + } } extension ImageScrollView: UIScrollViewDelegate { - - public func scrollViewDidScroll(_ scrollView: UIScrollView) { - imageScrollViewDelegate?.scrollViewDidScroll?(scrollView) - } - - public func scrollViewWillBeginDragging(_ scrollView: UIScrollView) { - imageScrollViewDelegate?.scrollViewWillBeginDragging?(scrollView) - } - - public func scrollViewWillEndDragging(_ scrollView: UIScrollView, withVelocity velocity: CGPoint, targetContentOffset: UnsafeMutablePointer) { - imageScrollViewDelegate?.scrollViewWillEndDragging?(scrollView, withVelocity: velocity, targetContentOffset: targetContentOffset) - } - - public func scrollViewDidEndDragging(_ scrollView: UIScrollView, willDecelerate decelerate: Bool) { - imageScrollViewDelegate?.scrollViewDidEndDragging?(scrollView, willDecelerate: decelerate) - } - - public func scrollViewWillBeginDecelerating(_ scrollView: UIScrollView) { - imageScrollViewDelegate?.scrollViewWillBeginDecelerating?(scrollView) - } - - public func scrollViewDidEndDecelerating(_ scrollView: UIScrollView) { - imageScrollViewDelegate?.scrollViewDidEndDecelerating?(scrollView) - } - - public func scrollViewDidEndScrollingAnimation(_ scrollView: UIScrollView) { - imageScrollViewDelegate?.scrollViewDidEndScrollingAnimation?(scrollView) - } - - public func scrollViewWillBeginZooming(_ scrollView: UIScrollView, with view: UIView?) { - imageScrollViewDelegate?.scrollViewWillBeginZooming?(scrollView, with: view) - } - - public func scrollViewDidEndZooming(_ scrollView: UIScrollView, with view: UIView?, atScale scale: CGFloat) { - imageScrollViewDelegate?.scrollViewDidEndZooming?(scrollView, with: view, atScale: scale) - } - - public func scrollViewShouldScrollToTop(_ scrollView: UIScrollView) -> Bool { - return false - } - - @available(iOS 11.0, *) - public func scrollViewDidChangeAdjustedContentInset(_ scrollView: UIScrollView) { - imageScrollViewDelegate?.scrollViewDidChangeAdjustedContentInset?(scrollView) - } - - public func viewForZooming(in scrollView: UIScrollView) -> UIView? { - return zoomView - } - - public func scrollViewDidZoom(_ scrollView: UIScrollView) { - adjustFrameToCenter() - imageScrollViewDelegate?.scrollViewDidZoom?(scrollView) - } - + + public func scrollViewDidScroll(_ scrollView: UIScrollView) { + imageScrollViewDelegate?.scrollViewDidScroll?(scrollView) + } + + public func scrollViewWillBeginDragging(_ scrollView: UIScrollView) { + imageScrollViewDelegate?.scrollViewWillBeginDragging?(scrollView) + } + + public func scrollViewWillEndDragging(_ scrollView: UIScrollView, withVelocity velocity: CGPoint, targetContentOffset: UnsafeMutablePointer) { + imageScrollViewDelegate?.scrollViewWillEndDragging?(scrollView, withVelocity: velocity, targetContentOffset: targetContentOffset) + } + + public func scrollViewDidEndDragging(_ scrollView: UIScrollView, willDecelerate decelerate: Bool) { + imageScrollViewDelegate?.scrollViewDidEndDragging?(scrollView, willDecelerate: decelerate) + } + + public func scrollViewWillBeginDecelerating(_ scrollView: UIScrollView) { + imageScrollViewDelegate?.scrollViewWillBeginDecelerating?(scrollView) + } + + public func scrollViewDidEndDecelerating(_ scrollView: UIScrollView) { + imageScrollViewDelegate?.scrollViewDidEndDecelerating?(scrollView) + } + + public func scrollViewDidEndScrollingAnimation(_ scrollView: UIScrollView) { + imageScrollViewDelegate?.scrollViewDidEndScrollingAnimation?(scrollView) + } + + public func scrollViewWillBeginZooming(_ scrollView: UIScrollView, with view: UIView?) { + imageScrollViewDelegate?.scrollViewWillBeginZooming?(scrollView, with: view) + } + + public func scrollViewDidEndZooming(_ scrollView: UIScrollView, with view: UIView?, atScale scale: CGFloat) { + imageScrollViewDelegate?.scrollViewDidEndZooming?(scrollView, with: view, atScale: scale) + } + + public func scrollViewShouldScrollToTop(_ scrollView: UIScrollView) -> Bool { + return false + } + + @available(iOS 11.0, *) + public func scrollViewDidChangeAdjustedContentInset(_ scrollView: UIScrollView) { + imageScrollViewDelegate?.scrollViewDidChangeAdjustedContentInset?(scrollView) + } + + public func viewForZooming(in scrollView: UIScrollView) -> UIView? { + return zoomView + } + + public func scrollViewDidZoom(_ scrollView: UIScrollView) { + adjustFrameToCenter() + imageScrollViewDelegate?.scrollViewDidZoom?(scrollView) + } + } diff --git a/iOS/Article/ImageViewController.swift b/iOS/Article/ImageViewController.swift index b1c40a153..d837b4d4b 100644 --- a/iOS/Article/ImageViewController.swift +++ b/iOS/Article/ImageViewController.swift @@ -28,6 +28,13 @@ class ImageViewController: UIViewController { imageScrollView.display(image: image) } + override func viewWillTransition(to size: CGSize, with coordinator: UIViewControllerTransitionCoordinator) { + super.viewWillTransition(to: size, with: coordinator) + coordinator.animate(alongsideTransition: { [weak self] context in + self?.imageScrollView.resize() + }) + } + @IBAction func share(_ sender: Any) { guard let image = image else { return } let activityViewController = UIActivityViewController(activityItems: [image], applicationActivities: nil) @@ -46,9 +53,6 @@ class ImageViewController: UIViewController { extension ImageViewController: ImageScrollViewDelegate { - func imageScrollViewDidChangeOrientation(imageScrollView: ImageScrollView) { - } - func imageScrollViewDidGestureSwipeUp(imageScrollView: ImageScrollView) { dismiss(animated: true) } diff --git a/iOS/RootSplitViewController.swift b/iOS/RootSplitViewController.swift index 89fb623f3..cf7040c42 100644 --- a/iOS/RootSplitViewController.swift +++ b/iOS/RootSplitViewController.swift @@ -14,6 +14,7 @@ class RootSplitViewController: UISplitViewController { var coordinator: SceneCoordinator! override func viewWillTransition(to size: CGSize, with coordinator: UIViewControllerTransitionCoordinator) { + super.viewWillTransition(to: size, with: coordinator) coordinator.animate(alongsideTransition: { [weak self] context in self?.coordinator.configureThreePanelMode(for: size) }) diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index a647e5b35..17c0dd94a 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -819,6 +819,14 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { masterFeedViewController.present(addViewController, animated: true) } + func showFullScreenImage(image: UIImage, transitioningDelegate: UIViewControllerTransitioningDelegate) { + let imageVC = UIStoryboard.main.instantiateController(ofType: ImageViewController.self) + imageVC.image = image + imageVC.modalPresentationStyle = .currentContext + imageVC.transitioningDelegate = transitioningDelegate + rootSplitViewController.present(imageVC, animated: true) + } + func toggleArticleExtractor() { guard let article = currentArticle else { From 76d7c0256a19740e177fa3ff5db0b866d2e6a7d7 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 16 Oct 2019 20:53:49 -0500 Subject: [PATCH 09/13] Fix full screen image background color for dark mode --- iOS/AppAssets.swift | 4 +++ iOS/Article/ImageTransition.swift | 2 +- iOS/Base.lproj/Main.storyboard | 5 ++- .../Contents.json | 31 +++++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 iOS/Resources/Assets.xcassets/fullScreenBackgroundColor.colorset/Contents.json diff --git a/iOS/AppAssets.swift b/iOS/AppAssets.swift index a31ac6ea5..2ca4fda10 100644 --- a/iOS/AppAssets.swift +++ b/iOS/AppAssets.swift @@ -68,6 +68,10 @@ struct AppAssets { return RSImage(named: "faviconTemplateImage")! }() + static var fullScreenBackgroundColor: UIColor = { + return UIColor(named: "fullScreenBackgroundColor")! + }() + static var infoImage: UIImage = { UIImage(systemName: "info.circle")! }() diff --git a/iOS/Article/ImageTransition.swift b/iOS/Article/ImageTransition.swift index 17a14c926..0c17a59b9 100644 --- a/iOS/Article/ImageTransition.swift +++ b/iOS/Article/ImageTransition.swift @@ -41,7 +41,7 @@ class ImageTransition: NSObject, UIViewControllerAnimatedTransitioning { let fromView = transitionContext.view(forKey: .from)! fromView.removeFromSuperview() - transitionContext.containerView.backgroundColor = UIColor.systemBackground + transitionContext.containerView.backgroundColor = AppAssets.fullScreenBackgroundColor transitionContext.containerView.addSubview(imageView) articleController?.hideClickedImage() diff --git a/iOS/Base.lproj/Main.storyboard b/iOS/Base.lproj/Main.storyboard index 14b53813d..df20db9ac 100644 --- a/iOS/Base.lproj/Main.storyboard +++ b/iOS/Base.lproj/Main.storyboard @@ -265,7 +265,7 @@ - + @@ -299,6 +299,9 @@ + + + diff --git a/iOS/Resources/Assets.xcassets/fullScreenBackgroundColor.colorset/Contents.json b/iOS/Resources/Assets.xcassets/fullScreenBackgroundColor.colorset/Contents.json new file mode 100644 index 000000000..ccbb901d9 --- /dev/null +++ b/iOS/Resources/Assets.xcassets/fullScreenBackgroundColor.colorset/Contents.json @@ -0,0 +1,31 @@ +{ + "info" : { + "version" : 1, + "author" : "xcode" + }, + "colors" : [ + { + "idiom" : "universal", + "color" : { + "platform" : "ios", + "reference" : "systemBackgroundColor" + } + }, + { + "idiom" : "universal", + "appearances" : [ + { + "appearance" : "luminosity", + "value" : "dark" + } + ], + "color" : { + "color-space" : "gray-gamma-22", + "components" : { + "white" : "0.000", + "alpha" : "1.000" + } + } + } + ] +} \ No newline at end of file From ae014375ed3f1567b16af479a534897151269ca9 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Thu, 17 Oct 2019 01:23:00 -0400 Subject: [PATCH 10/13] QueryItem helper --- .../Account/Account.xcodeproj/project.pbxproj | 4 +++ Frameworks/Account/URL+Extensions.swift | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 Frameworks/Account/URL+Extensions.swift diff --git a/Frameworks/Account/Account.xcodeproj/project.pbxproj b/Frameworks/Account/Account.xcodeproj/project.pbxproj index 0f04f4032..8f7abd3ee 100644 --- a/Frameworks/Account/Account.xcodeproj/project.pbxproj +++ b/Frameworks/Account/Account.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + 3BF611F223583530000EF978 /* URL+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3BF611E62358352F000EF978 /* URL+Extensions.swift */; }; 5107A099227DE42E00C7C3C5 /* AccountCredentialsTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5107A098227DE42E00C7C3C5 /* AccountCredentialsTest.swift */; }; 5107A09B227DE49500C7C3C5 /* TestAccountManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5107A09A227DE49500C7C3C5 /* TestAccountManager.swift */; }; 5107A09D227DE77700C7C3C5 /* TestTransport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5107A09C227DE77700C7C3C5 /* TestTransport.swift */; }; @@ -185,6 +186,7 @@ /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ + 3BF611E62358352F000EF978 /* URL+Extensions.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "URL+Extensions.swift"; sourceTree = ""; }; 5107A098227DE42E00C7C3C5 /* AccountCredentialsTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountCredentialsTest.swift; sourceTree = ""; }; 5107A09A227DE49500C7C3C5 /* TestAccountManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestAccountManager.swift; sourceTree = ""; }; 5107A09C227DE77700C7C3C5 /* TestTransport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestTransport.swift; sourceTree = ""; }; @@ -480,6 +482,7 @@ 841974001F6DD1EC006346C4 /* Folder.swift */, 844B297E210CE37E004020B3 /* UnreadCountProvider.swift */, 51FE1007234635A20056195D /* DeepLinkProvider.swift */, + 3BF611E62358352F000EF978 /* URL+Extensions.swift */, 5165D71F22835E9800D9D53D /* FeedFinder */, 515E4EB12324FF7D0057B0E7 /* Credentials */, 8419742B1F6DDE84006346C4 /* LocalAccount */, @@ -909,6 +912,7 @@ 9E1773D5234570E30056A5A8 /* FeedlyEntryParser.swift in Sources */, 9E1D1555233431A600F4944C /* FeedlyOperation.swift in Sources */, 9E1AF38B2353D41A008BD1D5 /* FeedlySetStarredArticlesOperation.swift in Sources */, + 3BF611F223583530000EF978 /* URL+Extensions.swift in Sources */, 84F1F06E2243524700DA0616 /* AccountMetadata.swift in Sources */, 84245C851FDDD8CB0074AFBB /* FeedbinSubscription.swift in Sources */, ); diff --git a/Frameworks/Account/URL+Extensions.swift b/Frameworks/Account/URL+Extensions.swift new file mode 100644 index 000000000..69a048ac8 --- /dev/null +++ b/Frameworks/Account/URL+Extensions.swift @@ -0,0 +1,30 @@ +// +// URL+Extensions.swift +// Account +// +// Created by Jonathan Bennett on 2019-10-16. +// Copyright © 2019 Ranchero Software, LLC. All rights reserved. +// + +import Foundation + + +public extension URL { + + func appendingQueryItem(_ queryItem: URLQueryItem) -> URL? { + appendingQueryItems([queryItem]) + } + + func appendingQueryItems(_ queryItems: [URLQueryItem]) -> URL? { + guard var components = URLComponents(url: self, resolvingAgainstBaseURL: false) else { + return nil + } + + var newQueryItems = components.queryItems ?? [] + newQueryItems.append(contentsOf: queryItems) + components.queryItems = newQueryItems + + return components.url + } + +} From d70c996c06146d3d910b5e1e006610257b2d4f70 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Thu, 17 Oct 2019 01:56:42 -0400 Subject: [PATCH 11/13] use the URLQueryItem helper --- .../Account/Feedbin/FeedbinAPICaller.swift | 32 +++-- .../Account/ReaderAPI/ReaderAPICaller.swift | 114 +++++++----------- 2 files changed, 64 insertions(+), 82 deletions(-) diff --git a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift index 8f46c00bc..ff8cd03e0 100644 --- a/Frameworks/Account/Feedbin/FeedbinAPICaller.swift +++ b/Frameworks/Account/Feedbin/FeedbinAPICaller.swift @@ -339,9 +339,13 @@ final class FeedbinAPICaller: NSObject { let concatIDs = articleIDs.reduce("") { param, articleID in return param + ",\(articleID)" } let paramIDs = String(concatIDs.dropFirst()) - var callComponents = URLComponents(url: feedbinBaseURL.appendingPathComponent("entries.json"), resolvingAgainstBaseURL: false)! - callComponents.queryItems = [URLQueryItem(name: "ids", value: paramIDs), URLQueryItem(name: "mode", value: "extended")] - let request = URLRequest(url: callComponents.url!, credentials: credentials) + let url = feedbinBaseURL + .appendingPathComponent("entries.json") + .appendingQueryItems([ + URLQueryItem(name: "ids", value: paramIDs), + URLQueryItem(name: "mode", value: "extended") + ]) + let request = URLRequest(url: url!, credentials: credentials) transport.send(request: request, resultType: [FeedbinEntry].self) { result in @@ -361,9 +365,14 @@ final class FeedbinAPICaller: NSObject { let since = Calendar.current.date(byAdding: .month, value: -3, to: Date()) ?? Date() let sinceString = FeedbinDate.formatter.string(from: since) - var callComponents = URLComponents(url: feedbinBaseURL.appendingPathComponent("feeds/\(feedID)/entries.json"), resolvingAgainstBaseURL: false)! - callComponents.queryItems = [URLQueryItem(name: "since", value: sinceString), URLQueryItem(name: "per_page", value: "100"), URLQueryItem(name: "mode", value: "extended")] - let request = URLRequest(url: callComponents.url!, credentials: credentials) + let url = feedbinBaseURL + .appendingPathComponent("feeds/\(feedID)/entries.json") + .appendingQueryItems([ + URLQueryItem(name: "since", value: sinceString), + URLQueryItem(name: "per_page", value: "100"), + URLQueryItem(name: "mode", value: "extended") + ]) + let request = URLRequest(url: url!, credentials: credentials) transport.send(request: request, resultType: [FeedbinEntry].self) { result in @@ -392,9 +401,14 @@ final class FeedbinAPICaller: NSObject { }() let sinceString = FeedbinDate.formatter.string(from: since) - var callComponents = URLComponents(url: feedbinBaseURL.appendingPathComponent("entries.json"), resolvingAgainstBaseURL: false)! - callComponents.queryItems = [URLQueryItem(name: "since", value: sinceString), URLQueryItem(name: "per_page", value: "100"), URLQueryItem(name: "mode", value: "extended")] - let request = URLRequest(url: callComponents.url!, credentials: credentials) + let url = feedbinBaseURL + .appendingPathComponent("entries.json") + .appendingQueryItems([ + URLQueryItem(name: "since", value: sinceString), + URLQueryItem(name: "per_page", value: "100"), + URLQueryItem(name: "mode", value: "extended") + ]) + let request = URLRequest(url: url!, credentials: credentials) transport.send(request: request, resultType: [FeedbinEntry].self) { result in diff --git a/Frameworks/Account/ReaderAPI/ReaderAPICaller.swift b/Frameworks/Account/ReaderAPI/ReaderAPICaller.swift index 23499a9c0..ba1234a81 100644 --- a/Frameworks/Account/ReaderAPI/ReaderAPICaller.swift +++ b/Frameworks/Account/ReaderAPI/ReaderAPICaller.swift @@ -166,17 +166,11 @@ final class ReaderAPICaller: NSObject { return } - // Add query string for getting JSON (probably should break this out as I will be doing it a lot) - guard var components = URLComponents(url: baseURL.appendingPathComponent(ReaderAPIEndpoints.tagList.rawValue), resolvingAgainstBaseURL: false) else { - completion(.failure(TransportError.noURL)) - return - } + let url = baseURL + .appendingPathComponent(ReaderAPIEndpoints.tagList.rawValue) + .appendingQueryItem(URLQueryItem(name: "output", value: "json")) - components.queryItems = [ - URLQueryItem(name: "output", value: "json") - ] - - guard let callURL = components.url else { + guard let callURL = url else { completion(.failure(TransportError.noURL)) return } @@ -278,17 +272,11 @@ final class ReaderAPICaller: NSObject { return } - // Add query string for getting JSON (probably should break this out as I will be doing it a lot) - guard var components = URLComponents(url: baseURL.appendingPathComponent(ReaderAPIEndpoints.subscriptionList.rawValue), resolvingAgainstBaseURL: false) else { - completion(.failure(TransportError.noURL)) - return - } + let url = baseURL + .appendingPathComponent(ReaderAPIEndpoints.subscriptionList.rawValue) + .appendingQueryItem(URLQueryItem(name: "output", value: "json")) - components.queryItems = [ - URLQueryItem(name: "output", value: "json") - ] - - guard let callURL = components.url else { + guard let callURL = url else { completion(.failure(TransportError.noURL)) return } @@ -333,16 +321,11 @@ final class ReaderAPICaller: NSObject { self.requestAuthorizationToken(endpoint: baseURL) { (result) in switch result { case .success(let token): - guard var components = URLComponents(url: baseURL.appendingPathComponent(ReaderAPIEndpoints.subscriptionAdd.rawValue), resolvingAgainstBaseURL: false) else { - completion(.failure(TransportError.noURL)) - return - } + let url = baseURL + .appendingPathComponent(ReaderAPIEndpoints.subscriptionAdd.rawValue) + .appendingQueryItem(URLQueryItem(name: "quickadd", value: url.absoluteString)) - components.queryItems = [ - URLQueryItem(name: "quickadd", value: url.absoluteString) - ] - - guard let callURL = components.url else { + guard let callURL = url else { completion(.failure(TransportError.noURL)) return } @@ -616,19 +599,15 @@ final class ReaderAPICaller: NSObject { return } - // Add query string for getting JSON (probably should break this out as I will be doing it a lot) - guard var components = URLComponents(url: baseURL.appendingPathComponent(ReaderAPIEndpoints.itemIds.rawValue), resolvingAgainstBaseURL: false) else { - completion(.failure(TransportError.noURL)) - return - } + let url = baseURL + .appendingPathComponent(ReaderAPIEndpoints.itemIds.rawValue) + .appendingQueryItems([ + URLQueryItem(name: "s", value: feedID), + URLQueryItem(name: "ot", value: String(since.timeIntervalSince1970)), + URLQueryItem(name: "output", value: "json") + ]) - components.queryItems = [ - URLQueryItem(name: "s", value: feedID), - URLQueryItem(name: "ot", value: String(since.timeIntervalSince1970)), - URLQueryItem(name: "output", value: "json") - ] - - guard let callURL = components.url else { + guard let callURL = url else { completion(.failure(TransportError.noURL)) return } @@ -684,22 +663,17 @@ final class ReaderAPICaller: NSObject { }() let sinceString = since.timeIntervalSince1970 + let url = baseURL + .appendingPathComponent(ReaderAPIEndpoints.itemIds.rawValue) + .appendingQueryItems([ + URLQueryItem(name: "o", value: String(sinceString)), + URLQueryItem(name: "n", value: "10000"), + URLQueryItem(name: "output", value: "json"), + URLQueryItem(name: "xt", value: ReaderState.read.rawValue), + URLQueryItem(name: "s", value: ReaderStreams.readingList.rawValue) + ]) - // Add query string for getting JSON (probably should break this out as I will be doing it a lot) - guard var components = URLComponents(url: baseURL.appendingPathComponent(ReaderAPIEndpoints.itemIds.rawValue), resolvingAgainstBaseURL: false) else { - completion(.failure(TransportError.noURL)) - return - } - - components.queryItems = [ - URLQueryItem(name: "o", value: String(sinceString)), - URLQueryItem(name: "n", value: "10000"), - URLQueryItem(name: "output", value: "json"), - URLQueryItem(name: "xt", value: ReaderState.read.rawValue), - URLQueryItem(name: "s", value: ReaderStreams.readingList.rawValue) - ] - - guard let callURL = components.url else { + guard let callURL = url else { completion(.failure(TransportError.noURL)) return } @@ -768,13 +742,11 @@ final class ReaderAPICaller: NSObject { func retrieveEntries(page: String, completion: @escaping (Result<([ReaderAPIEntry]?, String?), Error>) -> Void) { - guard let url = URL(string: page), var callComponents = URLComponents(url: url, resolvingAgainstBaseURL: false) else { + guard let url = URL(string: page)?.appendingQueryItem(URLQueryItem(name: "mode", value: "extended")) else { completion(.success((nil, nil))) return } - - callComponents.queryItems?.append(URLQueryItem(name: "mode", value: "extended")) - let request = URLRequest(url: callComponents.url!, credentials: credentials) + let request = URLRequest(url: url, credentials: credentials) transport.send(request: request, resultType: [ReaderAPIEntry].self) { result in @@ -800,20 +772,16 @@ final class ReaderAPICaller: NSObject { return } - // Add query string for getting JSON (probably should break this out as I will be doing it a lot) - guard var components = URLComponents(url: baseURL.appendingPathComponent(ReaderAPIEndpoints.itemIds.rawValue), resolvingAgainstBaseURL: false) else { - completion(.failure(TransportError.noURL)) - return - } + let url = baseURL + .appendingPathComponent(ReaderAPIEndpoints.itemIds.rawValue) + .appendingQueryItems([ + URLQueryItem(name: "s", value: ReaderStreams.readingList.rawValue), + URLQueryItem(name: "n", value: "10000"), + URLQueryItem(name: "xt", value: ReaderState.read.rawValue), + URLQueryItem(name: "output", value: "json") + ]) - components.queryItems = [ - URLQueryItem(name: "s", value: ReaderStreams.readingList.rawValue), - URLQueryItem(name: "n", value: "10000"), - URLQueryItem(name: "xt", value: ReaderState.read.rawValue), - URLQueryItem(name: "output", value: "json") - ] - - guard let callURL = components.url else { + guard let callURL = url else { completion(.failure(TransportError.noURL)) return } From 540320e6c05df9c8e5c10b4c1a18dc9bc642740f Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 17 Oct 2019 05:54:27 -0500 Subject: [PATCH 12/13] Change to use navigation stack instead of modal for account credential changes --- .../Account/SettingsDetailAccountView.swift | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/iOS/Settings/Account/SettingsDetailAccountView.swift b/iOS/Settings/Account/SettingsDetailAccountView.swift index d1c027a89..f67b29833 100644 --- a/iOS/Settings/Account/SettingsDetailAccountView.swift +++ b/iOS/Settings/Account/SettingsDetailAccountView.swift @@ -14,7 +14,7 @@ import RSWeb struct SettingsDetailAccountView : View { @Environment(\.presentationMode) var presentation @ObservedObject var viewModel: ViewModel - @State private var accountType: AccountType = nil + @State private var credentialsAction: Int? = nil @State private var isDeleteAlertPresented = false var body: some View { @@ -28,20 +28,21 @@ struct SettingsDetailAccountView : View { } } if viewModel.isCreditialsAvailable { - Section { - Button(action: { - self.accountType = self.viewModel.account.type - }) { + if viewModel.account.type == .feedbin { + NavigationLink(destination: self.settingsFeedbinAccountView, tag: 1, selection: $credentialsAction) { Text("Credentials") } + .modifier(VibrantSelectAction(action: { + self.credentialsAction = 1 + })) } - .sheet(item: $accountType) { type in - if type == .feedbin { - self.settingsFeedbinAccountView - } - if type == .freshRSS { - self.settingsReaderAPIAccountView + if viewModel.account.type == .freshRSS { + NavigationLink(destination: self.settingsReaderAPIAccountView, tag: 1, selection: $credentialsAction) { + Text("Credentials") } + .modifier(VibrantSelectAction(action: { + self.credentialsAction = 1 + })) } } if viewModel.isDeletable { From 579717dd861ca1f23ceb5cd9f13426c5569694a0 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Thu, 17 Oct 2019 06:01:08 -0500 Subject: [PATCH 13/13] Move extensions for compositing query items to RSWeb --- .../Account/Account.xcodeproj/project.pbxproj | 4 --- Frameworks/Account/URL+Extensions.swift | 30 ------------------- submodules/RSWeb | 2 +- 3 files changed, 1 insertion(+), 35 deletions(-) delete mode 100644 Frameworks/Account/URL+Extensions.swift diff --git a/Frameworks/Account/Account.xcodeproj/project.pbxproj b/Frameworks/Account/Account.xcodeproj/project.pbxproj index 8f7abd3ee..0f04f4032 100644 --- a/Frameworks/Account/Account.xcodeproj/project.pbxproj +++ b/Frameworks/Account/Account.xcodeproj/project.pbxproj @@ -7,7 +7,6 @@ objects = { /* Begin PBXBuildFile section */ - 3BF611F223583530000EF978 /* URL+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3BF611E62358352F000EF978 /* URL+Extensions.swift */; }; 5107A099227DE42E00C7C3C5 /* AccountCredentialsTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5107A098227DE42E00C7C3C5 /* AccountCredentialsTest.swift */; }; 5107A09B227DE49500C7C3C5 /* TestAccountManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5107A09A227DE49500C7C3C5 /* TestAccountManager.swift */; }; 5107A09D227DE77700C7C3C5 /* TestTransport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5107A09C227DE77700C7C3C5 /* TestTransport.swift */; }; @@ -186,7 +185,6 @@ /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ - 3BF611E62358352F000EF978 /* URL+Extensions.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "URL+Extensions.swift"; sourceTree = ""; }; 5107A098227DE42E00C7C3C5 /* AccountCredentialsTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AccountCredentialsTest.swift; sourceTree = ""; }; 5107A09A227DE49500C7C3C5 /* TestAccountManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestAccountManager.swift; sourceTree = ""; }; 5107A09C227DE77700C7C3C5 /* TestTransport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestTransport.swift; sourceTree = ""; }; @@ -482,7 +480,6 @@ 841974001F6DD1EC006346C4 /* Folder.swift */, 844B297E210CE37E004020B3 /* UnreadCountProvider.swift */, 51FE1007234635A20056195D /* DeepLinkProvider.swift */, - 3BF611E62358352F000EF978 /* URL+Extensions.swift */, 5165D71F22835E9800D9D53D /* FeedFinder */, 515E4EB12324FF7D0057B0E7 /* Credentials */, 8419742B1F6DDE84006346C4 /* LocalAccount */, @@ -912,7 +909,6 @@ 9E1773D5234570E30056A5A8 /* FeedlyEntryParser.swift in Sources */, 9E1D1555233431A600F4944C /* FeedlyOperation.swift in Sources */, 9E1AF38B2353D41A008BD1D5 /* FeedlySetStarredArticlesOperation.swift in Sources */, - 3BF611F223583530000EF978 /* URL+Extensions.swift in Sources */, 84F1F06E2243524700DA0616 /* AccountMetadata.swift in Sources */, 84245C851FDDD8CB0074AFBB /* FeedbinSubscription.swift in Sources */, ); diff --git a/Frameworks/Account/URL+Extensions.swift b/Frameworks/Account/URL+Extensions.swift deleted file mode 100644 index 69a048ac8..000000000 --- a/Frameworks/Account/URL+Extensions.swift +++ /dev/null @@ -1,30 +0,0 @@ -// -// URL+Extensions.swift -// Account -// -// Created by Jonathan Bennett on 2019-10-16. -// Copyright © 2019 Ranchero Software, LLC. All rights reserved. -// - -import Foundation - - -public extension URL { - - func appendingQueryItem(_ queryItem: URLQueryItem) -> URL? { - appendingQueryItems([queryItem]) - } - - func appendingQueryItems(_ queryItems: [URLQueryItem]) -> URL? { - guard var components = URLComponents(url: self, resolvingAgainstBaseURL: false) else { - return nil - } - - var newQueryItems = components.queryItems ?? [] - newQueryItems.append(contentsOf: queryItems) - components.queryItems = newQueryItems - - return components.url - } - -} diff --git a/submodules/RSWeb b/submodules/RSWeb index 06ba2a25f..1ea5f5ccf 160000 --- a/submodules/RSWeb +++ b/submodules/RSWeb @@ -1 +1 @@ -Subproject commit 06ba2a25fc15b016d2f5c41230e0ed9fe5feab25 +Subproject commit 1ea5f5ccfc3646ffdf2891abbc5ea63e3d449def