From eb1082c56415163879544dab40c6f0676f1ace1a Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Thu, 3 Jan 2019 00:40:11 +0530 Subject: [PATCH 01/12] Corrected constraints relationships. Previously attached to child of sibling. --- Client/Frontend/Browser/BrowserViewController.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Client/Frontend/Browser/BrowserViewController.swift b/Client/Frontend/Browser/BrowserViewController.swift index 29d1b4a95..0eec6eaf8 100644 --- a/Client/Frontend/Browser/BrowserViewController.swift +++ b/Client/Frontend/Browser/BrowserViewController.swift @@ -667,8 +667,8 @@ class BrowserViewController: UIViewController { webViewContainerTopOffset = make.top.equalTo(readerModeBar?.snp.bottom ?? self.header.snp.bottom).constraint let findInPageHeight = (findInPageBar == nil) ? 0 : UIConstants.ToolbarHeight - if let toolbar = self.toolbar { - make.bottom.equalTo(toolbar.snp.top).offset(-findInPageHeight) + if let footerView = self.footer { + make.bottom.equalTo(footerView.snp.top).offset(-findInPageHeight) } else { make.bottom.equalTo(self.view).offset(-findInPageHeight) } @@ -694,7 +694,7 @@ class BrowserViewController: UIViewController { make.left.right.equalTo(self.view) if self.homePanelIsInline { - make.bottom.equalTo(self.toolbar?.snp.top ?? self.view.snp.bottom) + make.bottom.equalTo(self.footer?.snp.top ?? self.view.snp.bottom) } else { make.bottom.equalTo(self.view.snp.bottom) } @@ -705,8 +705,8 @@ class BrowserViewController: UIViewController { make.width.equalTo(self.view.snp.width) if let keyboardHeight = keyboardState?.intersectionHeightForView(self.view), keyboardHeight > 0 { make.bottom.equalTo(self.view).offset(-keyboardHeight) - } else if let toolbar = self.toolbar { - make.bottom.equalTo(toolbar.snp.top) + } else if let footer = self.footer { + make.bottom.equalTo(footer.snp.top) } else { make.bottom.equalTo(self.view) } From 77926dafabec791d73bb5407c6d90c9c738bed3f Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Thu, 3 Jan 2019 00:41:16 +0530 Subject: [PATCH 02/12] Removed redundant creation of pan gesture when one already exists for scrollview. --- Client/Frontend/Browser/TabScrollController.swift | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/Client/Frontend/Browser/TabScrollController.swift b/Client/Frontend/Browser/TabScrollController.swift index 0d7ed300c..f7c315185 100644 --- a/Client/Frontend/Browser/TabScrollController.swift +++ b/Client/Frontend/Browser/TabScrollController.swift @@ -22,11 +22,12 @@ class TabScrollingController: NSObject { weak var tab: Tab? { willSet { self.scrollView?.delegate = nil - self.scrollView?.removeGestureRecognizer(panGesture) + self.scrollView?.panGestureRecognizer.removeTarget(self, action: nil) } didSet { - self.scrollView?.addGestureRecognizer(panGesture) + self.scrollView?.panGestureRecognizer.addTarget(self, action: #selector(handlePan)) + self.scrollView?.panGestureRecognizer.maximumNumberOfTouches = 1 scrollView?.delegate = self } } @@ -70,13 +71,6 @@ class TabScrollingController: NSObject { } } - fileprivate lazy var panGesture: UIPanGestureRecognizer = { - let panGesture = UIPanGestureRecognizer(target: self, action: #selector(handlePan)) - panGesture.maximumNumberOfTouches = 1 - panGesture.delegate = self - return panGesture - }() - fileprivate var scrollView: UIScrollView? { return tab?.webView?.scrollView } fileprivate var contentOffset: CGPoint { return scrollView?.contentOffset ?? .zero } fileprivate var contentSize: CGSize { return scrollView?.contentSize ?? .zero } From 083e02ffb856b9c5e0925f84ecf56b46c31e638b Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Thu, 3 Jan 2019 00:44:05 +0530 Subject: [PATCH 03/12] Corrected the logic for offset when using iPad(shows tabsbar). The `headerTopOffset` is already clamped by `-topScrollHeight` in scrollWithDelta, where topScrollHeight contains both urlView and tabsBarView. --- Client/Frontend/Browser/TabScrollController.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Client/Frontend/Browser/TabScrollController.swift b/Client/Frontend/Browser/TabScrollController.swift index f7c315185..379dd3ac9 100644 --- a/Client/Frontend/Browser/TabScrollController.swift +++ b/Client/Frontend/Browser/TabScrollController.swift @@ -53,7 +53,7 @@ class TabScrollingController: NSObject { fileprivate var headerTopOffset: CGFloat = 0 { didSet { - headerTopConstraint?.update(offset: headerTopOffset - tabsBarOffset) + headerTopConstraint?.update(offset: headerTopOffset) header?.superview?.setNeedsLayout() } } @@ -257,7 +257,7 @@ private extension TabScrollingController { if isShownFromHidden { scrollView.contentOffset = CGPoint(x: initialContentOffset.x, y: initialContentOffset.y + self.topScrollHeight) } - self.headerTopOffset = headerOffset + self.tabsBarOffset + self.headerTopOffset = headerOffset self.footerBottomOffset = footerOffset self.urlBar?.updateAlphaForSubviews(alpha) self.tabsBar?.view.alpha = alpha From baed29f2c8a3faf2b396cf6bfc046b7950d4a30c Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Thu, 3 Jan 2019 00:46:54 +0530 Subject: [PATCH 04/12] Improved animation as per apple guidelines. --- .../Frontend/Browser/TabScrollController.swift | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Client/Frontend/Browser/TabScrollController.swift b/Client/Frontend/Browser/TabScrollController.swift index 379dd3ac9..fee884b50 100644 --- a/Client/Frontend/Browser/TabScrollController.swift +++ b/Client/Frontend/Browser/TabScrollController.swift @@ -253,19 +253,19 @@ private extension TabScrollingController { // produce a ~50px page jumping effect in response to tap navigations. let isShownFromHidden = headerTopOffset == -topScrollHeight && headerOffset == 0 + if isShownFromHidden { + scrollView.contentOffset = CGPoint(x: initialContentOffset.x, y: initialContentOffset.y + self.topScrollHeight) + } + self.headerTopOffset = headerOffset + self.footerBottomOffset = footerOffset + self.urlBar?.updateAlphaForSubviews(alpha) + self.tabsBar?.view.alpha = alpha let animation: () -> Void = { - if isShownFromHidden { - scrollView.contentOffset = CGPoint(x: initialContentOffset.x, y: initialContentOffset.y + self.topScrollHeight) - } - self.headerTopOffset = headerOffset - self.footerBottomOffset = footerOffset - self.urlBar?.updateAlphaForSubviews(alpha) - self.tabsBar?.view.alpha = alpha self.header?.superview?.layoutIfNeeded() } if animated { - UIView.animate(withDuration: duration, delay: 0, options: .allowUserInteraction, animations: animation, completion: completion) + UIView.animate(withDuration: duration, delay: 0, options: [.beginFromCurrentState, .allowUserInteraction], animations: animation, completion: completion) } else { animation() completion?(true) From 1400373251aa728925d02978056bca3e1433796a Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Thu, 3 Jan 2019 02:46:40 +0530 Subject: [PATCH 05/12] Removed unnecessary setting of maxTouch on scrollView and enabled Zoom for WKWebview. --- Client/Frontend/Browser/BraveWebView.swift | 2 ++ Client/Frontend/Browser/TabScrollController.swift | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Client/Frontend/Browser/BraveWebView.swift b/Client/Frontend/Browser/BraveWebView.swift index e2a6fdbd7..9bc6c019b 100644 --- a/Client/Frontend/Browser/BraveWebView.swift +++ b/Client/Frontend/Browser/BraveWebView.swift @@ -13,6 +13,8 @@ class BraveWebView: WKWebView { configuration.websiteDataStore = WKWebsiteDataStore.nonPersistent() } + // Enables Zoom in website by ignoring their javascript based viewport Scale limits. + configuration.ignoresViewportScaleLimits = true super.init(frame: frame, configuration: configuration) } diff --git a/Client/Frontend/Browser/TabScrollController.swift b/Client/Frontend/Browser/TabScrollController.swift index fee884b50..81be4d1d6 100644 --- a/Client/Frontend/Browser/TabScrollController.swift +++ b/Client/Frontend/Browser/TabScrollController.swift @@ -27,7 +27,6 @@ class TabScrollingController: NSObject { didSet { self.scrollView?.panGestureRecognizer.addTarget(self, action: #selector(handlePan)) - self.scrollView?.panGestureRecognizer.maximumNumberOfTouches = 1 scrollView?.delegate = self } } From 1ec2c4e89133275a2faf5afeafb523fd2113c6d5 Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Thu, 3 Jan 2019 03:14:47 +0530 Subject: [PATCH 06/12] Review Fix --- Client/Frontend/Browser/BraveWebView.swift | 2 -- Client/Frontend/Browser/Tab.swift | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Client/Frontend/Browser/BraveWebView.swift b/Client/Frontend/Browser/BraveWebView.swift index 9bc6c019b..e2a6fdbd7 100644 --- a/Client/Frontend/Browser/BraveWebView.swift +++ b/Client/Frontend/Browser/BraveWebView.swift @@ -13,8 +13,6 @@ class BraveWebView: WKWebView { configuration.websiteDataStore = WKWebsiteDataStore.nonPersistent() } - // Enables Zoom in website by ignoring their javascript based viewport Scale limits. - configuration.ignoresViewportScaleLimits = true super.init(frame: frame, configuration: configuration) } diff --git a/Client/Frontend/Browser/Tab.swift b/Client/Frontend/Browser/Tab.swift index 059799484..e168507f6 100644 --- a/Client/Frontend/Browser/Tab.swift +++ b/Client/Frontend/Browser/Tab.swift @@ -189,6 +189,8 @@ class Tab: NSObject { configuration!.preferences = WKPreferences() configuration!.preferences.javaScriptCanOpenWindowsAutomatically = false configuration!.allowsInlineMediaPlayback = true + // Enables Zoom in website by ignoring their javascript based viewport Scale limits. + configuration!.ignoresViewportScaleLimits = true let webView = TabWebView(frame: .zero, configuration: configuration!) webView.delegate = self configuration = nil From 8fc0af851937935f37ae92b1fcc81719662ee907 Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Thu, 10 Jan 2019 03:24:00 +0530 Subject: [PATCH 07/12] The footer view will never be nil but is initialised in viewdidload so treating it as non-nil while declaring it as unwrapped optional. Removed old animation code fix which became redundant after update --- .../Browser/BrowserViewController.swift | 12 +++--------- .../Frontend/Browser/TabScrollController.swift | 17 +++++++++++------ 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/Client/Frontend/Browser/BrowserViewController.swift b/Client/Frontend/Browser/BrowserViewController.swift index 0eec6eaf8..a5bec8084 100644 --- a/Client/Frontend/Browser/BrowserViewController.swift +++ b/Client/Frontend/Browser/BrowserViewController.swift @@ -667,11 +667,7 @@ class BrowserViewController: UIViewController { webViewContainerTopOffset = make.top.equalTo(readerModeBar?.snp.bottom ?? self.header.snp.bottom).constraint let findInPageHeight = (findInPageBar == nil) ? 0 : UIConstants.ToolbarHeight - if let footerView = self.footer { - make.bottom.equalTo(footerView.snp.top).offset(-findInPageHeight) - } else { - make.bottom.equalTo(self.view).offset(-findInPageHeight) - } + make.bottom.equalTo(self.footer.snp.top).offset(-findInPageHeight) } // Setup the bottom toolbar @@ -694,7 +690,7 @@ class BrowserViewController: UIViewController { make.left.right.equalTo(self.view) if self.homePanelIsInline { - make.bottom.equalTo(self.footer?.snp.top ?? self.view.snp.bottom) + make.bottom.equalTo(self.footer.snp.top) } else { make.bottom.equalTo(self.view.snp.bottom) } @@ -705,10 +701,8 @@ class BrowserViewController: UIViewController { make.width.equalTo(self.view.snp.width) if let keyboardHeight = keyboardState?.intersectionHeightForView(self.view), keyboardHeight > 0 { make.bottom.equalTo(self.view).offset(-keyboardHeight) - } else if let footer = self.footer { - make.bottom.equalTo(footer.snp.top) } else { - make.bottom.equalTo(self.view) + make.bottom.equalTo(self.footer.snp.top) } } } diff --git a/Client/Frontend/Browser/TabScrollController.swift b/Client/Frontend/Browser/TabScrollController.swift index 81be4d1d6..deb4d8df7 100644 --- a/Client/Frontend/Browser/TabScrollController.swift +++ b/Client/Frontend/Browser/TabScrollController.swift @@ -244,22 +244,27 @@ private extension TabScrollingController { } func animateToolbarsWithOffsets(_ animated: Bool, duration: TimeInterval, headerOffset: CGFloat, footerOffset: CGFloat, alpha: CGFloat, completion: ((_ finished: Bool) -> Void)?) { + //NOTE: The issue below (& its fix) is irrelevant now since the the scroll logic is updated, infact the logic further + // created a jump effect while animating the toolbar. Keeping these comments for future reference. + /* guard let scrollView = scrollView else { return } let initialContentOffset = scrollView.contentOffset - - // If this function is used to fully animate the toolbar from hidden to shown, keep the page from scrolling by adjusting contentOffset, - // Otherwise when the toolbar is hidden and a link navigated, showing the toolbar will scroll the page and - // produce a ~50px page jumping effect in response to tap navigations. + + //If this function is used to fully animate the toolbar from hidden to shown, keep the page from scrolling by adjusting contentOffset, + //Otherwise when the toolbar is hidden and a link navigated, showing the toolbar will scroll the page and + //produce a ~50px page jumping effect in response to tap navigations. + let isShownFromHidden = headerTopOffset == -topScrollHeight && headerOffset == 0 if isShownFromHidden { scrollView.contentOffset = CGPoint(x: initialContentOffset.x, y: initialContentOffset.y + self.topScrollHeight) } + */ self.headerTopOffset = headerOffset self.footerBottomOffset = footerOffset - self.urlBar?.updateAlphaForSubviews(alpha) - self.tabsBar?.view.alpha = alpha let animation: () -> Void = { + self.urlBar?.updateAlphaForSubviews(alpha) + self.tabsBar?.view.alpha = alpha self.header?.superview?.layoutIfNeeded() } From d0b9ca75fa211e7f3be8e5bed77c8f2032b53eb5 Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Wed, 23 Jan 2019 23:00:29 +0530 Subject: [PATCH 08/12] Fixes an issue where the footer height is ambiguous due to lack of constraints. --- Client/Frontend/Browser/BrowserViewController.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Client/Frontend/Browser/BrowserViewController.swift b/Client/Frontend/Browser/BrowserViewController.swift index a5bec8084..1a0423b60 100644 --- a/Client/Frontend/Browser/BrowserViewController.swift +++ b/Client/Frontend/Browser/BrowserViewController.swift @@ -679,6 +679,9 @@ class BrowserViewController: UIViewController { footer.snp.remakeConstraints { make in scrollController.footerBottomConstraint = make.bottom.equalTo(self.view.snp.bottom).constraint make.leading.trailing.equalTo(self.view) + if self.toolbar == nil { + make.height.equalTo(0.0) + } } urlBar.setNeedsUpdateConstraints() From 09cc3e1034f84a3795a7152457a859684491d0dc Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Wed, 23 Jan 2019 23:20:19 +0530 Subject: [PATCH 09/12] refactored animation option --- Client/Frontend/Browser/TabScrollController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Client/Frontend/Browser/TabScrollController.swift b/Client/Frontend/Browser/TabScrollController.swift index deb4d8df7..9d51e6ea3 100644 --- a/Client/Frontend/Browser/TabScrollController.swift +++ b/Client/Frontend/Browser/TabScrollController.swift @@ -269,7 +269,7 @@ private extension TabScrollingController { } if animated { - UIView.animate(withDuration: duration, delay: 0, options: [.beginFromCurrentState, .allowUserInteraction], animations: animation, completion: completion) + UIView.animate(withDuration: duration, delay: 0, options: .allowUserInteraction, animations: animation, completion: completion) } else { animation() completion?(true) From 1f964054eec2475fc15eec5f08c9f9b97b408e6a Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Tue, 12 Feb 2019 22:32:14 +0530 Subject: [PATCH 10/12] Aligned fav vc with footer --- Client/Frontend/Browser/BrowserViewController.swift | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Client/Frontend/Browser/BrowserViewController.swift b/Client/Frontend/Browser/BrowserViewController.swift index 1a0423b60..54b2ca11a 100644 --- a/Client/Frontend/Browser/BrowserViewController.swift +++ b/Client/Frontend/Browser/BrowserViewController.swift @@ -692,11 +692,7 @@ class BrowserViewController: UIViewController { webViewContainerTopOffset = make.top.equalTo(readerModeBar?.snp.bottom ?? self.header.snp.bottom).constraint make.left.right.equalTo(self.view) - if self.homePanelIsInline { - make.bottom.equalTo(self.footer.snp.top) - } else { - make.bottom.equalTo(self.view.snp.bottom) - } + make.bottom.equalTo(self.footer.snp.top) } alertStackView.snp.remakeConstraints { make in From fedf1b66e8fa99c9de72a1f344cf1903faef3f60 Mon Sep 17 00:00:00 2001 From: Danish Jafri Date: Wed, 10 Apr 2019 00:38:29 +0530 Subject: [PATCH 11/12] resolved conflict --- Client/Frontend/Browser/Tab.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Client/Frontend/Browser/Tab.swift b/Client/Frontend/Browser/Tab.swift index e168507f6..5d545276c 100644 --- a/Client/Frontend/Browser/Tab.swift +++ b/Client/Frontend/Browser/Tab.swift @@ -190,7 +190,6 @@ class Tab: NSObject { configuration!.preferences.javaScriptCanOpenWindowsAutomatically = false configuration!.allowsInlineMediaPlayback = true // Enables Zoom in website by ignoring their javascript based viewport Scale limits. - configuration!.ignoresViewportScaleLimits = true let webView = TabWebView(frame: .zero, configuration: configuration!) webView.delegate = self configuration = nil From cc1307b338a5a6e303ee4ac437550b67c0d52d13 Mon Sep 17 00:00:00 2001 From: Joel Reis Date: Thu, 11 Apr 2019 09:37:14 -0400 Subject: [PATCH 12/12] Removed unneeded comment. --- .../Frontend/Browser/TabScrollController.swift | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/Client/Frontend/Browser/TabScrollController.swift b/Client/Frontend/Browser/TabScrollController.swift index 9d51e6ea3..bd6d33871 100644 --- a/Client/Frontend/Browser/TabScrollController.swift +++ b/Client/Frontend/Browser/TabScrollController.swift @@ -244,22 +244,6 @@ private extension TabScrollingController { } func animateToolbarsWithOffsets(_ animated: Bool, duration: TimeInterval, headerOffset: CGFloat, footerOffset: CGFloat, alpha: CGFloat, completion: ((_ finished: Bool) -> Void)?) { - //NOTE: The issue below (& its fix) is irrelevant now since the the scroll logic is updated, infact the logic further - // created a jump effect while animating the toolbar. Keeping these comments for future reference. - /* - guard let scrollView = scrollView else { return } - let initialContentOffset = scrollView.contentOffset - - //If this function is used to fully animate the toolbar from hidden to shown, keep the page from scrolling by adjusting contentOffset, - //Otherwise when the toolbar is hidden and a link navigated, showing the toolbar will scroll the page and - //produce a ~50px page jumping effect in response to tap navigations. - - let isShownFromHidden = headerTopOffset == -topScrollHeight && headerOffset == 0 - - if isShownFromHidden { - scrollView.contentOffset = CGPoint(x: initialContentOffset.x, y: initialContentOffset.y + self.topScrollHeight) - } - */ self.headerTopOffset = headerOffset self.footerBottomOffset = footerOffset let animation: () -> Void = {