Skip to content

Commit 0cf6cf2

Browse files
authored
Bugfix FXIOS-15200 ⁃ [TabScrollController] Fix bugs with Snapkit removal enable iPad and unit test (#32867)
* Refactor and fix zoombar and reader mode on iPad * Add unit test for edge cases
1 parent 3ff9207 commit 0cf6cf2

3 files changed

Lines changed: 208 additions & 15 deletions

File tree

firefox-ios/Client.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@
261261
1DEBC55E2AC4ED70006E4801 /* RemoteTabsPanel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DEBC55D2AC4ED70006E4801 /* RemoteTabsPanel.swift */; };
262262
1DF2BDC32BD1BCF300E53C57 /* WindowManager+DebugUtilities.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DF2BDC22BD1BCF300E53C57 /* WindowManager+DebugUtilities.swift */; };
263263
1DF9168A2F48EAB100FA2407 /* MockRelayController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DF916892F48EAAD00FA2407 /* MockRelayController.swift */; };
264+
21050F332F7D66C00006B76C /* BrowserTabScrollHandlerDelegateTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 21050F322F7D66C00006B76C /* BrowserTabScrollHandlerDelegateTest.swift */; };
264265
2109478928AFD24C00B73D44 /* OnboardingViewControllerProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2109478828AFD24C00B73D44 /* OnboardingViewControllerProtocol.swift */; };
265266
2109726E2DCA8831001162A2 /* ZoomTelemetryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2109726D2DCA8831001162A2 /* ZoomTelemetryTests.swift */; };
266267
210972702DCBA50F001162A2 /* GenericItemCellView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2109726F2DCBA4CF001162A2 /* GenericItemCellView.swift */; };
@@ -3013,6 +3014,7 @@
30133014
20C24CE7AEFE559B0B06390C /* nn */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = nn; path = nn.lproj/LoginManager.strings; sourceTree = "<group>"; };
30143015
20C6401CAC4ACAA189D446BC /* sq */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = sq; path = sq.lproj/Today.strings; sourceTree = "<group>"; };
30153016
20F2414BA51A5BFAF5098D35 /* bs */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = bs; path = bs.lproj/Menu.strings; sourceTree = "<group>"; };
3017+
21050F322F7D66C00006B76C /* BrowserTabScrollHandlerDelegateTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserTabScrollHandlerDelegateTest.swift; sourceTree = "<group>"; };
30163018
2109478828AFD24C00B73D44 /* OnboardingViewControllerProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingViewControllerProtocol.swift; sourceTree = "<group>"; };
30173019
2109726D2DCA8831001162A2 /* ZoomTelemetryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ZoomTelemetryTests.swift; sourceTree = "<group>"; };
30183020
2109726F2DCBA4CF001162A2 /* GenericItemCellView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GenericItemCellView.swift; sourceTree = "<group>"; };
@@ -12069,6 +12071,7 @@
1206912071
21365E3C2F314D1D0000C369 /* BrowserViewControllerLayoutManagerTests.swift */,
1207012072
E16941B32C4E4A2E00FF5F4E /* BrowserViewControllerStateTests.swift */,
1207112073
BC003F5D2B59F44500929ECB /* BrowserViewControllerTests.swift */,
12074+
21050F322F7D66C00006B76C /* BrowserTabScrollHandlerDelegateTest.swift */,
1207212075
);
1207312076
path = BrowserViewController;
1207412077
sourceTree = "<group>";
@@ -19739,6 +19742,7 @@
1973919742
0E6557402D82232B00D7F017 /* DownloadProgressManagerTests.swift in Sources */,
1974019743
AADBD0E42EBBA79B00F35E30 /* MockLanguageDetector.swift in Sources */,
1974119744
AAB434062E82F0600075E47F /* MockTrendingSearchProvider.swift in Sources */,
19745+
21050F332F7D66C00006B76C /* BrowserTabScrollHandlerDelegateTest.swift in Sources */,
1974219746
C80C11F428B3CD580062922A /* MockUserDefaultsTests.swift in Sources */,
1974319747
E14D6D342CCBA9FF0058B910 /* AddressBarStateTests.swift in Sources */,
1974419748
2F697F7E1A9FD22D009E03AE /* SearchEnginesManagerTests.swift in Sources */,

firefox-ios/Client/Frontend/Browser/BrowserViewController/Extensions/BrowserViewController+TabScrollHandlerDelegate.swift

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -63,29 +63,27 @@ extension BrowserViewController: TabScrollHandler.Delegate,
6363
}
6464

6565
private func calculateHeaderOffset() -> CGFloat {
66-
let baseOffset = -getHeaderSize().height
66+
let headerHeight = getHeaderSize().height
6767
let isiPad = UIDevice.current.userInterfaceIdiom == .pad
68-
let isNavToolbarVisible = ToolbarHelper().shouldShowNavigationToolbar(for: view.traitCollection)
69-
7068
let isReaderModeActive = tabManager.selectedTab?.url?.isReaderModeURL == true
69+
let isZoomBarInHeader = (isiPad && zoomPageBar != nil)
7170

72-
// Add extra offset when the minimal address bar is enabled and visible,
73-
// to preserve spacing for the compact header UI.
74-
if isMinimalAddressBarEnabled && (isiPad || isNavToolbarVisible) && !isReaderModeActive {
75-
return baseOffset + UX.minimalHeaderOffset
71+
// Reader mode and zoom bar in header require fully hiding all header content.
72+
// Use the stable layout position to handle cases where an animation is already in progress.
73+
guard !isReaderModeActive, !isZoomBarInHeader else {
74+
let headerLayoutY = header.frame.minY - header.transform.ty
75+
return -(headerHeight + headerLayoutY)
7676
}
7777

78-
// In Reader Mode, fully hide the header. Since an animation may already be
79-
// in progress, add the offset to the transform position to get the initial position.
80-
if isReaderModeActive {
81-
// Subtract the current transform to get the stable layout position, not the animated frame
82-
let headerYPosition = header.frame.minY - header.transform.ty
83-
let fullyHideOffset = -(baseOffset + headerYPosition)
84-
return fullyHideOffset
78+
// Minimal address bar: keep a small strip visible so the domain remains readable
79+
// while scrolled. Applies only when the nav toolbar (iPhone portrait) or iPad layout is visible
80+
let isNavToolbarVisible = ToolbarHelper().shouldShowNavigationToolbar(for: view.traitCollection)
81+
if isMinimalAddressBarEnabled && (isiPad || isNavToolbarVisible) {
82+
return -headerHeight + UX.minimalHeaderOffset
8583
}
8684

8785
// scroll the given the header height
88-
return baseOffset
86+
return -headerHeight
8987
}
9088

9189
// Checks if minimal address bar is enabled and tab is on reader mode bar or findInPage
Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
// This Source Code Form is subject to the terms of the Mozilla Public
2+
// License, v. 2.0. If a copy of the MPL was not distributed with this
3+
// file, You can obtain one at http://mozilla.org/MPL/2.0/
4+
5+
import Foundation
6+
import MozillaAppServices
7+
import Storage
8+
import XCTest
9+
import Glean
10+
import Common
11+
import Shared
12+
13+
@testable import Client
14+
15+
@MainActor
16+
final class BrowserTabScrollHandlerDelegateTest: BrowserViewControllerConstraintTestsBase {
17+
private let minimalHeaderOffset = BrowserViewController.UX.minimalHeaderOffset
18+
19+
// MARK: - Top Toolbar
20+
21+
func testHideToolbar_TopToolbar_headerMovesUp() {
22+
let subject = createSubject(isBottomSearchBar: false)
23+
subject.setupToolbarAnimator()
24+
25+
let initialHeaderFrame = subject.header.frame
26+
27+
subject.hideToolbar()
28+
29+
// Current Y position should be less than initial position, the offset should be
30+
// the height plus UX.minimalHeaderOffset
31+
let minimalHeaderOffset = (initialHeaderFrame.minY - initialHeaderFrame.height) + minimalHeaderOffset
32+
XCTAssertLessThan(subject.header.frame.minY, initialHeaderFrame.minY)
33+
XCTAssertEqual(subject.header.frame.minY, minimalHeaderOffset)
34+
}
35+
36+
func testShowToolbar_TopToolbar_afterHide_restoresHeaderPosition() {
37+
let subject = createSubject(isBottomSearchBar: false)
38+
subject.setupToolbarAnimator()
39+
let initialHeaderMinY = subject.header.frame.minY
40+
41+
subject.hideToolbar()
42+
subject.showToolbar()
43+
44+
XCTAssertEqual(subject.header.frame.minY, initialHeaderMinY)
45+
}
46+
47+
// MARK: - Top Toolbar Transition
48+
49+
func testUpdateToolbarTransition_TopToolbar_collapsing_movesHeaderUp() {
50+
let subject = createSubject(isBottomSearchBar: false)
51+
subject.setupToolbarAnimator()
52+
let initialHeaderMinY = subject.header.frame.minY
53+
54+
subject.updateToolbarTransition(progress: 20, towards: .collapsed)
55+
56+
XCTAssertLessThan(subject.header.frame.minY, initialHeaderMinY)
57+
}
58+
59+
func testUpdateToolbarTransition_TopToolbar_expanding_resetsTransform() {
60+
let subject = createSubject(isBottomSearchBar: false)
61+
subject.setupToolbarAnimator()
62+
subject.updateToolbarTransition(progress: 20, towards: .collapsed)
63+
64+
subject.updateToolbarTransition(progress: 0, towards: .expanded)
65+
66+
XCTAssertEqual(subject.header.transform, .identity)
67+
}
68+
69+
// MARK: - Top Toolbar + Reader Mode
70+
71+
func testHideToolbar_ReaderModeActive_headerMovesUp() {
72+
let subject = createSubject(isBottomSearchBar: false)
73+
selectReaderModeTab()
74+
subject.setupToolbarAnimator()
75+
let initialHeaderFrame = subject.header.frame
76+
77+
subject.hideToolbar()
78+
79+
// Current Y position should be less than initial position, the offset should be the height
80+
let minimalHeaderOffset = initialHeaderFrame.minY - initialHeaderFrame.height
81+
XCTAssertLessThan(subject.header.frame.minY, initialHeaderFrame.minY)
82+
XCTAssertEqual(subject.header.frame.minY, minimalHeaderOffset)
83+
}
84+
85+
func testShowToolbar_ReaderModeActive_afterHide_restoresHeaderPosition() {
86+
let subject = createSubject(isBottomSearchBar: false)
87+
selectReaderModeTab()
88+
subject.setupToolbarAnimator()
89+
let initialHeaderMinY = subject.header.frame.minY
90+
91+
subject.hideToolbar()
92+
subject.showToolbar()
93+
94+
XCTAssertEqual(subject.header.frame.minY, initialHeaderMinY)
95+
}
96+
97+
// MARK: - Bottom Search Bar
98+
99+
func testHideToolbar_BottomSearchBar_overKeyboardContainerMovesDown() {
100+
let subject = createSubject()
101+
guard subject.overKeyboardContainer.frame.height > 0 else { return }
102+
let initialMinY = subject.overKeyboardContainer.frame.minY
103+
104+
setupAnimator(for: subject, overKeyboardContainerHeight: subject.overKeyboardContainer.frame.height)
105+
subject.toolbarAnimator?.hideToolbar()
106+
107+
XCTAssertGreaterThan(subject.overKeyboardContainer.frame.minY, initialMinY)
108+
}
109+
110+
func testShowToolbar_BottomSearchBar_afterHide_restoresOverKeyboardPosition() {
111+
let subject = createSubject()
112+
guard subject.overKeyboardContainer.frame.height > 0 else { return }
113+
let initialMinY = subject.overKeyboardContainer.frame.minY
114+
115+
setupAnimator(for: subject, overKeyboardContainerHeight: subject.overKeyboardContainer.frame.height)
116+
subject.toolbarAnimator?.hideToolbar()
117+
subject.toolbarAnimator?.showToolbar()
118+
119+
XCTAssertEqual(subject.overKeyboardContainer.frame.minY, initialMinY)
120+
}
121+
122+
func testHideToolbar_BottomSearchBar_topHeaderDoesNotMove() {
123+
let subject = createSubject(isBottomSearchBar: true)
124+
subject.setupToolbarAnimator()
125+
let initialHeaderMinY = subject.header.frame.minY
126+
127+
subject.hideToolbar()
128+
129+
// For Bottom search bar header has height = 0 and should not be animated.
130+
XCTAssertEqual(subject.header.frame.minY, initialHeaderMinY)
131+
XCTAssertEqual(subject.header.frame.height, 0)
132+
}
133+
134+
// MARK: - Animator Setup
135+
136+
func testSetupToolbarAnimator_setsViewAndDelegate() {
137+
let subject = createSubject(isBottomSearchBar: false)
138+
139+
subject.setupToolbarAnimator()
140+
141+
XCTAssertNotNil(subject.toolbarAnimator)
142+
XCTAssertTrue(subject.toolbarAnimator?.view === subject)
143+
}
144+
145+
func testSetupToolbarAnimator_calledTwice_replacesAnimator() {
146+
let subject = createSubject(isBottomSearchBar: false)
147+
subject.setupToolbarAnimator()
148+
let firstAnimator = subject.toolbarAnimator
149+
150+
subject.setupToolbarAnimator()
151+
152+
XCTAssertFalse(subject.toolbarAnimator === firstAnimator)
153+
}
154+
155+
// MARK: - Private
156+
157+
private func selectReaderModeTab() {
158+
let tab = Tab(profile: profile, windowUUID: .XCTestDefaultUUID)
159+
tab.url = URL(string: "http://localhost:6571/reader-mode/page?url=https://example.com")
160+
tabManager.selectedTab = tab
161+
}
162+
163+
private func createToolbarAnimator(headerHeight: CGFloat = 60,
164+
bottomContainerHeight: CGFloat = 60,
165+
overKeyboardContainerHeight: CGFloat = 60) -> ToolbarAnimator {
166+
let context = ToolbarContext(overKeyboardContainerHeight: overKeyboardContainerHeight,
167+
bottomContainerHeight: bottomContainerHeight,
168+
headerHeight: headerHeight)
169+
return ToolbarAnimator(context: context)
170+
}
171+
172+
/// Wires a ToolbarAnimator with an explicit context onto the subject, bypassing
173+
/// `updateToolbarContext()`. Use this when the calculated context would be 0 in the test environment
174+
private func setupAnimator(for subject: BrowserViewController,
175+
headerHeight: CGFloat = 60,
176+
bottomContainerHeight: CGFloat = 60,
177+
overKeyboardContainerHeight: CGFloat = 60) {
178+
let animator = createToolbarAnimator(headerHeight: headerHeight,
179+
bottomContainerHeight: bottomContainerHeight,
180+
overKeyboardContainerHeight: overKeyboardContainerHeight)
181+
animator.view = subject
182+
animator.delegate = subject
183+
subject.toolbarAnimator = animator
184+
}
185+
186+
private func setupNimbusTabScrollRefactorTesting(isEnabled: Bool) {
187+
FxNimbus.shared.features.tabScrollRefactorFeature.with { _, _ in
188+
return TabScrollRefactorFeature(enabled: isEnabled)
189+
}
190+
}
191+
}

0 commit comments

Comments
 (0)