From f861f3add86af3fec98357c11093fd8e55ade992 Mon Sep 17 00:00:00 2001 From: Winnie Teichmann <4530+thatswinnie@users.noreply.github.com> Date: Tue, 5 Dec 2023 16:24:21 +0100 Subject: [PATCH] Refactor FXIOS-7812 [v121] Fakespot - iPad - sidebar open/close state resets itself when user changes from landscape to portrait mode (#17587) * Dismiss sidebar when opting out * Dismiss sidebar when tapping not now * Use state to dismiss and show Fakespot * Dismiss Fakespot when startAtHome * Use iPad sidebar state * Update Fakespot when user navigates using the back/forward buttons * Update Fakespot when user navigates using the back/forward buttons * Update Fakespot actions * Open and close fakespot via state change * Fix bugs related to deep-linking * Fix test * Address PR comments * Fix merge issues * Fix Swiftlint warning (cherry picked from commit f8565a9a19a9e5ba48b8e5e6fac7c1a5eba9cba4) # Conflicts: # Client/Frontend/Browser/BrowserViewController.swift # Client/Frontend/Fakespot/FakespotState.swift # Client/Frontend/FeltPrivacy/BrowserViewControllerState.swift --- .../Browser/BrowserCoordinator.swift | 6 +- .../Browser/BrowserViewController.swift | 63 ++++++++++++++++--- ...BrowserViewController+URLBarDelegate.swift | 46 +++++--------- ...owserViewController+WebViewDelegates.swift | 1 + Client/Frontend/Fakespot/FakespotAction.swift | 5 +- .../Fakespot/FakespotCoordinator.swift | 15 ++--- Client/Frontend/Fakespot/FakespotState.swift | 34 +++++++--- Client/Frontend/Fakespot/FakespotUtils.swift | 6 +- .../Fakespot/FakespotViewController.swift | 29 ++++----- .../BrowserViewControllerState.swift | 59 +++++++++++++++++ .../Toolbar+URLBar/TabLocationView.swift | 4 +- .../Frontend/Toolbar+URLBar/URLBarView.swift | 5 -- .../BrowserCoordinatorTests.swift | 2 +- .../FakespotCoordinatorTests.swift | 6 +- .../Fakespot/FakespotUtilsTests.swift | 10 +++ .../TabTray/Legacy/TabLocationViewTests.swift | 1 - 16 files changed, 199 insertions(+), 93 deletions(-) create mode 100644 Client/Frontend/FeltPrivacy/BrowserViewControllerState.swift diff --git a/Client/Coordinators/Browser/BrowserCoordinator.swift b/Client/Coordinators/Browser/BrowserCoordinator.swift index f2e9e54eeb5a..ef76081044ff 100644 --- a/Client/Coordinators/Browser/BrowserCoordinator.swift +++ b/Client/Coordinators/Browser/BrowserCoordinator.swift @@ -420,15 +420,15 @@ class BrowserCoordinator: BaseCoordinator, guard let fakespotCoordinator = childCoordinators.first(where: { $0 is FakespotCoordinator}) as? FakespotCoordinator else { return // there is no modal to close } - fakespotCoordinator.fakespotControllerDidDismiss(animated: animated) + fakespotCoordinator.dismissModal(animated: animated) } func dismissFakespotSidebar(sidebarContainer: SidebarEnabledViewProtocol, parentViewController: UIViewController) { guard let fakespotCoordinator = childCoordinators.first(where: { $0 is FakespotCoordinator}) as? FakespotCoordinator else { return // there is no sidebar to close } - fakespotCoordinator.fakespotControllerCloseSidebar(sidebarContainer: sidebarContainer, - parentViewController: parentViewController) + fakespotCoordinator.closeSidebar(sidebarContainer: sidebarContainer, + parentViewController: parentViewController) } func updateFakespotSidebar(productURL: URL, diff --git a/Client/Frontend/Browser/BrowserViewController.swift b/Client/Frontend/Browser/BrowserViewController.swift index 836ff3e06ddd..57824dca59dc 100644 --- a/Client/Frontend/Browser/BrowserViewController.swift +++ b/Client/Frontend/Browser/BrowserViewController.swift @@ -161,8 +161,6 @@ class BrowserViewController: UIViewController, return NewTabAccessors.getNewTabPage(profile.prefs) } - lazy var isReduxIntegrationEnabled: Bool = ReduxFlagManager.isReduxEnabled - @available(iOS 13.4, *) func keyboardPressesHandler() -> KeyboardPressesHandler { guard let keyboardPressesHandlerValue = keyboardPressesHandlerValue as? KeyboardPressesHandler else { @@ -460,34 +458,56 @@ class BrowserViewController: UIViewController, private func dismissModalsIfStartAtHome() { if tabManager.startAtHomeCheck() { - guard !dismissFakespotIfNeeded(), presentedViewController != nil else { return } + store.dispatch(FakespotAction.setAppearanceTo(false)) + guard presentedViewController != nil else { return } dismissVC() } } // MARK: - Redux +<<<<<<< HEAD private func subscribeRedux() { guard isReduxIntegrationEnabled else { return } store.dispatch(ActiveScreensStateAction.showScreen(.fakespot)) +======= + func subscribeToRedux() { + store.dispatch(ActiveScreensStateAction.showScreen(.browserViewController)) +>>>>>>> f8565a9a1 (Refactor FXIOS-7812 [v121] Fakespot - iPad - sidebar open/close state resets itself when user changes from landscape to portrait mode (#17587)) store.subscribe(self, transform: { $0.select(FakespotState.init) }) } +<<<<<<< HEAD func newState(state: FakespotState) { +======= + func unsubscribeFromRedux() { + store.unsubscribe(self) + } + + func newState(state: BrowserViewControllerState) { +>>>>>>> f8565a9a1 (Refactor FXIOS-7812 [v121] Fakespot - iPad - sidebar open/close state resets itself when user changes from landscape to portrait mode (#17587)) ensureMainThread { [weak self] in guard let self else { return } fakespotState = state // opens or close sidebar/bottom sheet to match the saved state +<<<<<<< HEAD if state.isOpenOnProductPage { guard let productURL = urlBar.currentURL else { return } handleFakespotFlow(productURL: productURL) } else { _ = dismissFakespotIfNeeded() +======= + if state.fakespotState.isOpen { + guard let productURL = urlBar.currentURL else { return } + handleFakespotFlow(productURL: productURL) + } else if !state.fakespotState.isOpen { + dismissFakespotIfNeeded() +>>>>>>> f8565a9a1 (Refactor FXIOS-7812 [v121] Fakespot - iPad - sidebar open/close state resets itself when user changes from landscape to portrait mode (#17587)) } } } @@ -757,12 +777,17 @@ class BrowserViewController: UIViewController, var fakespotNeedsUpdate = false if urlBar.currentURL != nil { fakespotNeedsUpdate = contentStackView.isSidebarVisible != FakespotUtils().shouldDisplayInSidebar(viewSize: size) +<<<<<<< HEAD if isReduxIntegrationEnabled, let fakespotState = fakespotState { fakespotNeedsUpdate = fakespotNeedsUpdate && fakespotState.isOpenOnProductPage +======= + if let fakespotState = browserViewControllerState?.fakespotState { + fakespotNeedsUpdate = fakespotNeedsUpdate && fakespotState.isOpen +>>>>>>> f8565a9a1 (Refactor FXIOS-7812 [v121] Fakespot - iPad - sidebar open/close state resets itself when user changes from landscape to portrait mode (#17587)) } if fakespotNeedsUpdate { - _ = dismissFakespotIfNeeded(animated: false) + dismissFakespotIfNeeded(animated: false) } } @@ -1688,26 +1713,48 @@ class BrowserViewController: UIViewController, } } - private func updateFakespot(tab: Tab) { + internal func updateFakespot(tab: Tab) { guard let webView = tab.webView, let url = webView.url else { + // We're on homepage or a blank tab + store.dispatch(FakespotAction.setAppearanceTo(false)) return } let environment = featureFlags.isCoreFeatureEnabled(.useStagingFakespotAPI) ? FakespotEnvironment.staging : .prod let product = ShoppingProduct(url: url, client: FakespotClient(environment: environment)) - if product.product != nil, !tab.isPrivate, contentStackView.isSidebarVisible { + + guard product.product != nil, !tab.isPrivate else { + store.dispatch(FakespotAction.setAppearanceTo(false)) + + // Quick fix: make sure to sidebar is hidden when opened from deep-link + // Relates to FXIOS-7844 + contentStackView.hideSidebar(self) + return + } + + if contentStackView.isSidebarVisible { + // Sidebar is visible, update content navigationHandler?.updateFakespotSidebar(productURL: url, sidebarContainer: contentStackView, parentViewController: self) +<<<<<<< HEAD } else if product.product != nil, !tab.isPrivate, FakespotUtils().shouldDisplayInSidebar(), isReduxIntegrationEnabled, let fakespotState = fakespotState, fakespotState.isOpenOnProductPage { +======= + } else if FakespotUtils().shouldDisplayInSidebar(), + let fakespotState = browserViewControllerState?.fakespotState, + fakespotState.isOpen { + // Sidebar should be displayed and Fakespot is open, display Fakespot +>>>>>>> f8565a9a1 (Refactor FXIOS-7812 [v121] Fakespot - iPad - sidebar open/close state resets itself when user changes from landscape to portrait mode (#17587)) handleFakespotFlow(productURL: url) - } else if contentStackView.isSidebarVisible { - _ = dismissFakespotIfNeeded(animated: true) + } else if let fakespotState = browserViewControllerState?.fakespotState, + fakespotState.sidebarOpenForiPadLandscape { + // Sidebar should be displayed, display Fakespot + store.dispatch(FakespotAction.setAppearanceTo(true)) } } diff --git a/Client/Frontend/Browser/BrowserViewController/BrowserViewController+URLBarDelegate.swift b/Client/Frontend/Browser/BrowserViewController/BrowserViewController+URLBarDelegate.swift index e6b5edcb1138..14a363423d36 100644 --- a/Client/Frontend/Browser/BrowserViewController/BrowserViewController+URLBarDelegate.swift +++ b/Client/Frontend/Browser/BrowserViewController/BrowserViewController+URLBarDelegate.swift @@ -113,38 +113,26 @@ extension BrowserViewController: URLBarDelegate { } } - func urlBarDidPressShopping(_ urlBar: URLBarView, shoppingButton: UIButton) { - guard let productURL = urlBar.currentURL else { return } - TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .shoppingButton) - - let dismissedFakespot = dismissFakespotIfNeeded() - if !dismissedFakespot { - // open flow - handleFakespotFlow(productURL: productURL) - if isReduxIntegrationEnabled { - store.dispatch(FakespotAction.toggleAppearance(true)) - } - } else if isReduxIntegrationEnabled { - // Fakespot was closed/dismissed - store.dispatch(FakespotAction.toggleAppearance(false)) - } - } - - internal func dismissFakespotIfNeeded(animated: Bool = true) -> Bool { - if contentStackView.isSidebarVisible { + internal func dismissFakespotIfNeeded(animated: Bool = true) { + guard !contentStackView.isSidebarVisible else { // hide sidebar as user tapped on shopping icon for a second time navigationHandler?.dismissFakespotSidebar(sidebarContainer: contentStackView, parentViewController: self) - return true - } else if presentedViewController as? FakespotViewController != nil { - // dismiss modal as user tapped on shopping icon for a second time - navigationHandler?.dismissFakespotModal(animated: animated) - return true + return } - return false + + // dismiss modal as user tapped on shopping icon for a second time + navigationHandler?.dismissFakespotModal(animated: animated) } internal func handleFakespotFlow(productURL: URL, viewSize: CGSize? = nil) { - if FakespotUtils().shouldDisplayInSidebar(viewSize: viewSize) { + let shouldDisplayInSidebar = FakespotUtils().shouldDisplayInSidebar(viewSize: viewSize) + if !shouldDisplayInSidebar, contentStackView.isSidebarVisible { + // Quick fix: make sure to sidebar is hidden + // Relates to FXIOS-7844 + contentStackView.hideSidebar(self) + } + + if shouldDisplayInSidebar { navigationHandler?.showFakespotFlowAsSidebar(productURL: productURL, sidebarContainer: contentStackView, parentViewController: self) @@ -171,10 +159,8 @@ extension BrowserViewController: URLBarDelegate { value: .shoppingCFRsDisplayed ) }, - andActionForButton: { [weak self] in - guard let self else { return } - guard let productURL = self.urlBar.currentURL else { return } - self.handleFakespotFlow(productURL: productURL) + andActionForButton: { + store.dispatch(FakespotAction.show) }, overlayState: overlayManager) } diff --git a/Client/Frontend/Browser/BrowserViewController/BrowserViewController+WebViewDelegates.swift b/Client/Frontend/Browser/BrowserViewController/BrowserViewController+WebViewDelegates.swift index 1b9e7e284f0a..159d51f83825 100644 --- a/Client/Frontend/Browser/BrowserViewController/BrowserViewController+WebViewDelegates.swift +++ b/Client/Frontend/Browser/BrowserViewController/BrowserViewController+WebViewDelegates.swift @@ -760,6 +760,7 @@ extension BrowserViewController: WKNavigationDelegate { if tabManager.selectedTab === tab { self.scrollController.showToolbars(animated: false) updateUIForReaderHomeStateForTab(tab, focusUrlBar: true) + updateFakespot(tab: tab) } } diff --git a/Client/Frontend/Fakespot/FakespotAction.swift b/Client/Frontend/Fakespot/FakespotAction.swift index 494034351589..3791f5266297 100644 --- a/Client/Frontend/Fakespot/FakespotAction.swift +++ b/Client/Frontend/Fakespot/FakespotAction.swift @@ -7,5 +7,8 @@ import Redux enum FakespotAction: Action { // UI trigger actions - case toggleAppearance(Bool) + case pressedShoppingButton + case show + case dismiss + case setAppearanceTo(Bool) } diff --git a/Client/Frontend/Fakespot/FakespotCoordinator.swift b/Client/Frontend/Fakespot/FakespotCoordinator.swift index a16e6f548aad..5ab1daee5b25 100644 --- a/Client/Frontend/Fakespot/FakespotCoordinator.swift +++ b/Client/Frontend/Fakespot/FakespotCoordinator.swift @@ -10,11 +10,7 @@ protocol FakespotCoordinatorDelegate: AnyObject { // Define any coordinator delegate methods } -protocol FakespotViewControllerDelegate: AnyObject { - func fakespotControllerDidDismiss(animated: Bool) -} - -class FakespotCoordinator: BaseCoordinator, FakespotViewControllerDelegate, FeatureFlaggable { +class FakespotCoordinator: BaseCoordinator, FeatureFlaggable { weak var parentCoordinator: ParentCoordinatorDelegate? private var profile: Profile @@ -48,13 +44,13 @@ class FakespotCoordinator: BaseCoordinator, FakespotViewControllerDelegate, Feat sidebarContainer.showSidebar(viewController, parentViewController: parentViewController) } - func fakespotControllerCloseSidebar(sidebarContainer: SidebarEnabledViewProtocol, - parentViewController: UIViewController) { + func closeSidebar(sidebarContainer: SidebarEnabledViewProtocol, + parentViewController: UIViewController) { sidebarContainer.hideSidebar(parentViewController) - fakespotControllerDidDismiss(animated: true) + dismissModal(animated: true) } - func fakespotControllerDidDismiss(animated: Bool) { + func dismissModal(animated: Bool) { router.dismiss(animated: animated, completion: nil) parentCoordinator?.didFinish(from: self) } @@ -69,7 +65,6 @@ class FakespotCoordinator: BaseCoordinator, FakespotViewControllerDelegate, Feat private func createFakespotViewController(productURL: URL) -> FakespotViewController { let viewModel = createFakespotViewModel(productURL: productURL) let fakespotViewController = FakespotViewController(viewModel: viewModel) - fakespotViewController.delegate = self return fakespotViewController } diff --git a/Client/Frontend/Fakespot/FakespotState.swift b/Client/Frontend/Fakespot/FakespotState.swift index f4bc294265d6..68308a570077 100644 --- a/Client/Frontend/Fakespot/FakespotState.swift +++ b/Client/Frontend/Fakespot/FakespotState.swift @@ -6,8 +6,10 @@ import Common import Redux struct FakespotState: ScreenState, Equatable { - var isOpenOnProductPage: Bool + var isOpen: Bool + var sidebarOpenForiPadLandscape: Bool +<<<<<<< HEAD init(_ appState: AppState) { guard let fakespotState = store.state.screenState(FakespotState.self, for: .fakespot) else { self.init() @@ -15,26 +17,38 @@ struct FakespotState: ScreenState, Equatable { } self.init(isOpenOnProductPage: fakespotState.isOpenOnProductPage) +======= + init(_ appState: BrowserViewControllerState) { + self.init(isOpen: appState.fakespotState.isOpen, + sidebarOpenForiPadLandscape: appState.fakespotState.sidebarOpenForiPadLandscape) +>>>>>>> f8565a9a1 (Refactor FXIOS-7812 [v121] Fakespot - iPad - sidebar open/close state resets itself when user changes from landscape to portrait mode (#17587)) } init() { - self.init(isOpenOnProductPage: false) + self.init(isOpen: false, sidebarOpenForiPadLandscape: false) } - init(isOpenOnProductPage: Bool) { - self.isOpenOnProductPage = isOpenOnProductPage + init(isOpen: Bool, sidebarOpenForiPadLandscape: Bool) { + self.isOpen = isOpen + self.sidebarOpenForiPadLandscape = sidebarOpenForiPadLandscape } static let reducer: Reducer = { state, action in switch action { - case FakespotAction.toggleAppearance(let isEnabled): - return FakespotState(isOpenOnProductPage: isEnabled) + case FakespotAction.pressedShoppingButton: + return FakespotState(isOpen: !state.isOpen, + sidebarOpenForiPadLandscape: !state.isOpen) + case FakespotAction.show: + return FakespotState(isOpen: true, + sidebarOpenForiPadLandscape: true) + case FakespotAction.dismiss: + return FakespotState(isOpen: false, + sidebarOpenForiPadLandscape: false) + case FakespotAction.setAppearanceTo(let isEnabled): + return FakespotState(isOpen: isEnabled, + sidebarOpenForiPadLandscape: state.sidebarOpenForiPadLandscape) default: return state } } - - static func == (lhs: FakespotState, rhs: FakespotState) -> Bool { - return lhs.isOpenOnProductPage == rhs.isOpenOnProductPage - } } diff --git a/Client/Frontend/Fakespot/FakespotUtils.swift b/Client/Frontend/Fakespot/FakespotUtils.swift index e08744adeb88..5cd8664d7ac7 100644 --- a/Client/Frontend/Fakespot/FakespotUtils.swift +++ b/Client/Frontend/Fakespot/FakespotUtils.swift @@ -88,8 +88,12 @@ public struct FakespotUtils: FeatureFlaggable { func shouldDisplayInSidebar(device: UIUserInterfaceIdiom = UIDevice.current.userInterfaceIdiom, window: UIWindow? = UIWindow.attachedKeyWindow, viewSize: CGSize? = nil, + isPortrait: Bool = UIWindow.isPortrait, orientation: UIDeviceOrientation = UIDevice.current.orientation) -> Bool { let isPadInMultitasking = isPadInMultitasking(device: device, window: window, viewSize: viewSize) - return device == .pad && !isPadInMultitasking && !orientation.isPortrait + + // UIDevice is not always returning the correct orientation so we check against the window orientation as well + let isPortrait = orientation.isPortrait || isPortrait + return device == .pad && !isPadInMultitasking && !isPortrait } } diff --git a/Client/Frontend/Fakespot/FakespotViewController.swift b/Client/Frontend/Fakespot/FakespotViewController.swift index 5a2a96244b89..b468b80c5919 100644 --- a/Client/Frontend/Fakespot/FakespotViewController.swift +++ b/Client/Frontend/Fakespot/FakespotViewController.swift @@ -41,7 +41,6 @@ class FakespotViewController: var themeManager: ThemeManager var themeObserver: NSObjectProtocol? private var viewModel: FakespotViewModel - weak var delegate: FakespotViewControllerDelegate? lazy var isReduxIntegrationEnabled: Bool = ReduxFlagManager.isReduxEnabled @@ -346,9 +345,9 @@ class FakespotViewController: case .onboarding: let view: FakespotOptInCardView = .build() viewModel.optInCardViewModel.dismissViewController = { [weak self] action in - guard let self = self else { return } - self.delegate?.fakespotControllerDidDismiss(animated: true) - guard let action else { return } + store.dispatch(FakespotAction.setAppearanceTo(false)) + + guard let self = self, let action else { return } viewModel.recordDismissTelemetry(by: action) } viewModel.optInCardViewModel.onOptIn = { [weak self] in @@ -378,9 +377,8 @@ class FakespotViewController: case .qualityDeterminationCard: let reviewQualityCardView: FakespotReviewQualityCardView = .build() - viewModel.reviewQualityCardViewModel.dismissViewController = { [weak self] in - guard let self = self else { return } - self.delegate?.fakespotControllerDidDismiss(animated: true) + viewModel.reviewQualityCardViewModel.dismissViewController = { + store.dispatch(FakespotAction.setAppearanceTo(false)) } reviewQualityCardView.configure(viewModel.reviewQualityCardViewModel) return reviewQualityCardView @@ -389,9 +387,9 @@ class FakespotViewController: let view: FakespotSettingsCardView = .build() view.configure(viewModel.settingsCardViewModel) viewModel.settingsCardViewModel.dismissViewController = { [weak self] action in - guard let self = self else { return } - self.delegate?.fakespotControllerDidDismiss(animated: true) - guard let action else { return } + guard let self = self, let action else { return } + + store.dispatch(FakespotAction.setAppearanceTo(false)) viewModel.recordDismissTelemetry(by: action) } return view @@ -407,9 +405,8 @@ class FakespotViewController: case .productAdCard(let adData): let view: FakespotAdView = .build() var viewModel = FakespotAdViewModel(productAdsData: adData) - viewModel.dismissViewController = { [weak self] in - guard let self = self else { return } - self.delegate?.fakespotControllerDidDismiss(animated: true) + viewModel.dismissViewController = { + store.dispatch(FakespotAction.setAppearanceTo(false)) } view.configure(viewModel) return view @@ -482,11 +479,7 @@ class FakespotViewController: } private func triggerDismiss() { - delegate?.fakespotControllerDidDismiss(animated: true) - - if isReduxIntegrationEnabled { - store.dispatch(FakespotAction.toggleAppearance(false)) - } + store.dispatch(FakespotAction.dismiss) } deinit { diff --git a/Client/Frontend/FeltPrivacy/BrowserViewControllerState.swift b/Client/Frontend/FeltPrivacy/BrowserViewControllerState.swift new file mode 100644 index 000000000000..b86ad186d1c8 --- /dev/null +++ b/Client/Frontend/FeltPrivacy/BrowserViewControllerState.swift @@ -0,0 +1,59 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/ + +import Foundation +import Redux + +struct BrowserViewControllerState: ScreenState, Equatable { + var feltPrivacyState: FeltPrivacyState + var fakespotState: FakespotState + + init(_ appState: AppState) { + guard let bvcState = store.state.screenState( + BrowserViewControllerState.self, + for: .browserViewController) + else { + self.init() + return + } + + self.init(feltPrivacyState: bvcState.feltPrivacyState, + fakespotState: bvcState.fakespotState) + } + + init() { + self.init( + feltPrivacyState: FeltPrivacyState(), + fakespotState: FakespotState()) + } + + init( + feltPrivacyState: FeltPrivacyState, + fakespotState: FakespotState + ) { + self.feltPrivacyState = feltPrivacyState + self.fakespotState = fakespotState + } + + static let reducer: Reducer = { state, action in + switch action { + case FeltPrivacyAction.privateModeUpdated(let privacyState): + return BrowserViewControllerState( + feltPrivacyState: FeltPrivacyState.reducer(state.feltPrivacyState, action), + fakespotState: state.fakespotState) + case FakespotAction.pressedShoppingButton, + FakespotAction.show, + FakespotAction.dismiss: + return BrowserViewControllerState( + feltPrivacyState: state.feltPrivacyState, + fakespotState: FakespotState.reducer(state.fakespotState, action)) + case FakespotAction.setAppearanceTo(let isEnabled): + return BrowserViewControllerState( + feltPrivacyState: state.feltPrivacyState, + fakespotState: FakespotState.reducer(state.fakespotState, action)) + default: + return state + } + } +} diff --git a/Client/Frontend/Toolbar+URLBar/TabLocationView.swift b/Client/Frontend/Toolbar+URLBar/TabLocationView.swift index 9c5e5e76cdad..5f366f1f5eef 100644 --- a/Client/Frontend/Toolbar+URLBar/TabLocationView.swift +++ b/Client/Frontend/Toolbar+URLBar/TabLocationView.swift @@ -14,7 +14,6 @@ protocol TabLocationViewDelegate: AnyObject { func tabLocationViewDidTapShield(_ tabLocationView: TabLocationView) func tabLocationViewDidBeginDragInteraction(_ tabLocationView: TabLocationView) func tabLocationViewDidTapShare(_ tabLocationView: TabLocationView, button: UIButton) - func tabLocationViewDidTapShopping(_ tabLocationView: TabLocationView, button: UIButton) func tabLocationViewPresentCFR(at sourceView: UIView) /// - returns: whether the long-press was handled by the delegate; i.e. return `false` when the conditions for even starting handling long-press were not satisfied @@ -273,7 +272,8 @@ class TabLocationView: UIView, FeatureFlaggable { @objc func didPressShoppingButton(_ button: UIButton) { button.isSelected = true - delegate?.tabLocationViewDidTapShopping(self, button: button) + TelemetryWrapper.recordEvent(category: .action, method: .tap, object: .shoppingButton) + store.dispatch(FakespotAction.pressedShoppingButton) } @objc diff --git a/Client/Frontend/Toolbar+URLBar/URLBarView.swift b/Client/Frontend/Toolbar+URLBar/URLBarView.swift index ceaed93bdde5..9d290c50115a 100644 --- a/Client/Frontend/Toolbar+URLBar/URLBarView.swift +++ b/Client/Frontend/Toolbar+URLBar/URLBarView.swift @@ -46,7 +46,6 @@ protocol URLBarDelegate: AnyObject { func urlBarDisplayTextForURL(_ url: URL?) -> (String?, Bool) func urlBarDidBeginDragInteraction(_ urlBar: URLBarView) func urlBarDidPressShare(_ urlBar: URLBarView, shareView: UIView) - func urlBarDidPressShopping(_ urlBar: URLBarView, shoppingButton: UIButton) func urlBarPresentCFR(at sourceView: UIView) } @@ -763,10 +762,6 @@ extension URLBarView: TabLocationViewDelegate { delegate?.urlBarDidPressShare(self, shareView: button) } - func tabLocationViewDidTapShopping(_ tabLocationView: TabLocationView, button: UIButton) { - delegate?.urlBarDidPressShopping(self, shoppingButton: button) - } - func tabLocationViewPresentCFR(at sourceView: UIView) { delegate?.urlBarPresentCFR(at: sourceView) } diff --git a/firefox-ios/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swift b/firefox-ios/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swift index c2a01bf141c1..999d3db766af 100644 --- a/firefox-ios/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swift +++ b/firefox-ios/Tests/ClientTests/Coordinators/BrowserCoordinatorTests.swift @@ -836,7 +836,7 @@ final class BrowserCoordinatorTests: XCTestCase { subject.showFakespotFlowAsModal(productURL: URL(string: "www.example.com")!) let fakespotCoordinator = subject.childCoordinators[0] as! FakespotCoordinator - fakespotCoordinator.fakespotControllerDidDismiss(animated: false) + fakespotCoordinator.dismissModal(animated: false) XCTAssertEqual(mockRouter.dismissCalled, 1) XCTAssertTrue(subject.childCoordinators.isEmpty) diff --git a/firefox-ios/Tests/ClientTests/Coordinators/FakespotCoordinatorTests.swift b/firefox-ios/Tests/ClientTests/Coordinators/FakespotCoordinatorTests.swift index 45a1482a6157..22c99cee2225 100644 --- a/firefox-ios/Tests/ClientTests/Coordinators/FakespotCoordinatorTests.swift +++ b/firefox-ios/Tests/ClientTests/Coordinators/FakespotCoordinatorTests.swift @@ -64,8 +64,8 @@ final class FakespotCoordinatorTests: XCTestCase { subject.startSidebar(productURL: exampleProduct, sidebarContainer: sidebarContainer, parentViewController: viewController) - subject.fakespotControllerCloseSidebar(sidebarContainer: sidebarContainer, - parentViewController: viewController) + subject.closeSidebar(sidebarContainer: sidebarContainer, + parentViewController: viewController) XCTAssertEqual(sidebarContainer.hideSidebarCalled, 1) XCTAssertEqual(mockRouter.dismissCalled, 1) @@ -76,7 +76,7 @@ final class FakespotCoordinatorTests: XCTestCase { let subject = createSubject() subject.startModal(productURL: exampleProduct) - subject.fakespotControllerDidDismiss(animated: false) + subject.dismissModal(animated: false) XCTAssertEqual(mockRouter.dismissCalled, 1) XCTAssertTrue(subject.childCoordinators.isEmpty) diff --git a/firefox-ios/Tests/ClientTests/Fakespot/FakespotUtilsTests.swift b/firefox-ios/Tests/ClientTests/Fakespot/FakespotUtilsTests.swift index 111f1a3fad4e..ee5db8d72dfc 100644 --- a/firefox-ios/Tests/ClientTests/Fakespot/FakespotUtilsTests.swift +++ b/firefox-ios/Tests/ClientTests/Fakespot/FakespotUtilsTests.swift @@ -34,11 +34,13 @@ final class FakespotUtilsTests: XCTestCase { func testShouldDisplayInSidebar_withIphone() { let device = UIUserInterfaceIdiom.phone let keyWindow = UIWindow.attachedKeyWindow! + let isPortrait = true let orientation = UIDeviceOrientation.portrait let subject = FakespotUtils() XCTAssertFalse(subject.shouldDisplayInSidebar(device: device, window: keyWindow, viewSize: keyWindow.frame.size, + isPortrait: isPortrait, orientation: orientation), "Should return false for an iPhone") } @@ -46,11 +48,13 @@ final class FakespotUtilsTests: XCTestCase { func testShouldDisplayInSidebar_withIpad_landscape_fullScreen() { let device = UIUserInterfaceIdiom.pad let keyWindow = UIWindow.attachedKeyWindow! + let isPortrait = false let orientation = UIDeviceOrientation.landscapeLeft let subject = FakespotUtils() XCTAssertTrue(subject.shouldDisplayInSidebar(device: device, window: keyWindow, viewSize: keyWindow.frame.size, + isPortrait: isPortrait, orientation: orientation), "Should return true on iPad in landscape in full screen") } @@ -58,11 +62,13 @@ final class FakespotUtilsTests: XCTestCase { func testShouldDisplayInSidebar_withIpad_landscape_splitScreen() { let device = UIUserInterfaceIdiom.pad let keyWindow = UIWindow.attachedKeyWindow! + let isPortrait = false let orientation = UIDeviceOrientation.landscapeRight let subject = FakespotUtils() XCTAssertFalse(subject.shouldDisplayInSidebar(device: device, window: keyWindow, viewSize: CGSize.zero, + isPortrait: isPortrait, orientation: orientation), "Should return false on iPad in landscape in split screen") } @@ -70,11 +76,13 @@ final class FakespotUtilsTests: XCTestCase { func testShouldDisplayInSidebar_withIpad_portrait_fullScreen() { let device = UIUserInterfaceIdiom.pad let keyWindow = UIWindow.attachedKeyWindow! + let isPortrait = true let orientation = UIDeviceOrientation.portrait let subject = FakespotUtils() XCTAssertFalse(subject.shouldDisplayInSidebar(device: device, window: keyWindow, viewSize: keyWindow.frame.size, + isPortrait: isPortrait, orientation: orientation), "Should return false on iPad in portrait in full screen") } @@ -82,11 +90,13 @@ final class FakespotUtilsTests: XCTestCase { func testShouldDisplayInSidebar_withIpad_portrait_splitScreen() { let device = UIUserInterfaceIdiom.pad let keyWindow = UIWindow.attachedKeyWindow! + let isPortrait = true let orientation = UIDeviceOrientation.portrait let subject = FakespotUtils() XCTAssertFalse(subject.shouldDisplayInSidebar(device: device, window: keyWindow, viewSize: CGSize.zero, + isPortrait: isPortrait, orientation: orientation), "Should return false on iPad in portrait in split screen") } diff --git a/firefox-ios/Tests/ClientTests/TabTray/Legacy/TabLocationViewTests.swift b/firefox-ios/Tests/ClientTests/TabTray/Legacy/TabLocationViewTests.swift index 2dcc16e08426..5e6490a6702f 100644 --- a/firefox-ios/Tests/ClientTests/TabTray/Legacy/TabLocationViewTests.swift +++ b/firefox-ios/Tests/ClientTests/TabTray/Legacy/TabLocationViewTests.swift @@ -35,7 +35,6 @@ class MockTabLocationViewDelegate: TabLocationViewDelegate { func tabLocationViewDidTapShield(_ tabLocationView: TabLocationView) {} func tabLocationViewDidBeginDragInteraction(_ tabLocationView: TabLocationView) {} func tabLocationViewDidTapShare(_ tabLocationView: TabLocationView, button: UIButton) {} - func tabLocationViewDidTapShopping(_ tabLocationView: TabLocationView, button: UIButton) {} func tabLocationViewDidLongPressReaderMode(_ tabLocationView: TabLocationView) -> Bool { return false }