From 8aa4fa6a3a79d2295cd63440d8c745cedafc2fe6 Mon Sep 17 00:00:00 2001 From: Carson Ramsden Date: Wed, 1 Jan 2025 20:55:00 -0700 Subject: [PATCH] Refactor FXIOS-10908 [Homepage Rebuild] Move logic for cancelling edit from homepage to toolbar state (#23976) * Move logic for cancelling edit to toolbar state * Add pass through action to toolbar state * PR Feedback --- .../Toolbars/Redux/AddressBarState.swift | 11 ++++++ .../Toolbars/Redux/ToolbarAction.swift | 1 + .../Browser/Toolbars/Redux/ToolbarState.swift | 1 + .../HomepageViewController.swift | 21 ++--------- .../HomepageViewControllerTests.swift | 26 +++++++++++++ .../Toolbar/AddressBarStateTests.swift | 37 ++++++++++++------- 6 files changed, 67 insertions(+), 30 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift index 97a7320b76ba..69bb83014541 100644 --- a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift +++ b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift @@ -181,6 +181,9 @@ struct AddressBarState: StateType, Equatable { case ToolbarActionType.didStartEditingUrl: return handleDidStartEditingUrlAction(state: state, action: action) + case ToolbarActionType.cancelEditOnHomepage: + return handleCancelEditOnHomepageAction(state: state, action: action) + case ToolbarActionType.cancelEdit: return handleCancelEditAction(state: state, action: action) @@ -523,6 +526,14 @@ struct AddressBarState: StateType, Equatable { ) } + private static func handleCancelEditOnHomepageAction(state: Self, action: Action) -> Self { + if state.url == nil { + return handleCancelEditAction(state: state, action: action) + } else { + return handleHideKeyboardAction(state: state) + } + } + private static func handleCancelEditAction(state: Self, action: Action) -> Self { guard let toolbarAction = action as? ToolbarAction else { return defaultState(from: state) } diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarAction.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarAction.swift index 9a92374886a4..98c9865aadae 100644 --- a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarAction.swift +++ b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarAction.swift @@ -81,6 +81,7 @@ enum ToolbarActionType: ActionType { case showMenuWarningBadge case didPasteSearchTerm case didStartEditingUrl + case cancelEditOnHomepage case cancelEdit case hideKeyboard case readerModeStateChanged diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarState.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarState.swift index 81af138553a6..24d6ef216afd 100644 --- a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarState.swift +++ b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarState.swift @@ -114,6 +114,7 @@ struct ToolbarState: ScreenState, Equatable { case ToolbarActionType.borderPositionChanged, ToolbarActionType.urlDidChange, ToolbarActionType.didSetTextInLocationView, ToolbarActionType.didPasteSearchTerm, ToolbarActionType.didStartEditingUrl, ToolbarActionType.cancelEdit, + ToolbarActionType.cancelEditOnHomepage, ToolbarActionType.hideKeyboard, ToolbarActionType.websiteLoadingStateDidChange, ToolbarActionType.searchEngineDidChange, ToolbarActionType.clearSearch, ToolbarActionType.didDeleteSearchTerm, ToolbarActionType.didEnterSearchTerm, diff --git a/firefox-ios/Client/Frontend/Home/Homepage Rebuild/HomepageViewController.swift b/firefox-ios/Client/Frontend/Home/Homepage Rebuild/HomepageViewController.swift index 27afe99ac6ea..3e6a5f2c31ab 100644 --- a/firefox-ios/Client/Frontend/Home/Homepage Rebuild/HomepageViewController.swift +++ b/firefox-ios/Client/Frontend/Home/Homepage Rebuild/HomepageViewController.swift @@ -159,23 +159,10 @@ final class HomepageViewController: UIViewController, } private func handleToolbarStateOnScroll() { - // TODO: FXIOS-10877 This logic will be handled by toolbar state, the homepage will just dispatch the action - 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, - toolbarState.addressToolbar.isEditing { - // When the user scrolls the homepage (not overlaid on a webpage when searching) we cancel edit mode - // On a website we just dismiss the keyboard - if toolbarState.addressToolbar.url == nil { - let action = ToolbarAction(windowUUID: windowUUID, actionType: ToolbarActionType.cancelEdit) - store.dispatch(action) - } else { - let action = ToolbarAction(windowUUID: windowUUID, actionType: ToolbarActionType.hideKeyboard) - store.dispatch(action) - } - } + guard featureFlags.isFeatureEnabled(.toolbarRefactor, checking: .buildOnly) else { return } + // When the user scrolls the homepage (not overlaid on a webpage when searching) we cancel edit mode + let action = ToolbarAction(windowUUID: windowUUID, actionType: ToolbarActionType.cancelEditOnHomepage) + store.dispatch(action) } // MARK: - Redux diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Frontend/Homepage Rebuild/HomepageViewControllerTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Frontend/Homepage Rebuild/HomepageViewControllerTests.swift index 87441a7ce431..0a2942e03b79 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Frontend/Homepage Rebuild/HomepageViewControllerTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Frontend/Homepage Rebuild/HomepageViewControllerTests.swift @@ -15,6 +15,7 @@ final class HomepageViewControllerTests: XCTestCase, StoreTestUtility { override func setUp() { super.setUp() + LegacyFeatureFlagsManager.shared.initializeDeveloperFeatures(with: MockProfile()) DependencyHelperMock().bootstrapDependencies() setupStore() } @@ -144,6 +145,23 @@ final class HomepageViewControllerTests: XCTestCase, StoreTestUtility { XCTAssertEqual(actionType, GeneralBrowserMiddlewareActionType.websiteDidScroll) } + func test_scrollViewWillBeginDragging_triggersHomepageAction() throws { + let homepageVC = createSubject() + let scrollView = UIScrollView() + scrollView.contentOffset.y = 10 + setupNimbusToolbarRefactorTesting(isEnabled: true) + + homepageVC.scrollViewWillBeginDragging(scrollView) + + let actionCalled = try XCTUnwrap( + mockStore.dispatchedActions.first(where: { + $0 is ToolbarAction + }) as? ToolbarAction + ) + let actionType = try XCTUnwrap(actionCalled.actionType as? ToolbarActionType) + XCTAssertEqual(actionType, ToolbarActionType.cancelEditOnHomepage) + } + private func createSubject(statusBarScrollDelegate: StatusBarScrollDelegate? = nil) -> HomepageViewController { let notificationCenter = MockNotificationCenter() let themeManager = MockThemeManager() @@ -174,4 +192,12 @@ final class HomepageViewControllerTests: XCTestCase, StoreTestUtility { func resetStore() { StoreTestUtilityHelper.resetStore() } + + private func setupNimbusToolbarRefactorTesting(isEnabled: Bool) { + FxNimbus.shared.features.toolbarRefactorFeature.with { _, _ in + return ToolbarRefactorFeature( + enabled: isEnabled + ) + } + } } diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Toolbar/AddressBarStateTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Toolbar/AddressBarStateTests.swift index f87c80a834c0..2634b723f108 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Toolbar/AddressBarStateTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Toolbar/AddressBarStateTests.swift @@ -428,35 +428,46 @@ final class AddressBarStateTests: XCTestCase, StoreTestUtility { XCTAssertFalse(newState.showQRPageAction) } - func test_cancelEditAction_onHomepage_returnsExpectedState() { + func test_cancelEditOnHomepageAction_withURL_returnsExpectedState() { setupStore() let initialState = createSubject() let reducer = addressBarReducer() + let didChangeURLAction = ToolbarAction(url: URL(string: "https://mozilla.com")!, + windowUUID: windowUUID, + actionType: ToolbarActionType.urlDidChange + ) + + let stateWithURL = reducer(initialState, didChangeURLAction) let newState = reducer( - initialState, + stateWithURL, ToolbarAction( windowUUID: windowUUID, - actionType: ToolbarActionType.cancelEdit + actionType: ToolbarActionType.cancelEditOnHomepage ) ) XCTAssertEqual(newState.windowUUID, windowUUID) - XCTAssertEqual(newState.navigationActions.count, 0) + XCTAssertFalse(newState.shouldShowKeyboard) + XCTAssertEqual(newState.isEditing, initialState.isEditing) + } - XCTAssertEqual(newState.pageActions.count, 1) - XCTAssertEqual(newState.pageActions[0].actionType, .qrCode) + func test_cancelEditOnHomepageAction_withNoURL_returnsExpectedState() { + setupStore() + let initialState = createSubject() + let reducer = addressBarReducer() - XCTAssertEqual(newState.browserActions.count, 2) - XCTAssertEqual(newState.browserActions[0].actionType, .tabs) - XCTAssertEqual(newState.browserActions[1].actionType, .menu) + let newState = reducer( + initialState, + ToolbarAction( + windowUUID: windowUUID, + actionType: ToolbarActionType.cancelEditOnHomepage + ) + ) - XCTAssertEqual(newState.searchTerm, nil) + XCTAssertEqual(newState.windowUUID, windowUUID) XCTAssertFalse(newState.isEditing) XCTAssertTrue(newState.shouldShowKeyboard) - XCTAssertFalse(newState.shouldSelectSearchTerm) - XCTAssertFalse(newState.didStartTyping) - XCTAssertTrue(newState.showQRPageAction) } func test_cancelEditAction_withWebsite_returnsExpectedState() {