Bugfix FXIOS-10004 [Toolbar redesign] Fix address toolbar bottom bord…

…ers on scroll (#22079)
mozilla-mobile · Sep 19, 2024 · 8ee8763 · 8ee8763
1 parent 6fdec62
commit 8ee8763
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 32 deletions.
diff --git a/firefox-ios/Client/Frontend/Browser/TabScrollController.swift b/firefox-ios/Client/Frontend/Browser/TabScrollController.swift
@@ -52,7 +52,6 @@ class TabScrollingController: NSObject, FeatureFlaggable, SearchBarLocationProvi
     var headerTopConstraint: Constraint?
 
     private var lastPanTranslation: CGFloat = 0
-    private var lastContentOffsetY: CGFloat = 0
     private var scrollDirection: ScrollDirection = .down
     var toolbarState: ToolbarState = .visible
 
@@ -456,10 +455,6 @@ extension TabScrollingController: UIGestureRecognizerDelegate {
 }
 
 extension TabScrollingController: UIScrollViewDelegate {
-    func scrollViewWillBeginDragging(_ scrollView: UIScrollView) {
-        lastContentOffsetY = scrollView.contentOffset.y
-    }
-
     func scrollViewDidEndDragging(_ scrollView: UIScrollView, willDecelerate decelerate: Bool) {
         guard !tabIsLoading(), !isBouncingAtBottom(), isAbleToScroll, let tab else { return }
 
@@ -482,15 +477,22 @@ extension TabScrollingController: UIScrollViewDelegate {
             setOffset(y: 0, for: scrollView)
         }
 
-        let scrolledToTop = lastContentOffsetY > 0 && scrollView.contentOffset.y <= 0
-        let scrolledDown = lastContentOffsetY == 0 && scrollView.contentOffset.y > 0
-
-        if scrolledDown || scrolledToTop {
-            lastContentOffsetY = scrollView.contentOffset.y
-            let action = GeneralBrowserMiddlewareAction(scrollOffset: scrollView.contentOffset,
-                                                        windowUUID: windowUUID,
-                                                        actionType: GeneralBrowserMiddlewareActionType.websiteDidScroll)
-            store.dispatch(action)
+        let toolbarState = store.state.screenState(Client.ToolbarState.self, for: .toolbar, window: windowUUID)
+
+        guard let toolbarState,
+              let borderPosition = toolbarState.addressToolbar.borderPosition
+        else { return }
+
+        // Only dispatch the action to update the toolbar border if needed, which is only if either
+        // a) we scroll down, and the toolbar border is not already at the bottom (so we show it), or
+        // b) we scroll up past the top of the scroll view, and the border is currently at the bottom (so we hide it)
+        if (scrollView.contentOffset.y > 0 && borderPosition != .bottom)
+           || (scrollView.contentOffset.y < 0 && borderPosition != .none) {
+            store.dispatch(
+                GeneralBrowserMiddlewareAction(
+                    scrollOffset: scrollView.contentOffset,
+                    windowUUID: windowUUID,
+                    actionType: GeneralBrowserMiddlewareActionType.websiteDidScroll))
         }
 
         guard isAnimatingToolbar else { return }

diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarMiddleware.swift
@@ -237,10 +237,6 @@ final class ToolbarMiddleware: FeatureFlaggable {
             scrollY: scrollOffset.y
         )
 
-        let needsAddressToolbarUpdate = toolbarState.addressToolbar.borderPosition != addressBorderPosition
-
-        guard needsAddressToolbarUpdate else { return }
-
         let toolbarAction = ToolbarAction(
             addressBorderPosition: addressBorderPosition,
             windowUUID: action.windowUUID,

diff --git a/firefox-ios/Client/Frontend/Home/HomepageViewController.swift b/firefox-ios/Client/Frontend/Home/HomepageViewController.swift
@@ -43,7 +43,6 @@ class HomepageViewController:
     private var jumpBackInContextualHintViewController: ContextualHintViewController
     private var syncTabContextualHintViewController: ContextualHintViewController
     private var collectionView: UICollectionView! = nil
-    private var lastContentOffsetY: CGFloat = 0
     private var logger: Logger
     var windowUUID: WindowUUID { return tabManager.windowUUID }
     var currentWindowUUID: UUID? { return windowUUID }
@@ -397,10 +396,6 @@ class HomepageViewController:
         }
     }
 
-    func scrollViewWillBeginDragging(_ scrollView: UIScrollView) {
-        lastContentOffsetY = scrollView.contentOffset.y
-    }
-
     func scrollViewDidScroll(_ scrollView: UIScrollView) {
         // We only handle status bar overlay alpha if there's a wallpaper applied on the homepage
         if WallpaperManager().currentWallpaper.type != .defaultWallpaper {
@@ -410,9 +405,11 @@ class HomepageViewController:
                                                          theme: theme)
         }
 
+        let toolbarState = store.state.screenState(ToolbarState.self, for: .toolbar, window: windowUUID)
+
         // Only dispatch action when user is in edit mode to avoid having the toolbar re-displayed
         if featureFlags.isFeatureEnabled(.toolbarRefactor, checking: .buildOnly),
-           let toolbarState = store.state.screenState(ToolbarState.self, for: .toolbar, window: windowUUID),
+           let toolbarState,
            toolbarState.addressToolbar.isEditing {
             // When the user scrolls the homepage we cancel edit mode
             // On a website we just dismiss the keyboard
@@ -425,15 +422,20 @@ class HomepageViewController:
             }
         }
 
-        let scrolledToTop = lastContentOffsetY > 0 && scrollView.contentOffset.y <= 0
-        let scrolledDown = lastContentOffsetY == 0 && scrollView.contentOffset.y > 0
+        guard let toolbarState,
+              let borderPosition = toolbarState.addressToolbar.borderPosition
+        else { return }
 
-        if scrolledDown || scrolledToTop {
-            lastContentOffsetY = scrollView.contentOffset.y
-            let action = GeneralBrowserMiddlewareAction(scrollOffset: scrollView.contentOffset,
-                                                        windowUUID: windowUUID,
-                                                        actionType: GeneralBrowserMiddlewareActionType.websiteDidScroll)
-            store.dispatch(action)
+        // Only dispatch the action to update the toolbar border if needed, which is only if either
+        // a) we scroll down, and the toolbar border is not already at the bottom (so we show it), or
+        // b) we scroll up past the top of the scroll view, and the border is currently at the bottom (so we hide it)
+        if (scrollView.contentOffset.y > 0 && borderPosition != .bottom)
+           || (scrollView.contentOffset.y < 0 && borderPosition != .none) {
+            store.dispatch(
+                GeneralBrowserMiddlewareAction(
+                    scrollOffset: scrollView.contentOffset,
+                    windowUUID: windowUUID,
+                    actionType: GeneralBrowserMiddlewareActionType.websiteDidScroll))
         }
     }