From b6cedec97ee9c6beb2eb059a4240deeb3aa22225 Mon Sep 17 00:00:00 2001 From: Razvan Litianu Date: Mon, 18 May 2026 11:23:59 +0300 Subject: [PATCH 1/3] Refactor FXIOS-15120 [Translations] Make leading page actions generic and decouple from ToolbarActions Move 5 translation lifecycle cases (didStartTranslatingPage, translationCompleted, receivedTranslationLanguage, didReceiveErrorTranslating, didTranslationSettingsChange) off ToolbarActionType onto TranslationsActionType. Teach AddressBarState.reducer to observe TranslationsAction so the translate icon updates without forcing translation state changes through the toolbar reducer. Collapse the toggleTranslationsEnabled double-dispatch in TranslationSettingsMiddleware into a single TranslationsAction. Both the toolbar reducer and the settings state reducer now react to the same action, removing the redux smell flagged in PR #32529. Remove the in-code apology comment on ToolbarActionType. --- .../Toolbars/Redux/AddressBarState.swift | 57 ++++-- .../Toolbars/Redux/ToolbarAction.swift | 7 - .../Browser/Toolbars/Redux/ToolbarState.swift | 39 ++-- .../TranslationSettingsMiddleware.swift | 13 +- .../TranslationSettingsState.swift | 10 + .../TranslationSettingsViewController.swift | 4 +- .../Translations/TranslationsAction.swift | 11 + .../Translations/TranslationsMiddleware.swift | 62 +++--- .../TranslationSettingsMiddlewareTests.swift | 50 ++--- .../TranslationSettingsStateTests.swift | 24 +++ .../Toolbar/AddressBarStateTests.swift | 12 +- .../TranslationsMiddlewareTests.swift | 192 +++++++++--------- 12 files changed, 269 insertions(+), 212 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift index ea1dcc0882eb6..2071579c3a3a8 100644 --- a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift +++ b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift @@ -146,11 +146,11 @@ struct AddressBarState: StateType, Sendable, Equatable { return handleDidSetTabScreenshotAction(state: state, action: action) // Translation related actions - case ToolbarActionType.didStartTranslatingPage, - ToolbarActionType.translationCompleted, - ToolbarActionType.receivedTranslationLanguage, - ToolbarActionType.didReceiveErrorTranslating, - ToolbarActionType.didTranslationSettingsChange: + case TranslationsActionType.didStartTranslatingPage, + TranslationsActionType.translationCompleted, + TranslationsActionType.receivedTranslationLanguage, + TranslationsActionType.didReceiveErrorTranslating, + TranslationsActionType.didTranslationSettingsChange: return handleLeadingPageChangedAction(state: state, action: action) case ToolbarActionType.didSummarizeSettingsChange: @@ -315,14 +315,14 @@ struct AddressBarState: StateType, Sendable, Equatable { @MainActor private static func handleLeadingPageChangedAction(state: Self, action: Action) -> Self { - guard let toolbarAction = action as? ToolbarAction else { + guard let translationsAction = action as? TranslationsAction else { return defaultState(from: state) } return AddressBarState( windowUUID: state.windowUUID, navigationActions: state.navigationActions, - leadingPageActions: leadingPageActions(action: toolbarAction, + leadingPageActions: leadingPageActions(action: translationsAction, addressBarState: state, isEditing: state.isEditing), trailingPageActions: state.trailingPageActions, @@ -340,7 +340,7 @@ struct AddressBarState: StateType, Sendable, Equatable { isLoading: state.isLoading, readerModeState: state.readerModeState, canSummarize: state.canSummarize, - translationConfiguration: toolbarAction.translationConfiguration, + translationConfiguration: translationsAction.translationConfiguration, didStartTyping: state.didStartTyping, isEmptySearch: state.isEmptySearch, alternativeSearchEngine: state.alternativeSearchEngine @@ -1104,7 +1104,7 @@ struct AddressBarState: StateType, Sendable, Equatable { @MainActor private static func leadingPageActions( - action: ToolbarAction, + action: Action, addressBarState: AddressBarState, isEditing: Bool = false ) -> [ToolbarActionConfiguration] { @@ -1114,11 +1114,14 @@ struct AddressBarState: StateType, Sendable, Equatable { !isEditing else { return actions } - let isShowingNavigationToolbar = action.isShowingNavigationToolbar ?? toolbarState.isShowingNavigationToolbar + let toolbarAction = action as? ToolbarAction + let actionTranslationConfiguration = translationConfiguration(from: action) + let isShowingNavigationToolbar = toolbarAction?.isShowingNavigationToolbar + ?? toolbarState.isShowingNavigationToolbar let isURLDidChangeAction = action.actionType as? ToolbarActionType == .urlDidChange - let isHomepage = (isURLDidChangeAction ? action.url : toolbarState.addressToolbar.url) == nil + let isHomepage = (isURLDidChangeAction ? toolbarAction?.url : toolbarState.addressToolbar.url) == nil let isLoadingChangeAction = action.actionType as? ToolbarActionType == .websiteLoadingStateDidChange - let isLoading = isLoadingChangeAction ? action.isLoading : addressBarState.isLoading + let isLoading = isLoadingChangeAction ? toolbarAction?.isLoading : addressBarState.isLoading let hasAlternativeLocationColor = shouldUseAlternativeLocationColor(action: action) if !isHomepage, !isShowingNavigationToolbar { @@ -1127,7 +1130,7 @@ struct AddressBarState: StateType, Sendable, Equatable { actions.append(shareAction) if let translationAction = configureTranslationIcon( - for: action, + actionTranslationConfiguration: actionTranslationConfiguration, addressBarState: addressBarState, isLoading: isLoading, hasAlternativeLocationColor: hasAlternativeLocationColor @@ -1140,7 +1143,7 @@ struct AddressBarState: StateType, Sendable, Equatable { actions.append(shareAction) if let translationAction = configureTranslationIcon( - for: action, + actionTranslationConfiguration: actionTranslationConfiguration, addressBarState: addressBarState, isLoading: isLoading, hasAlternativeLocationColor: hasAlternativeLocationColor @@ -1152,10 +1155,19 @@ struct AddressBarState: StateType, Sendable, Equatable { return actions } + // Pulls the `translationConfiguration` payload off whichever action carries it. + // Both `ToolbarAction` (e.g. urlDidChange) and `TranslationsAction` (e.g. didStartTranslatingPage) + // can drive the leading translate icon, so the reducer reads the field generically. + private static func translationConfiguration(from action: Action) -> TranslationConfiguration? { + if let toolbarAction = action as? ToolbarAction { return toolbarAction.translationConfiguration } + if let translationsAction = action as? TranslationsAction { return translationsAction.translationConfiguration } + return nil + } + // Checks whether we should show the translation icon based on the translation configuration // state and setups up the configuration for the translation icon on the toolbar (for iPad and iPhone) private static func configureTranslationIcon( - for action: ToolbarAction, + actionTranslationConfiguration: TranslationConfiguration?, addressBarState: AddressBarState, isLoading: Bool?, hasAlternativeLocationColor: Bool @@ -1167,13 +1179,13 @@ struct AddressBarState: StateType, Sendable, Equatable { // When the action explicitly provides a config, use it as the authority (e.g. settings toggle). // Only fall back to state when the action carries no config. let shouldShowTranslationIcon: Bool - if let actionConfig = action.translationConfiguration { + if let actionConfig = actionTranslationConfiguration { shouldShowTranslationIcon = actionConfig.isTranslationFeatureEnabled } else { shouldShowTranslationIcon = addressBarState.translationConfiguration?.isTranslationFeatureEnabled ?? false } guard shouldShowTranslationIcon else { return nil } - let iconState = action.translationConfiguration?.state ?? addressBarState.translationConfiguration?.state + let iconState = actionTranslationConfiguration?.state ?? addressBarState.translationConfiguration?.state guard let iconState else { return nil } return translateAction( enabled: isLoading == false, @@ -1312,12 +1324,12 @@ struct AddressBarState: StateType, Sendable, Equatable { // MARK: - Helper @MainActor - private static func toolbarPosition(action: ToolbarAction) -> AddressToolbarPosition? { + private static func toolbarPosition(action: Action) -> AddressToolbarPosition? { guard let toolbarState = store.state.componentState(ToolbarState.self, for: .toolbar, window: action.windowUUID) else { return nil } guard action.actionType as? ToolbarActionType == .toolbarPositionChanged, - let toolbarPosition = action.toolbarPosition + let toolbarPosition = (action as? ToolbarAction)?.toolbarPosition else { return toolbarState.toolbarPosition } @@ -1329,18 +1341,19 @@ struct AddressBarState: StateType, Sendable, Equatable { } @MainActor - private static func shouldUseAlternativeLocationColor(action: ToolbarAction) -> Bool { + private static func shouldUseAlternativeLocationColor(action: Action) -> Bool { guard let toolbarState = store.state.componentState(ToolbarState.self, for: .toolbar, window: action.windowUUID) else { return false } + let toolbarAction = action as? ToolbarAction let isTraitCollectionDidChangeAction = action.actionType as? ToolbarActionType == .traitCollectionDidChange let isShowingNavigationToolbar = if isTraitCollectionDidChangeAction { - action.isShowingNavigationToolbar ?? toolbarState.isShowingNavigationToolbar + toolbarAction?.isShowingNavigationToolbar ?? toolbarState.isShowingNavigationToolbar } else { toolbarState.isShowingNavigationToolbar } let isShowingTopTabs = if isTraitCollectionDidChangeAction { - action.isShowingTopTabs ?? toolbarState.isShowingTopTabs + toolbarAction?.isShowingTopTabs ?? toolbarState.isShowingTopTabs } else { toolbarState.isShowingTopTabs } diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarAction.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarAction.swift index 0f2d597cbdaa1..90bdfa04dd2d8 100644 --- a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarAction.swift +++ b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarAction.swift @@ -140,13 +140,6 @@ enum ToolbarActionType: ActionType { case didStartTyping case translucencyDidChange case navigationMiddleButtonDidChange - // Translations related actions that are needed to associate with the toolbar - // due to how our leadingPageActions are tied to ToolbarActions - case didStartTranslatingPage - case translationCompleted - case receivedTranslationLanguage - case didReceiveErrorTranslating - case didTranslationSettingsChange } struct ToolbarMiddlewareAction: Action { diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarState.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarState.swift index 0937ad9f61ff5..b20c94f9a1b4a 100644 --- a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarState.swift +++ b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/ToolbarState.swift @@ -157,11 +157,11 @@ struct ToolbarState: ScreenState, Sendable { ToolbarActionType.animationStateChanged, ToolbarActionType.translucencyDidChange, ToolbarActionType.scrollAlphaNeedsUpdate, ToolbarActionType.readerModeStateChanged, ToolbarActionType.navigationMiddleButtonDidChange, - ToolbarActionType.didStartTranslatingPage, - ToolbarActionType.translationCompleted, - ToolbarActionType.receivedTranslationLanguage, - ToolbarActionType.didReceiveErrorTranslating, - ToolbarActionType.didTranslationSettingsChange, + TranslationsActionType.didStartTranslatingPage, + TranslationsActionType.translationCompleted, + TranslationsActionType.receivedTranslationLanguage, + TranslationsActionType.didReceiveErrorTranslating, + TranslationsActionType.didTranslationSettingsChange, ToolbarActionType.didSummarizeSettingsChange: return handleToolbarUpdates(state: state, action: action) @@ -234,27 +234,32 @@ struct ToolbarState: ScreenState, Sendable { @MainActor private static func handleToolbarUpdates(state: Self, action: Action) -> ToolbarState { - guard let toolbarAction = action as? ToolbarAction else { return defaultState(from: state) } + // Both `ToolbarAction` (lifecycle) and `TranslationsAction` (translation events) reach this + // handler — translation actions carry only `isTranslationsEnabled` + payload, so every other + // ToolbarAction-only field falls back to state when the action is not a `ToolbarAction`. + let toolbarAction = action as? ToolbarAction + let translationsAction = action as? TranslationsAction + let actionIsTranslationsEnabled = toolbarAction?.isTranslationsEnabled ?? translationsAction?.isTranslationsEnabled return ToolbarState( windowUUID: state.windowUUID, toolbarPosition: state.toolbarPosition, toolbarLayout: state.toolbarLayout, tabTrayButtonStyle: state.tabTrayButtonStyle, - isPrivateMode: toolbarAction.isPrivate ?? state.isPrivateMode, - addressToolbar: AddressBarState.reducer(state.addressToolbar, toolbarAction), - navigationToolbar: NavigationBarState.reducer(state.navigationToolbar, toolbarAction), - isShowingNavigationToolbar: toolbarAction.isShowingNavigationToolbar ?? state.isShowingNavigationToolbar, - isShowingTopTabs: toolbarAction.isShowingTopTabs ?? state.isShowingTopTabs, - canGoBack: toolbarAction.canGoBack ?? state.canGoBack, - canGoForward: toolbarAction.canGoForward ?? state.canGoForward, + isPrivateMode: toolbarAction?.isPrivate ?? state.isPrivateMode, + addressToolbar: AddressBarState.reducer(state.addressToolbar, action), + navigationToolbar: NavigationBarState.reducer(state.navigationToolbar, action), + isShowingNavigationToolbar: toolbarAction?.isShowingNavigationToolbar ?? state.isShowingNavigationToolbar, + isShowingTopTabs: toolbarAction?.isShowingTopTabs ?? state.isShowingTopTabs, + canGoBack: toolbarAction?.canGoBack ?? state.canGoBack, + canGoForward: toolbarAction?.canGoForward ?? state.canGoForward, numberOfTabs: state.numberOfTabs, - scrollAlpha: toolbarAction.scrollAlpha ?? state.scrollAlpha, + scrollAlpha: toolbarAction?.scrollAlpha ?? state.scrollAlpha, showMenuWarningBadge: state.showMenuWarningBadge, canShowNavigationHint: state.canShowNavigationHint, - shouldAnimate: toolbarAction.shouldAnimate ?? state.shouldAnimate, - isTranslucent: toolbarAction.isTranslucent ?? state.isTranslucent, - isTranslationsEnabled: toolbarAction.isTranslationsEnabled ?? state.isTranslationsEnabled, + shouldAnimate: toolbarAction?.shouldAnimate ?? state.shouldAnimate, + isTranslucent: toolbarAction?.isTranslucent ?? state.isTranslucent, + isTranslationsEnabled: actionIsTranslationsEnabled ?? state.isTranslationsEnabled, previousTabScreenshot: state.previousTabScreenshot, nextTabScreenshot: state.nextTabScreenshot ) diff --git a/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsMiddleware.swift b/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsMiddleware.swift index 48d6c246d8e53..180286802a41a 100644 --- a/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsMiddleware.swift +++ b/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsMiddleware.swift @@ -74,16 +74,11 @@ final class TranslationSettingsMiddleware { from: "\(current)" ) } - store.dispatch(ToolbarAction( + store.dispatch(TranslationsAction( isTranslationsEnabled: newValue, translationConfiguration: TranslationConfiguration(prefs: prefs, isUserSettingEnabled: newValue), windowUUID: action.windowUUID, - actionType: ToolbarActionType.didTranslationSettingsChange - )) - store.dispatch(TranslationSettingsMiddlewareAction( - isTranslationsEnabled: newValue, - windowUUID: action.windowUUID, - actionType: TranslationSettingsMiddlewareActionType.didUpdateSettings + actionType: TranslationsActionType.didTranslationSettingsChange )) if !newValue { resetStorageTask?.cancel() @@ -105,10 +100,10 @@ final class TranslationSettingsMiddleware { actionType: TranslationSettingsMiddlewareActionType.didUpdateSettings )) if newValue { - store.dispatch(ToolbarAction( + store.dispatch(TranslationsAction( translationConfiguration: TranslationConfiguration(prefs: prefs), windowUUID: action.windowUUID, - actionType: ToolbarActionType.didTranslationSettingsChange + actionType: TranslationsActionType.didTranslationSettingsChange )) } diff --git a/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsState.swift b/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsState.swift index 43b5cd1b01977..5df3260b7efaf 100644 --- a/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsState.swift +++ b/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsState.swift @@ -93,6 +93,16 @@ struct TranslationSettingsState: ScreenState, Equatable { if let action = action as? TranslationSettingsViewAction { return reduceViewAction(state: state, action: action) } + if let action = action as? TranslationsAction, + action.actionType as? TranslationsActionType == .didTranslationSettingsChange { + // The settings middleware dispatches a single `TranslationsAction` for a toggle — + // both the toolbar reducer (icon visibility) and this reducer (settings UI toggle) + // read `isTranslationsEnabled` off the same dispatch instead of fanning out a second + // middleware-only update. + return state.copyWithUpdates( + isTranslationsEnabled: action.isTranslationsEnabled ?? state.isTranslationsEnabled + ) + } return defaultState(from: state) } diff --git a/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsViewController.swift b/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsViewController.swift index aaf56d164b2e9..d77fe899618d5 100644 --- a/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsViewController.swift +++ b/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsViewController.swift @@ -38,7 +38,7 @@ final class TranslationSettingsViewController: SettingsTableViewController { guard let self else { return } let isEnabled = self.prefs.boolForKey(PrefsKeys.Settings.translationsFeature) ?? true store.dispatch( - ToolbarAction( + TranslationsAction( isTranslationsEnabled: isEnabled, translationConfiguration: TranslationConfiguration( prefs: self.prefs, @@ -46,7 +46,7 @@ final class TranslationSettingsViewController: SettingsTableViewController { state: .inactive ), windowUUID: self.windowUUID, - actionType: ToolbarActionType.didTranslationSettingsChange + actionType: TranslationsActionType.didTranslationSettingsChange ) ) } diff --git a/firefox-ios/Client/Frontend/Translations/TranslationsAction.swift b/firefox-ios/Client/Frontend/Translations/TranslationsAction.swift index 411b5d04415b7..0e4133f0634ad 100644 --- a/firefox-ios/Client/Frontend/Translations/TranslationsAction.swift +++ b/firefox-ios/Client/Frontend/Translations/TranslationsAction.swift @@ -8,13 +8,19 @@ import Redux struct TranslationsAction: Action { let windowUUID: WindowUUID let actionType: ActionType + let isTranslationsEnabled: Bool? + let translationConfiguration: TranslationConfiguration? init( + isTranslationsEnabled: Bool? = nil, + translationConfiguration: TranslationConfiguration? = nil, windowUUID: WindowUUID, actionType: any ActionType, ) { self.windowUUID = windowUUID self.actionType = actionType + self.isTranslationsEnabled = isTranslationsEnabled + self.translationConfiguration = translationConfiguration } } @@ -42,4 +48,9 @@ enum TranslationsActionType: ActionType { case showAutoTranslatePrompt case didTapEnableAutoTranslate case didDismissAutoTranslatePrompt + case didStartTranslatingPage + case translationCompleted + case receivedTranslationLanguage + case didReceiveErrorTranslating + case didTranslationSettingsChange } diff --git a/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift b/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift index d137ccd494c6f..41a2c3fb99857 100644 --- a/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift +++ b/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift @@ -85,8 +85,8 @@ final class TranslationsMiddleware: FeatureFlaggable { case TranslationsActionType.didTapEnableAutoTranslate: self.profile.prefs.setBool(true, forKey: PrefsKeys.Settings.translationAutoTranslate) - case ToolbarActionType.didTranslationSettingsChange: - guard let action = (action as? ToolbarAction) else { return } + case TranslationsActionType.didTranslationSettingsChange: + guard let action = (action as? TranslationsAction) else { return } self.handleTranslationSettingsChange(action: action, windowUUID: windowUUID) default: @@ -94,7 +94,7 @@ final class TranslationsMiddleware: FeatureFlaggable { } } - private func handleTranslationSettingsChange(action: ToolbarAction, windowUUID: WindowUUID) { + private func handleTranslationSettingsChange(action: TranslationsAction, windowUUID: WindowUUID) { if action.isTranslationsEnabled == false { for uuid in translationFlowIds.keys { let translationState = store.state.componentState( @@ -328,12 +328,12 @@ final class TranslationsMiddleware: FeatureFlaggable { ) { let config = TranslationConfiguration(prefs: profile.prefs, state: state) persistTranslationConfig(config, on: tab ?? selectedTab(for: action.windowUUID)) - let toolbarAction = ToolbarAction( + let translationsAction = TranslationsAction( translationConfiguration: config, windowUUID: action.windowUUID, - actionType: ToolbarActionType.didStartTranslatingPage + actionType: TranslationsActionType.didStartTranslatingPage ) - store.dispatch(toolbarAction) + store.dispatch(translationsAction) } /// Mirrors the dispatched translation config onto the originating tab so it survives tab-tray @@ -350,7 +350,7 @@ final class TranslationsMiddleware: FeatureFlaggable { /// If auto-translate is enabled, triggers translation to the user's top preferred language. /// Returns `true` if auto-translation was initiated (caller should skip manual offer). - private func tryAutoTranslate(for action: ToolbarAction, on tab: Tab?) async -> Bool { + private func tryAutoTranslate(for action: Action, on tab: Tab?) async -> Bool { if restoringWindows.remove(action.windowUUID) != nil { return false } if let pendingLanguage = pendingLanguageSwitchTargets.removeValue(forKey: action.windowUUID) { @@ -422,14 +422,14 @@ final class TranslationsMiddleware: FeatureFlaggable { } /// Checks whether the current page in the active tab is eligible for translation, - /// and if so, dispatches a toolbar action to update the translation state. - private func checkTranslationsAreEligible(for action: ToolbarAction) { + /// and if so, dispatches a translation action to update the translation state. + private func checkTranslationsAreEligible(for action: Action) { // Pre-capture the tab so a tab switch mid-flight doesn't stomp the new active tab's state. let originatingTab = selectedTab(for: action.windowUUID) - // action.isTranslationsEnabled carries the explicit new value for settings-change actions - // (where store.state hasn't been updated yet). For urlDidChange it is nil, so we fall back - // to ToolbarState which is always up-to-date by the time urlDidChange fires. - let translationsEnabled = action.isTranslationsEnabled ?? (store.state.componentState( + // The action's `isTranslationsEnabled` carries the explicit new value for settings-change + // actions (where store.state hasn't been updated yet). For `urlDidChange` it is nil, so we + // fall back to ToolbarState which is always up-to-date by the time `urlDidChange` fires. + let translationsEnabled = isTranslationsEnabled(from: action) ?? (store.state.componentState( ToolbarState.self, for: .toolbar, window: action.windowUUID @@ -465,12 +465,12 @@ final class TranslationsMiddleware: FeatureFlaggable { // Auto-translate didn't run; offer manual translation instead. let config = TranslationConfiguration(prefs: profile.prefs, state: .inactive) self.persistTranslationConfig(config, on: originatingTab) - let toolbarAction = ToolbarAction( + let translationsAction = TranslationsAction( translationConfiguration: config, windowUUID: action.windowUUID, - actionType: ToolbarActionType.receivedTranslationLanguage + actionType: TranslationsActionType.receivedTranslationLanguage ) - store.dispatch(toolbarAction) + store.dispatch(translationsAction) } catch { let serviceError = TranslationsServiceError.fromUnknown(error) translationsTelemetry.pageLanguageIdentificationFailed( @@ -488,13 +488,29 @@ final class TranslationsMiddleware: FeatureFlaggable { private func dispatchClearTranslationIcon(windowUUID: WindowUUID, on tab: Tab? = nil) { persistTranslationConfig(nil, on: tab ?? selectedTab(for: windowUUID)) - store.dispatch(ToolbarAction( + store.dispatch(TranslationsAction( translationConfiguration: nil, windowUUID: windowUUID, - actionType: ToolbarActionType.receivedTranslationLanguage + actionType: TranslationsActionType.receivedTranslationLanguage )) } + // Pulls `translationConfiguration` from whichever action carries it. + // The lifecycle dispatchers swapped to `TranslationsAction`, but `urlDidChange` still + // arrives as `ToolbarAction`, so the eligibility entry point reads both shapes. + private func translationConfiguration(from action: Action) -> TranslationConfiguration? { + if let toolbarAction = action as? ToolbarAction { return toolbarAction.translationConfiguration } + if let translationsAction = action as? TranslationsAction { return translationsAction.translationConfiguration } + return nil + } + + // Pulls the optional `isTranslationsEnabled` payload from whichever action carries it. + private func isTranslationsEnabled(from action: Action) -> Bool? { + if let toolbarAction = action as? ToolbarAction { return toolbarAction.isTranslationsEnabled } + if let translationsAction = action as? TranslationsAction { return translationsAction.isTranslationsEnabled } + return nil + } + private func retrieveTranslations( for action: Action, targetLanguage: String, @@ -543,7 +559,7 @@ final class TranslationsMiddleware: FeatureFlaggable { ) try await translationsService.firstResponseReceived(for: action.windowUUID) dispatchAction( - for: ToolbarActionType.translationCompleted, + for: TranslationsActionType.translationCompleted, with: .active, translatedToLanguage: targetLanguage, sourceLanguage: detectedSourceLanguage, @@ -583,7 +599,7 @@ final class TranslationsMiddleware: FeatureFlaggable { // We also want to display a toast. private func handleErrorFromTranslatingPage(for action: Action, on tab: Tab? = nil) { dispatchAction( - for: ToolbarActionType.didReceiveErrorTranslating, + for: TranslationsActionType.didReceiveErrorTranslating, with: .inactive, and: action.windowUUID, on: tab @@ -592,7 +608,7 @@ final class TranslationsMiddleware: FeatureFlaggable { } private func dispatchAction( - for actionType: ToolbarActionType, + for actionType: TranslationsActionType, with state: TranslationConfiguration.IconState, translatedToLanguage: String? = nil, sourceLanguage: String? = nil, @@ -606,12 +622,12 @@ final class TranslationsMiddleware: FeatureFlaggable { sourceLanguage: sourceLanguage ) persistTranslationConfig(config, on: tab ?? selectedTab(for: windowUUID)) - let toolbarAction = ToolbarAction( + let translationsAction = TranslationsAction( translationConfiguration: config, windowUUID: windowUUID, actionType: actionType ) - store.dispatch(toolbarAction) + store.dispatch(translationsAction) } private func dispatchShowRetryTranslationToastAction( diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Settings/TranslationSettingsMiddlewareTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Settings/TranslationSettingsMiddlewareTests.swift index d3baee4d9600d..c304f77414c41 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Settings/TranslationSettingsMiddlewareTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Settings/TranslationSettingsMiddlewareTests.swift @@ -203,26 +203,21 @@ final class TranslationSettingsMiddlewareTests: XCTestCase, StoreTestUtility { actionType: TranslationSettingsViewActionType.toggleTranslationsEnabled ) - let expectation = XCTestExpectation(description: "wait for actions to dispatch") - expectation.expectedFulfillmentCount = 2 + let expectation = XCTestExpectation(description: "wait for action to dispatch") mockStore.dispatchCalled = { expectation.fulfill() } subject.translationSettingsProvider(mockStore.state, action) wait(for: [expectation], timeout: 1.0) - // Expects ToolbarAction + TranslationSettingsMiddlewareAction - XCTAssertEqual(mockStore.dispatchedActions.count, 2) - - let toolbarAction = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let toolbarActionType = try XCTUnwrap(toolbarAction.actionType as? ToolbarActionType) - XCTAssertEqual(toolbarActionType, ToolbarActionType.didTranslationSettingsChange) - XCTAssertNil(toolbarAction.translationConfiguration?.state) + // Single TranslationsAction.didTranslationSettingsChange — toolbar and settings reducers + // both react to the same dispatch (FXIOS-15120). + XCTAssertEqual(mockStore.dispatchedActions.count, 1) - let settingsAction = try XCTUnwrap(mockStore.dispatchedActions[1] as? TranslationSettingsMiddlewareAction) - let settingsActionType = try XCTUnwrap(settingsAction.actionType as? TranslationSettingsMiddlewareActionType) - XCTAssertEqual(settingsActionType, TranslationSettingsMiddlewareActionType.didUpdateSettings) - XCTAssertEqual(settingsAction.isTranslationsEnabled, false) + let translationsAction = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let actionType = try XCTUnwrap(translationsAction.actionType as? TranslationsActionType) + XCTAssertEqual(actionType, TranslationsActionType.didTranslationSettingsChange) + XCTAssertNil(translationsAction.translationConfiguration?.state) XCTAssertEqual(mockProfile.prefs.boolForKey(PrefsKeys.Settings.translationsFeature), false) subject.translationSettingsProvider = { _, _ in } } @@ -238,15 +233,12 @@ final class TranslationSettingsMiddlewareTests: XCTestCase, StoreTestUtility { subject.translationSettingsProvider(mockStore.state, action) - XCTAssertEqual(mockStore.dispatchedActions.count, 2) - - let toolbarAction = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let toolbarActionType = try XCTUnwrap(toolbarAction.actionType as? ToolbarActionType) - XCTAssertEqual(toolbarActionType, ToolbarActionType.didTranslationSettingsChange) - XCTAssertNil(toolbarAction.translationConfiguration?.state) + XCTAssertEqual(mockStore.dispatchedActions.count, 1) - let settingsAction = try XCTUnwrap(mockStore.dispatchedActions.last as? TranslationSettingsMiddlewareAction) - XCTAssertEqual(settingsAction.isTranslationsEnabled, true) + let translationsAction = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let actionType = try XCTUnwrap(translationsAction.actionType as? TranslationsActionType) + XCTAssertEqual(actionType, TranslationsActionType.didTranslationSettingsChange) + XCTAssertNil(translationsAction.translationConfiguration?.state) XCTAssertEqual(mockProfile.prefs.boolForKey(PrefsKeys.Settings.translationsFeature), true) subject.translationSettingsProvider = { _, _ in } } @@ -254,8 +246,7 @@ final class TranslationSettingsMiddlewareTests: XCTestCase, StoreTestUtility { func test_toggleTranslationsEnabled_whenEnabled_resetsStorage() { mockProfile.prefs.setBool(true, forKey: PrefsKeys.Settings.translationsFeature) - let expectation = XCTestExpectation(description: "wait for actions to dispatch") - expectation.expectedFulfillmentCount = 2 + let expectation = XCTestExpectation(description: "wait for action to dispatch") mockStore.dispatchCalled = { expectation.fulfill() } let resetExpectation = XCTestExpectation(description: "reset storage was called") @@ -271,15 +262,14 @@ final class TranslationSettingsMiddlewareTests: XCTestCase, StoreTestUtility { wait(for: [expectation, resetExpectation], timeout: 1.0) - XCTAssertEqual(mockStore.dispatchedActions.count, 2) + XCTAssertEqual(mockStore.dispatchedActions.count, 1) subject.translationSettingsProvider = { _, _ in } } func test_toggleTranslationsEnabled_whenDisabled_doesNotResetStorage() { mockProfile.prefs.setBool(false, forKey: PrefsKeys.Settings.translationsFeature) - let expectation = XCTestExpectation(description: "wait for actions to dispatch") - expectation.expectedFulfillmentCount = 2 + let expectation = XCTestExpectation(description: "wait for action to dispatch") expectation.assertForOverFulfill = true mockStore.dispatchCalled = { expectation.fulfill() } @@ -297,7 +287,7 @@ final class TranslationSettingsMiddlewareTests: XCTestCase, StoreTestUtility { wait(for: [expectation, resetExpectation], timeout: 2.0) - XCTAssertEqual(mockStore.dispatchedActions.count, 2) + XCTAssertEqual(mockStore.dispatchedActions.count, 1) subject.translationSettingsProvider = { _, _ in } } @@ -320,9 +310,9 @@ final class TranslationSettingsMiddlewareTests: XCTestCase, StoreTestUtility { XCTAssertEqual(firstActionType, TranslationSettingsMiddlewareActionType.didUpdateSettings) XCTAssertEqual(firstAction.isAutoTranslateEnabled, true) XCTAssertEqual(mockProfile.prefs.boolForKey(PrefsKeys.Settings.translationAutoTranslate), true) - let secondAction = try XCTUnwrap(mockStore.dispatchedActions[1] as? ToolbarAction) - let secondActionType = try XCTUnwrap(secondAction.actionType as? ToolbarActionType) - XCTAssertEqual(secondActionType, ToolbarActionType.didTranslationSettingsChange) + let secondAction = try XCTUnwrap(mockStore.dispatchedActions[1] as? TranslationsAction) + let secondActionType = try XCTUnwrap(secondAction.actionType as? TranslationsActionType) + XCTAssertEqual(secondActionType, TranslationsActionType.didTranslationSettingsChange) subject.translationSettingsProvider = { _, _ in } } diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Settings/TranslationSettingsStateTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Settings/TranslationSettingsStateTests.swift index f0ddb4f73a5a9..63fd704d450bb 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Settings/TranslationSettingsStateTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Settings/TranslationSettingsStateTests.swift @@ -280,6 +280,30 @@ final class TranslationSettingsStateTests: XCTestCase { XCTAssertEqual(newState.supportedLanguages, ["en", "fr", "de"]) } + // MARK: - Reducer Tests - TranslationsAction.didTranslationSettingsChange + + /// FXIOS-15120: the settings reducer reacts to the same `TranslationsAction` the toolbar listens + /// to, so the toggle flow only needs a single dispatch. + func test_didTranslationSettingsChange_updatesTranslationsEnabledFromAction() { + let initialState = TranslationSettingsState( + windowUUID: .XCTestDefaultUUID, + isTranslationsEnabled: false, + preferredLanguages: [], + supportedLanguages: [] + ) + let reducer = translationSettingsReducer() + + let action = TranslationsAction( + isTranslationsEnabled: true, + windowUUID: .XCTestDefaultUUID, + actionType: TranslationsActionType.didTranslationSettingsChange + ) + + let newState = reducer(initialState, action) + + XCTAssertTrue(newState.isTranslationsEnabled) + } + // MARK: - Equality Tests func test_equality_sameValues_returnsTrue() { 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 c0c1010ac7adf..93576e2f12b4d 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Toolbar/AddressBarStateTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Toolbar/AddressBarStateTests.swift @@ -596,10 +596,10 @@ final class AddressBarStateTests: XCTestCase, StoreTestUtility { let stateWithInactiveIcon = reducer( initialState, - ToolbarAction( + TranslationsAction( translationConfiguration: TranslationConfiguration(prefs: mockProfile.prefs, state: .inactive), windowUUID: windowUUID, - actionType: ToolbarActionType.receivedTranslationLanguage + actionType: TranslationsActionType.receivedTranslationLanguage ) ) @@ -625,10 +625,10 @@ final class AddressBarStateTests: XCTestCase, StoreTestUtility { let stateWithInactiveIcon = reducer( initialState, - ToolbarAction( + TranslationsAction( translationConfiguration: TranslationConfiguration(prefs: mockProfile.prefs, state: .inactive), windowUUID: windowUUID, - actionType: ToolbarActionType.receivedTranslationLanguage + actionType: TranslationsActionType.receivedTranslationLanguage ) ) @@ -654,14 +654,14 @@ final class AddressBarStateTests: XCTestCase, StoreTestUtility { // Simulate the previous tab's `.active` state still in Redux at the moment of switch. let stateWithActiveIcon = reducer( initialState, - ToolbarAction( + TranslationsAction( translationConfiguration: TranslationConfiguration( prefs: mockProfile.prefs, state: .active, translatedToLanguage: "fr" ), windowUUID: windowUUID, - actionType: ToolbarActionType.translationCompleted + actionType: TranslationsActionType.translationCompleted ) ) diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/TranslationsTests/TranslationsMiddlewareTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/TranslationsTests/TranslationsMiddlewareTests.swift index 0b9d7ec6bf624..8a6cfb212c646 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/TranslationsTests/TranslationsMiddlewareTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/TranslationsTests/TranslationsMiddlewareTests.swift @@ -114,11 +114,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [expectation], timeout: 1.0) - let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let actionType = try XCTUnwrap(actionCalled.actionType as? ToolbarActionType) + let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let actionType = try XCTUnwrap(actionCalled.actionType as? TranslationsActionType) XCTAssertEqual(actionCalled.translationConfiguration?.state, .inactive) - XCTAssertEqual(actionType, ToolbarActionType.receivedTranslationLanguage) + XCTAssertEqual(actionType, TranslationsActionType.receivedTranslationLanguage) XCTAssertEqual(mockStore.dispatchedActions.count, 1) XCTAssertNil(mockTranslationsTelemetry.lastTranslationFlowId) XCTAssertEqual(mockTranslationsTelemetry.pageLanguageIdentificationFailedCalledCount, 0) @@ -144,11 +144,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [expectation], timeout: 1.0) - let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let actionType = try XCTUnwrap(actionCalled.actionType as? ToolbarActionType) + let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let actionType = try XCTUnwrap(actionCalled.actionType as? TranslationsActionType) XCTAssertEqual(actionCalled.translationConfiguration?.state, .inactive) - XCTAssertEqual(actionType, ToolbarActionType.receivedTranslationLanguage) + XCTAssertEqual(actionType, TranslationsActionType.receivedTranslationLanguage) XCTAssertEqual(mockStore.dispatchedActions.count, 1) } @@ -234,11 +234,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [expectation], timeout: 1.0) - let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let actionType = try XCTUnwrap(actionCalled.actionType as? ToolbarActionType) + let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let actionType = try XCTUnwrap(actionCalled.actionType as? TranslationsActionType) XCTAssertNil(actionCalled.translationConfiguration) - XCTAssertEqual(actionType, ToolbarActionType.receivedTranslationLanguage) + XCTAssertEqual(actionType, TranslationsActionType.receivedTranslationLanguage) XCTAssertEqual(mockStore.dispatchedActions.count, 1) } @@ -321,8 +321,8 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [expectation], timeout: 1.0) XCTAssertEqual(mockStore.dispatchedActions.count, 1) - let dispatched = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - XCTAssertEqual(dispatched.actionType as? ToolbarActionType, .receivedTranslationLanguage) + let dispatched = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + XCTAssertEqual(dispatched.actionType as? TranslationsActionType, .receivedTranslationLanguage) } /// Eligible page persists `.inactive` to the tab's store entry. @@ -407,9 +407,9 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [expectation], timeout: 1.0) - let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) + let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) XCTAssertNil(actionCalled.translationConfiguration) - XCTAssertEqual(actionCalled.actionType as? ToolbarActionType, .receivedTranslationLanguage) + XCTAssertEqual(actionCalled.actionType as? TranslationsActionType, .receivedTranslationLanguage) XCTAssertNil(tab.translationConfiguration) } @@ -439,9 +439,9 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [expectation], timeout: 1.0) - let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) + let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) XCTAssertNil(actionCalled.translationConfiguration) - XCTAssertEqual(actionCalled.actionType as? ToolbarActionType, .receivedTranslationLanguage) + XCTAssertEqual(actionCalled.actionType as? TranslationsActionType, .receivedTranslationLanguage) XCTAssertNil(tab.translationConfiguration) } @@ -467,7 +467,7 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility let expectation = XCTestExpectation(description: "receivedTranslationLanguage dispatched") mockStore.dispatchCalled = { [weak mockStore] in - if (mockStore?.dispatchedActions.last?.actionType as? ToolbarActionType) == .receivedTranslationLanguage { + if (mockStore?.dispatchedActions.last?.actionType as? TranslationsActionType) == .receivedTranslationLanguage { expectation.fulfill() } } @@ -498,7 +498,7 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility let completedExpectation = XCTestExpectation(description: "translationCompleted dispatched") mockStore.dispatchCalled = { [weak mockStore] in - if (mockStore?.dispatchedActions.last?.actionType as? ToolbarActionType) == .translationCompleted { + if (mockStore?.dispatchedActions.last?.actionType as? TranslationsActionType) == .translationCompleted { completedExpectation.fulfill() } } @@ -529,7 +529,7 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility let completedExpectation = XCTestExpectation(description: "translationCompleted dispatched") mockStore.dispatchCalled = { [weak mockStore] in - if (mockStore?.dispatchedActions.last?.actionType as? ToolbarActionType) == .translationCompleted { + if (mockStore?.dispatchedActions.last?.actionType as? TranslationsActionType) == .translationCompleted { completedExpectation.fulfill() } } @@ -564,7 +564,7 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility let errorExpectation = XCTestExpectation(description: "didReceiveErrorTranslating dispatched") mockStore.dispatchCalled = { [weak mockStore] in - if (mockStore?.dispatchedActions.last?.actionType as? ToolbarActionType) == .didReceiveErrorTranslating { + if (mockStore?.dispatchedActions.last?.actionType as? TranslationsActionType) == .didReceiveErrorTranslating { errorExpectation.fulfill() } } @@ -630,7 +630,7 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility let didStartExpectation = XCTestExpectation(description: "didStartTranslatingPage dispatched") let completedExpectation = XCTestExpectation(description: "translationCompleted dispatched") mockStore.dispatchCalled = { [weak mockStore] in - guard let type = mockStore?.dispatchedActions.last?.actionType as? ToolbarActionType else { return } + guard let type = mockStore?.dispatchedActions.last?.actionType as? TranslationsActionType else { return } switch type { case .didStartTranslatingPage: didStartExpectation.fulfill() case .translationCompleted: completedExpectation.fulfill() @@ -641,12 +641,12 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [didStartExpectation, completedExpectation], timeout: 3.0, enforceOrder: true) - let toolbarActions = mockStore.dispatchedActions.compactMap { $0 as? ToolbarAction } + let toolbarActions = mockStore.dispatchedActions.compactMap { $0 as? TranslationsAction } let didStart = try XCTUnwrap(toolbarActions.first { - ($0.actionType as? ToolbarActionType) == .didStartTranslatingPage + ($0.actionType as? TranslationsActionType) == .didStartTranslatingPage }) let completed = try XCTUnwrap(toolbarActions.first { - ($0.actionType as? ToolbarActionType) == .translationCompleted + ($0.actionType as? TranslationsActionType) == .translationCompleted }) XCTAssertEqual(didStart.translationConfiguration?.state, .loading) @@ -702,19 +702,19 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 3) - let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? ToolbarAction) - let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? ToolbarActionType) + let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? TranslationsAction) + let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? TranslationsActionType) - let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? ToolbarAction) - let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? ToolbarActionType) + let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? TranslationsAction) + let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? TranslationsActionType) let thirdActionCalled = try XCTUnwrap(mockStore.dispatchedActions[2] as? GeneralBrowserAction) let thirdActionType = try XCTUnwrap(thirdActionCalled.actionType as? GeneralBrowserActionType) XCTAssertEqual(firstActionCalled.translationConfiguration?.state, .loading) - XCTAssertEqual(firstActionType, ToolbarActionType.didStartTranslatingPage) + XCTAssertEqual(firstActionType, TranslationsActionType.didStartTranslatingPage) XCTAssertEqual(secondActionCalled.translationConfiguration?.state, .inactive) - XCTAssertEqual(secondActionType, ToolbarActionType.didReceiveErrorTranslating) + XCTAssertEqual(secondActionType, TranslationsActionType.didReceiveErrorTranslating) XCTAssertEqual(thirdActionCalled.toastType, .retryTranslatingPage) XCTAssertEqual(thirdActionType, GeneralBrowserActionType.showToast) @@ -752,19 +752,19 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 3) - let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? ToolbarAction) - let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? ToolbarActionType) + let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? TranslationsAction) + let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? TranslationsActionType) - let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? ToolbarAction) - let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? ToolbarActionType) + let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? TranslationsAction) + let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? TranslationsActionType) let thirdActionCalled = try XCTUnwrap(mockStore.dispatchedActions[2] as? GeneralBrowserAction) let thirdActionType = try XCTUnwrap(thirdActionCalled.actionType as? GeneralBrowserActionType) XCTAssertEqual(firstActionCalled.translationConfiguration?.state, .loading) - XCTAssertEqual(firstActionType, ToolbarActionType.didStartTranslatingPage) + XCTAssertEqual(firstActionType, TranslationsActionType.didStartTranslatingPage) XCTAssertEqual(secondActionCalled.translationConfiguration?.state, .inactive) - XCTAssertEqual(secondActionType, ToolbarActionType.didReceiveErrorTranslating) + XCTAssertEqual(secondActionType, TranslationsActionType.didReceiveErrorTranslating) XCTAssertEqual(thirdActionCalled.toastType, .retryTranslatingPage) XCTAssertEqual(thirdActionType, GeneralBrowserActionType.showToast) @@ -803,14 +803,14 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 2) - let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? ToolbarAction) - let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? ToolbarActionType) + let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? TranslationsAction) + let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? TranslationsActionType) let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? GeneralBrowserAction) let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? GeneralBrowserActionType) XCTAssertEqual(firstActionCalled.translationConfiguration?.state, .inactive) - XCTAssertEqual(firstActionType, ToolbarActionType.didStartTranslatingPage) + XCTAssertEqual(firstActionType, TranslationsActionType.didStartTranslatingPage) XCTAssertEqual(secondActionType, GeneralBrowserActionType.reloadWebsite) XCTAssertEqual(mockTranslationsTelemetry.lastActionType, .willRestore) XCTAssertEqual(mockTranslationsTelemetry.webpageRestoredCalledCount, 1) @@ -853,10 +853,10 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility setTranslationsFeatureEnabled(enabled: true) let mockTranslationService = MockTranslationsService(shouldOfferTranslationResult: .success(true)) let subject = createSubject(translationsService: mockTranslationService) - let action = ToolbarAction( + let action = TranslationsAction( translationConfiguration: TranslationConfiguration(prefs: mockProfile.prefs), windowUUID: .XCTestDefaultUUID, - actionType: ToolbarActionType.didTranslationSettingsChange + actionType: TranslationsActionType.didTranslationSettingsChange ) let expectation = XCTestExpectation(description: "receivedTranslationLanguage dispatched after feature enabled") @@ -866,9 +866,9 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [expectation], timeout: 1.0) - let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let actionType = try XCTUnwrap(actionCalled.actionType as? ToolbarActionType) - XCTAssertEqual(actionType, ToolbarActionType.receivedTranslationLanguage) + let actionCalled = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let actionType = try XCTUnwrap(actionCalled.actionType as? TranslationsActionType) + XCTAssertEqual(actionType, TranslationsActionType.receivedTranslationLanguage) XCTAssertEqual(actionCalled.translationConfiguration?.state, .inactive) XCTAssertEqual(mockStore.dispatchedActions.count, 1) } @@ -877,11 +877,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility setTranslationsFeatureEnabled(enabled: true) let mockTranslationService = MockTranslationsService(shouldOfferTranslationResult: .success(true)) let subject = createSubject(translationsService: mockTranslationService) - let action = ToolbarAction( + let action = TranslationsAction( isTranslationsEnabled: false, translationConfiguration: TranslationConfiguration(prefs: mockProfile.prefs, isUserSettingEnabled: false), windowUUID: .XCTestDefaultUUID, - actionType: ToolbarActionType.didTranslationSettingsChange + actionType: TranslationsActionType.didTranslationSettingsChange ) let expectation = XCTestExpectation(description: "no action dispatched when feature disabled") @@ -903,11 +903,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility seedTargetLanguage(in: subject, successDispatchCount: 2) mockStore.state = setupAppState(translationState: .active) - let action = ToolbarAction( + let action = TranslationsAction( isTranslationsEnabled: false, translationConfiguration: TranslationConfiguration(prefs: mockProfile.prefs, isUserSettingEnabled: false), windowUUID: .XCTestDefaultUUID, - actionType: ToolbarActionType.didTranslationSettingsChange + actionType: TranslationsActionType.didTranslationSettingsChange ) let expectation = XCTestExpectation(description: "reloadWebsite dispatched when disabling translations") @@ -932,11 +932,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility seedTargetLanguage(in: subject, successDispatchCount: 2) mockStore.state = setupAppState(translationState: .inactive) - let action = ToolbarAction( + let action = TranslationsAction( isTranslationsEnabled: false, translationConfiguration: TranslationConfiguration(prefs: mockProfile.prefs, isUserSettingEnabled: false), windowUUID: .XCTestDefaultUUID, - actionType: ToolbarActionType.didTranslationSettingsChange + actionType: TranslationsActionType.didTranslationSettingsChange ) let expectation = XCTestExpectation(description: "no reload dispatched for inactive translation") @@ -955,11 +955,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility seedTargetLanguage(in: subject, successDispatchCount: 2) - let toggleAction = ToolbarAction( + let toggleAction = TranslationsAction( isTranslationsEnabled: false, translationConfiguration: TranslationConfiguration(prefs: mockProfile.prefs, isUserSettingEnabled: false), windowUUID: .XCTestDefaultUUID, - actionType: ToolbarActionType.didTranslationSettingsChange + actionType: TranslationsActionType.didTranslationSettingsChange ) subject.translationsProvider(mockStore.state, toggleAction) mockStore.dispatchedActions.removeAll() @@ -1008,16 +1008,16 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 2) - let firstAction = try XCTUnwrap(mockStore.dispatchedActions[0] as? ToolbarAction) - let firstActionType = try XCTUnwrap(firstAction.actionType as? ToolbarActionType) + let firstAction = try XCTUnwrap(mockStore.dispatchedActions[0] as? TranslationsAction) + let firstActionType = try XCTUnwrap(firstAction.actionType as? TranslationsActionType) - let secondAction = try XCTUnwrap(mockStore.dispatchedActions[1] as? ToolbarAction) - let secondActionType = try XCTUnwrap(secondAction.actionType as? ToolbarActionType) + let secondAction = try XCTUnwrap(mockStore.dispatchedActions[1] as? TranslationsAction) + let secondActionType = try XCTUnwrap(secondAction.actionType as? TranslationsActionType) XCTAssertEqual(firstAction.translationConfiguration?.state, .loading) - XCTAssertEqual(firstActionType, ToolbarActionType.didStartTranslatingPage) + XCTAssertEqual(firstActionType, TranslationsActionType.didStartTranslatingPage) XCTAssertEqual(secondAction.translationConfiguration?.state, .active) - XCTAssertEqual(secondActionType, ToolbarActionType.translationCompleted) + XCTAssertEqual(secondActionType, TranslationsActionType.translationCompleted) } func test_urlDidChangeAction_withAutoTranslateEnabled_andNoPreferredLanguages_offersManualTranslation() throws { @@ -1044,11 +1044,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 1) - let dispatchedAction = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let dispatchedActionType = try XCTUnwrap(dispatchedAction.actionType as? ToolbarActionType) + let dispatchedAction = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let dispatchedActionType = try XCTUnwrap(dispatchedAction.actionType as? TranslationsActionType) XCTAssertEqual(dispatchedAction.translationConfiguration?.state, .inactive) - XCTAssertEqual(dispatchedActionType, ToolbarActionType.receivedTranslationLanguage) + XCTAssertEqual(dispatchedActionType, TranslationsActionType.receivedTranslationLanguage) } func test_urlDidChangeAction_withAutoTranslateEnabled_afterRestore_skipsAutoTranslateOnce() throws { @@ -1091,11 +1091,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 1) - let dispatchedAction = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let dispatchedActionType = try XCTUnwrap(dispatchedAction.actionType as? ToolbarActionType) + let dispatchedAction = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let dispatchedActionType = try XCTUnwrap(dispatchedAction.actionType as? TranslationsActionType) XCTAssertEqual(dispatchedAction.translationConfiguration?.state, .inactive) - XCTAssertEqual(dispatchedActionType, ToolbarActionType.receivedTranslationLanguage) + XCTAssertEqual(dispatchedActionType, TranslationsActionType.receivedTranslationLanguage) } func test_urlDidChangeAction_withAutoTranslateEnabled_andPageInPreferredLanguages_skipsAutoTranslate() throws { @@ -1126,11 +1126,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 1) - let dispatchedAction = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let dispatchedActionType = try XCTUnwrap(dispatchedAction.actionType as? ToolbarActionType) + let dispatchedAction = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let dispatchedActionType = try XCTUnwrap(dispatchedAction.actionType as? TranslationsActionType) XCTAssertEqual(dispatchedAction.translationConfiguration?.state, .inactive) - XCTAssertEqual(dispatchedActionType, ToolbarActionType.receivedTranslationLanguage) + XCTAssertEqual(dispatchedActionType, TranslationsActionType.receivedTranslationLanguage) } // MARK: - maybeShowAutoTranslatePrompt tests @@ -1185,8 +1185,8 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 2) - let lastActionType = try XCTUnwrap(mockStore.dispatchedActions.last?.actionType as? ToolbarActionType) - XCTAssertEqual(lastActionType, ToolbarActionType.translationCompleted) + let lastActionType = try XCTUnwrap(mockStore.dispatchedActions.last?.actionType as? TranslationsActionType) + XCTAssertEqual(lastActionType, TranslationsActionType.translationCompleted) } func test_translationCompleted_whenAutoTranslateAlreadyEnabled_doesNotDispatchShowPrompt() throws { @@ -1211,8 +1211,8 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 2) - let lastActionType = try XCTUnwrap(mockStore.dispatchedActions.last?.actionType as? ToolbarActionType) - XCTAssertEqual(lastActionType, ToolbarActionType.translationCompleted) + let lastActionType = try XCTUnwrap(mockStore.dispatchedActions.last?.actionType as? TranslationsActionType) + XCTAssertEqual(lastActionType, TranslationsActionType.translationCompleted) } // MARK: - didTapRetryFailedTranslation tests @@ -1273,17 +1273,17 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 2) - let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? ToolbarAction) - let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? ToolbarActionType) + let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? TranslationsAction) + let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? TranslationsActionType) - let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? ToolbarAction) - let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? ToolbarActionType) + let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? TranslationsAction) + let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? TranslationsActionType) XCTAssertEqual(firstActionCalled.translationConfiguration?.state, .loading) - XCTAssertEqual(firstActionType, ToolbarActionType.didStartTranslatingPage) + XCTAssertEqual(firstActionType, TranslationsActionType.didStartTranslatingPage) XCTAssertEqual(secondActionCalled.translationConfiguration?.state, .active) - XCTAssertEqual(secondActionType, ToolbarActionType.translationCompleted) + XCTAssertEqual(secondActionType, TranslationsActionType.translationCompleted) XCTAssertEqual(mockTranslationsTelemetry.pageLanguageIdentifiedCalledCount, 1) } @@ -1317,11 +1317,11 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [expectation], timeout: 1.0) - let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? ToolbarAction) - let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? ToolbarActionType) + let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? TranslationsAction) + let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? TranslationsActionType) - let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? ToolbarAction) - let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? ToolbarActionType) + let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? TranslationsAction) + let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? TranslationsActionType) let thirdActionCalled = try XCTUnwrap(mockStore.dispatchedActions[2] as? GeneralBrowserAction) let thirdActionType = try XCTUnwrap(thirdActionCalled.actionType as? GeneralBrowserActionType) @@ -1329,9 +1329,9 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 3) XCTAssertEqual(firstActionCalled.translationConfiguration?.state, .loading) - XCTAssertEqual(firstActionType, ToolbarActionType.didStartTranslatingPage) + XCTAssertEqual(firstActionType, TranslationsActionType.didStartTranslatingPage) XCTAssertEqual(secondActionCalled.translationConfiguration?.state, .inactive) - XCTAssertEqual(secondActionType, ToolbarActionType.didReceiveErrorTranslating) + XCTAssertEqual(secondActionType, TranslationsActionType.didReceiveErrorTranslating) XCTAssertEqual(thirdActionCalled.toastType, .retryTranslatingPage) XCTAssertEqual(thirdActionType, GeneralBrowserActionType.showToast) @@ -1370,19 +1370,19 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility XCTAssertEqual(mockStore.dispatchedActions.count, 3) - let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? ToolbarAction) - let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? ToolbarActionType) + let firstActionCalled = try XCTUnwrap(mockStore.dispatchedActions[0] as? TranslationsAction) + let firstActionType = try XCTUnwrap(firstActionCalled.actionType as? TranslationsActionType) - let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? ToolbarAction) - let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? ToolbarActionType) + let secondActionCalled = try XCTUnwrap(mockStore.dispatchedActions[1] as? TranslationsAction) + let secondActionType = try XCTUnwrap(secondActionCalled.actionType as? TranslationsActionType) let thirdActionCalled = try XCTUnwrap(mockStore.dispatchedActions[2] as? GeneralBrowserAction) let thirdActionType = try XCTUnwrap(thirdActionCalled.actionType as? GeneralBrowserActionType) XCTAssertEqual(firstActionCalled.translationConfiguration?.state, .loading) - XCTAssertEqual(firstActionType, ToolbarActionType.didStartTranslatingPage) + XCTAssertEqual(firstActionType, TranslationsActionType.didStartTranslatingPage) XCTAssertEqual(secondActionCalled.translationConfiguration?.state, .inactive) - XCTAssertEqual(secondActionType, ToolbarActionType.didReceiveErrorTranslating) + XCTAssertEqual(secondActionType, TranslationsActionType.didReceiveErrorTranslating) XCTAssertEqual(thirdActionCalled.toastType, .retryTranslatingPage) XCTAssertEqual(thirdActionType, GeneralBrowserActionType.showToast) @@ -1406,7 +1406,7 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility translatedToLanguage: String, sourceLanguage: String? = nil ) -> AppState { - let action = ToolbarAction( + let action = TranslationsAction( translationConfiguration: TranslationConfiguration( prefs: mockProfile.prefs, state: .active, @@ -1414,7 +1414,7 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility sourceLanguage: sourceLanguage ), windowUUID: .XCTestDefaultUUID, - actionType: ToolbarActionType.translationCompleted + actionType: TranslationsActionType.translationCompleted ) return AppState.reducer(mockStore.state, action) } @@ -1648,14 +1648,14 @@ final class TranslationsMiddlewareIntegrationTests: XCTestCase, StoreTestUtility wait(for: [expectation], timeout: 1.0) - let startAction = try XCTUnwrap(mockStore.dispatchedActions.first as? ToolbarAction) - let startActionType = try XCTUnwrap(startAction.actionType as? ToolbarActionType) - XCTAssertEqual(startActionType, ToolbarActionType.didStartTranslatingPage) + let startAction = try XCTUnwrap(mockStore.dispatchedActions.first as? TranslationsAction) + let startActionType = try XCTUnwrap(startAction.actionType as? TranslationsActionType) + XCTAssertEqual(startActionType, TranslationsActionType.didStartTranslatingPage) XCTAssertEqual(startAction.translationConfiguration?.state, .loading) - let completedAction = try XCTUnwrap(mockStore.dispatchedActions.last as? ToolbarAction) - let completedActionType = try XCTUnwrap(completedAction.actionType as? ToolbarActionType) - XCTAssertEqual(completedActionType, ToolbarActionType.translationCompleted) + let completedAction = try XCTUnwrap(mockStore.dispatchedActions.last as? TranslationsAction) + let completedActionType = try XCTUnwrap(completedAction.actionType as? TranslationsActionType) + XCTAssertEqual(completedActionType, TranslationsActionType.translationCompleted) XCTAssertEqual(completedAction.translationConfiguration?.state, .active) XCTAssertEqual(completedAction.translationConfiguration?.sourceLanguage, "en") } From 5c1263dcd2089c45bce118d424982777b1874252 Mon Sep 17 00:00:00 2001 From: Razvan Litianu Date: Wed, 20 May 2026 15:32:09 +0300 Subject: [PATCH 2/3] Address PR review feedback: specialize method signatures and reduce duplication - Add guard for valid action types in leadingPageActions - Move translationConfiguration extraction to TranslationConfiguration.init?(from:) - Revert toolbarPosition and shouldUseAlternativeLocationColor to ToolbarAction - Restructure TranslationSettingsState reducer with switch pattern - Narrow tryAutoTranslate call chain from Action to WindowUUID --- .../Toolbars/Redux/AddressBarState.swift | 33 ++++--- .../Redux/TranslationsConfiguration.swift | 13 +++ .../TranslationSettingsState.swift | 22 ++--- .../Translations/TranslationsMiddleware.swift | 94 +++++++++---------- 4 files changed, 83 insertions(+), 79 deletions(-) diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift index 2071579c3a3a8..6d27c18bac683 100644 --- a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift +++ b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/AddressBarState.swift @@ -1110,19 +1110,28 @@ struct AddressBarState: StateType, Sendable, Equatable { ) -> [ToolbarActionConfiguration] { var actions = [ToolbarActionConfiguration]() + guard action is ToolbarAction || action is TranslationsAction else { return actions } + guard let toolbarState = store.state.componentState(ToolbarState.self, for: .toolbar, window: action.windowUUID), !isEditing else { return actions } let toolbarAction = action as? ToolbarAction - let actionTranslationConfiguration = translationConfiguration(from: action) + let actionTranslationConfiguration = TranslationConfiguration(from: action) let isShowingNavigationToolbar = toolbarAction?.isShowingNavigationToolbar ?? toolbarState.isShowingNavigationToolbar let isURLDidChangeAction = action.actionType as? ToolbarActionType == .urlDidChange let isHomepage = (isURLDidChangeAction ? toolbarAction?.url : toolbarState.addressToolbar.url) == nil let isLoadingChangeAction = action.actionType as? ToolbarActionType == .websiteLoadingStateDidChange let isLoading = isLoadingChangeAction ? toolbarAction?.isLoading : addressBarState.isLoading - let hasAlternativeLocationColor = shouldUseAlternativeLocationColor(action: action) + let hasAlternativeLocationColor: Bool + if let toolbarAction { + hasAlternativeLocationColor = shouldUseAlternativeLocationColor(action: toolbarAction) + } else { + hasAlternativeLocationColor = toolbarState.toolbarPosition == .top + && !toolbarState.isShowingTopTabs + && toolbarState.isShowingNavigationToolbar + } if !isHomepage, !isShowingNavigationToolbar { let shareAction = shareAction(enabled: isLoading == false, @@ -1155,15 +1164,6 @@ struct AddressBarState: StateType, Sendable, Equatable { return actions } - // Pulls the `translationConfiguration` payload off whichever action carries it. - // Both `ToolbarAction` (e.g. urlDidChange) and `TranslationsAction` (e.g. didStartTranslatingPage) - // can drive the leading translate icon, so the reducer reads the field generically. - private static func translationConfiguration(from action: Action) -> TranslationConfiguration? { - if let toolbarAction = action as? ToolbarAction { return toolbarAction.translationConfiguration } - if let translationsAction = action as? TranslationsAction { return translationsAction.translationConfiguration } - return nil - } - // Checks whether we should show the translation icon based on the translation configuration // state and setups up the configuration for the translation icon on the toolbar (for iPad and iPhone) private static func configureTranslationIcon( @@ -1324,12 +1324,12 @@ struct AddressBarState: StateType, Sendable, Equatable { // MARK: - Helper @MainActor - private static func toolbarPosition(action: Action) -> AddressToolbarPosition? { + private static func toolbarPosition(action: ToolbarAction) -> AddressToolbarPosition? { guard let toolbarState = store.state.componentState(ToolbarState.self, for: .toolbar, window: action.windowUUID) else { return nil } guard action.actionType as? ToolbarActionType == .toolbarPositionChanged, - let toolbarPosition = (action as? ToolbarAction)?.toolbarPosition + let toolbarPosition = action.toolbarPosition else { return toolbarState.toolbarPosition } @@ -1341,19 +1341,18 @@ struct AddressBarState: StateType, Sendable, Equatable { } @MainActor - private static func shouldUseAlternativeLocationColor(action: Action) -> Bool { + private static func shouldUseAlternativeLocationColor(action: ToolbarAction) -> Bool { guard let toolbarState = store.state.componentState(ToolbarState.self, for: .toolbar, window: action.windowUUID) else { return false } - let toolbarAction = action as? ToolbarAction let isTraitCollectionDidChangeAction = action.actionType as? ToolbarActionType == .traitCollectionDidChange let isShowingNavigationToolbar = if isTraitCollectionDidChangeAction { - toolbarAction?.isShowingNavigationToolbar ?? toolbarState.isShowingNavigationToolbar + action.isShowingNavigationToolbar ?? toolbarState.isShowingNavigationToolbar } else { toolbarState.isShowingNavigationToolbar } let isShowingTopTabs = if isTraitCollectionDidChangeAction { - toolbarAction?.isShowingTopTabs ?? toolbarState.isShowingTopTabs + action.isShowingTopTabs ?? toolbarState.isShowingTopTabs } else { toolbarState.isShowingTopTabs } diff --git a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/TranslationsConfiguration.swift b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/TranslationsConfiguration.swift index 9ea7222e9a753..93fd5009151d7 100644 --- a/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/TranslationsConfiguration.swift +++ b/firefox-ios/Client/Frontend/Browser/Toolbars/Redux/TranslationsConfiguration.swift @@ -2,6 +2,7 @@ // 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 Common +import Redux import Shared // Holds the configuration / state of the translation button on the toolbar @@ -76,6 +77,18 @@ struct TranslationConfiguration: Equatable, FeatureFlaggable { self.sourceLanguage = sourceLanguage } + init?(from action: Action) { + if let config = (action as? ToolbarAction)?.translationConfiguration { + self = config + return + } + if let config = (action as? TranslationsAction)?.translationConfiguration { + self = config + return + } + return nil + } + var isMultiLanguageFlow: Bool { guard featureFlagsProvider.isEnabled(.translationLanguagePicker) else { return false } guard let stored = prefs.stringForKey(PrefsKeys.Settings.translationPreferredLanguages), diff --git a/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsState.swift b/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsState.swift index 5df3260b7efaf..45427622da2bb 100644 --- a/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsState.swift +++ b/firefox-ios/Client/Frontend/Settings/Translation/TranslationSettingsState.swift @@ -87,23 +87,23 @@ struct TranslationSettingsState: ScreenState, Equatable { guard action.windowUUID == .unavailable || action.windowUUID == state.windowUUID else { return defaultState(from: state) } - if let action = action as? TranslationSettingsMiddlewareAction { + + switch action { + case let action as TranslationSettingsMiddlewareAction: return reduceMiddlewareAction(state: state, action: action) - } - if let action = action as? TranslationSettingsViewAction { + + case let action as TranslationSettingsViewAction: return reduceViewAction(state: state, action: action) - } - if let action = action as? TranslationsAction, - action.actionType as? TranslationsActionType == .didTranslationSettingsChange { - // The settings middleware dispatches a single `TranslationsAction` for a toggle — - // both the toolbar reducer (icon visibility) and this reducer (settings UI toggle) - // read `isTranslationsEnabled` off the same dispatch instead of fanning out a second - // middleware-only update. + + case let action as TranslationsAction + where action.actionType as? TranslationsActionType == .didTranslationSettingsChange: return state.copyWithUpdates( isTranslationsEnabled: action.isTranslationsEnabled ?? state.isTranslationsEnabled ) + + default: + return defaultState(from: state) } - return defaultState(from: state) } private static func reduceMiddlewareAction( diff --git a/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift b/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift index 41a2c3fb99857..cad50b9b65173 100644 --- a/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift +++ b/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift @@ -186,9 +186,13 @@ final class TranslationsMiddleware: FeatureFlaggable { actionType: .willTranslate, translationFlowId: newFlowId ) - self.handleUpdatingTranslationIcon(for: action, with: .loading, on: originatingTab) + self.handleUpdatingTranslationIcon( + windowUUID: action.windowUUID, + with: .loading, + on: originatingTab + ) self.retrieveTranslations( - for: action, + windowUUID: action.windowUUID, targetLanguage: deviceLanguage, isPrivate: isPrivate, on: originatingTab @@ -201,7 +205,7 @@ final class TranslationsMiddleware: FeatureFlaggable { actionType: .willRestore, translationFlowId: flowId(for: action.windowUUID) ) - self.handleUpdatingTranslationIcon(for: action, with: .inactive, on: originatingTab) + self.handleUpdatingTranslationIcon(windowUUID: action.windowUUID, with: .inactive, on: originatingTab) restoringWindows.insert(action.windowUUID) // Mark the next same-URL reload as a restore-flow reload so `webView(_:didCommit:)` // keeps the just-dispatched `.inactive` (FXIOS-15227). Manual reloads, with this @@ -282,8 +286,8 @@ final class TranslationsMiddleware: FeatureFlaggable { window: action.windowUUID )?.isPrivateMode ?? false let originatingTab = selectedTab(for: action.windowUUID) - self.handleUpdatingTranslationIcon(for: action, with: .loading, on: originatingTab) - retrieveTranslations(for: action, targetLanguage: language, isPrivate: isPrivate, on: originatingTab) + self.handleUpdatingTranslationIcon(windowUUID: action.windowUUID, with: .loading, on: originatingTab) + retrieveTranslations(windowUUID: action.windowUUID, targetLanguage: language, isPrivate: isPrivate, on: originatingTab) } private func handleLanguageSelected(for action: TranslationLanguageSelectedAction, and state: AppState) { @@ -305,15 +309,15 @@ final class TranslationsMiddleware: FeatureFlaggable { ) if isCurrentlyTranslated { pendingLanguageSwitchTargets[action.windowUUID] = action.targetLanguage - handleUpdatingTranslationIcon(for: action, with: .inactive, on: originatingTab) + handleUpdatingTranslationIcon(windowUUID: action.windowUUID, with: .inactive, on: originatingTab) store.dispatch(GeneralBrowserAction( windowUUID: action.windowUUID, actionType: GeneralBrowserActionType.reloadWebsite )) } else { - handleUpdatingTranslationIcon(for: action, with: .loading, on: originatingTab) + handleUpdatingTranslationIcon(windowUUID: action.windowUUID, with: .loading, on: originatingTab) retrieveTranslations( - for: action, + windowUUID: action.windowUUID, targetLanguage: action.targetLanguage, isPrivate: toolbarState.isPrivateMode, on: originatingTab @@ -322,15 +326,15 @@ final class TranslationsMiddleware: FeatureFlaggable { } private func handleUpdatingTranslationIcon( - for action: Action, + windowUUID: WindowUUID, with state: TranslationConfiguration.IconState, on tab: Tab? = nil ) { let config = TranslationConfiguration(prefs: profile.prefs, state: state) - persistTranslationConfig(config, on: tab ?? selectedTab(for: action.windowUUID)) + persistTranslationConfig(config, on: tab ?? selectedTab(for: windowUUID)) let translationsAction = TranslationsAction( translationConfiguration: config, - windowUUID: action.windowUUID, + windowUUID: windowUUID, actionType: TranslationsActionType.didStartTranslatingPage ) store.dispatch(translationsAction) @@ -350,20 +354,20 @@ final class TranslationsMiddleware: FeatureFlaggable { /// If auto-translate is enabled, triggers translation to the user's top preferred language. /// Returns `true` if auto-translation was initiated (caller should skip manual offer). - private func tryAutoTranslate(for action: Action, on tab: Tab?) async -> Bool { - if restoringWindows.remove(action.windowUUID) != nil { return false } + private func tryAutoTranslate(windowUUID: WindowUUID, on tab: Tab?) async -> Bool { + if restoringWindows.remove(windowUUID) != nil { return false } - if let pendingLanguage = pendingLanguageSwitchTargets.removeValue(forKey: action.windowUUID) { + if let pendingLanguage = pendingLanguageSwitchTargets.removeValue(forKey: windowUUID) { let isPrivate = store.state.componentState( ToolbarState.self, for: .toolbar, - window: action.windowUUID + window: windowUUID )?.isPrivateMode ?? false let newFlowId = UUID() - translationFlowIds[action.windowUUID] = newFlowId - selectedTargetLanguages[action.windowUUID] = pendingLanguage - handleUpdatingTranslationIcon(for: action, with: .loading, on: tab) - retrieveTranslations(for: action, targetLanguage: pendingLanguage, isPrivate: isPrivate, on: tab) + translationFlowIds[windowUUID] = newFlowId + selectedTargetLanguages[windowUUID] = pendingLanguage + handleUpdatingTranslationIcon(windowUUID: windowUUID, with: .loading, on: tab) + retrieveTranslations(windowUUID: windowUUID, targetLanguage: pendingLanguage, isPrivate: isPrivate, on: tab) return true } @@ -371,19 +375,19 @@ final class TranslationsMiddleware: FeatureFlaggable { let manager = PreferredTranslationLanguagesManager(prefs: profile.prefs) let supported = await translationsService.fetchSupportedTargetLanguages() let preferred = manager.preferredLanguages(supportedTargetLanguages: supported) - let pageLanguage = try? await translationsService.detectPageLanguage(for: action.windowUUID) + let pageLanguage = try? await translationsService.detectPageLanguage(for: windowUUID) if let pageLanguage, preferred.contains(pageLanguage) { return false } guard let targetLanguage = preferred.first else { return false } let isPrivate = store.state.componentState( ToolbarState.self, for: .toolbar, - window: action.windowUUID + window: windowUUID )?.isPrivateMode ?? false let newFlowId = UUID() - translationFlowIds[action.windowUUID] = newFlowId - selectedTargetLanguages[action.windowUUID] = targetLanguage - handleUpdatingTranslationIcon(for: action, with: .loading, on: tab) - retrieveTranslations(for: action, targetLanguage: targetLanguage, isPrivate: isPrivate, autoTranslate: true, on: tab) + translationFlowIds[windowUUID] = newFlowId + selectedTargetLanguages[windowUUID] = targetLanguage + handleUpdatingTranslationIcon(windowUUID: windowUUID, with: .loading, on: tab) + retrieveTranslations(windowUUID: windowUUID, targetLanguage: targetLanguage, isPrivate: isPrivate, autoTranslate: true, on: tab) return true } @@ -460,7 +464,7 @@ final class TranslationsMiddleware: FeatureFlaggable { } // Auto-translate handled the page load — skip the manual offer. - if await self.tryAutoTranslate(for: action, on: originatingTab) { return } + if await self.tryAutoTranslate(windowUUID: action.windowUUID, on: originatingTab) { return } // Auto-translate didn't run; offer manual translation instead. let config = TranslationConfiguration(prefs: profile.prefs, state: .inactive) @@ -495,15 +499,6 @@ final class TranslationsMiddleware: FeatureFlaggable { )) } - // Pulls `translationConfiguration` from whichever action carries it. - // The lifecycle dispatchers swapped to `TranslationsAction`, but `urlDidChange` still - // arrives as `ToolbarAction`, so the eligibility entry point reads both shapes. - private func translationConfiguration(from action: Action) -> TranslationConfiguration? { - if let toolbarAction = action as? ToolbarAction { return toolbarAction.translationConfiguration } - if let translationsAction = action as? TranslationsAction { return translationsAction.translationConfiguration } - return nil - } - // Pulls the optional `isTranslationsEnabled` payload from whichever action carries it. private func isTranslationsEnabled(from action: Action) -> Bool? { if let toolbarAction = action as? ToolbarAction { return toolbarAction.isTranslationsEnabled } @@ -512,7 +507,7 @@ final class TranslationsMiddleware: FeatureFlaggable { } private func retrieveTranslations( - for action: Action, + windowUUID: WindowUUID, targetLanguage: String, isPrivate: Bool, autoTranslate: Bool = false, @@ -520,7 +515,7 @@ final class TranslationsMiddleware: FeatureFlaggable { ) { Task { await self.performTranslation( - for: action, + windowUUID: windowUUID, targetLanguage: targetLanguage, isPrivate: isPrivate, autoTranslate: autoTranslate, @@ -530,7 +525,7 @@ final class TranslationsMiddleware: FeatureFlaggable { } private func performTranslation( - for action: Action, + windowUUID: WindowUUID, targetLanguage: String, isPrivate: Bool, autoTranslate: Bool, @@ -539,7 +534,7 @@ final class TranslationsMiddleware: FeatureFlaggable { do { var detectedSourceLanguage: String? try await translationsService.translateCurrentPage( - for: action.windowUUID, + for: windowUUID, from: nil, to: targetLanguage, onLanguageIdentified: { identifiedLanguage, deviceLanguage in @@ -550,27 +545,27 @@ final class TranslationsMiddleware: FeatureFlaggable { ) self.translationsTelemetry.translationRequested( isPrivate: isPrivate, - translationFlowId: self.flowId(for: action.windowUUID), + translationFlowId: self.flowId(for: windowUUID), fromLanguage: identifiedLanguage, toLanguage: targetLanguage, autoTranslate: autoTranslate ) } ) - try await translationsService.firstResponseReceived(for: action.windowUUID) + try await translationsService.firstResponseReceived(for: windowUUID) dispatchAction( for: TranslationsActionType.translationCompleted, with: .active, translatedToLanguage: targetLanguage, sourceLanguage: detectedSourceLanguage, - and: action.windowUUID, + and: windowUUID, on: tab ) - maybeShowAutoTranslatePrompt(windowUUID: action.windowUUID) + maybeShowAutoTranslatePrompt(windowUUID: windowUUID) } catch { let serviceError = TranslationsServiceError.fromUnknown(error) translationsTelemetry.translationFailed( - translationFlowId: flowId(for: action.windowUUID), + translationFlowId: flowId(for: windowUUID), errorType: serviceError.telemetryDescription ) logger.log( @@ -579,7 +574,7 @@ final class TranslationsMiddleware: FeatureFlaggable { category: .translations, extra: ["Translations error": "\(error.localizedDescription)"] ) - self.handleErrorFromTranslatingPage(for: action, on: tab) + self.handleErrorFromTranslatingPage(windowUUID: windowUUID, on: tab) } } @@ -594,17 +589,14 @@ final class TranslationsMiddleware: FeatureFlaggable { clearFlowId(for: action) } - // When we receive an error translating the page, we want to update the translation - // icon on the toolbar to be inactive. - // We also want to display a toast. - private func handleErrorFromTranslatingPage(for action: Action, on tab: Tab? = nil) { + private func handleErrorFromTranslatingPage(windowUUID: WindowUUID, on tab: Tab? = nil) { dispatchAction( for: TranslationsActionType.didReceiveErrorTranslating, with: .inactive, - and: action.windowUUID, + and: windowUUID, on: tab ) - dispatchShowRetryTranslationToastAction(for: action.windowUUID) + dispatchShowRetryTranslationToastAction(for: windowUUID) } private func dispatchAction( From 4cab273412a625d00311b2c247d0c4fcacf64bde Mon Sep 17 00:00:00 2001 From: Razvan Litianu Date: Wed, 20 May 2026 15:33:08 +0300 Subject: [PATCH 3/3] Fix SwiftLint line length violations --- .../Translations/TranslationsMiddleware.swift | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift b/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift index cad50b9b65173..28d9cb1ae67a8 100644 --- a/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift +++ b/firefox-ios/Client/Frontend/Translations/TranslationsMiddleware.swift @@ -287,7 +287,12 @@ final class TranslationsMiddleware: FeatureFlaggable { )?.isPrivateMode ?? false let originatingTab = selectedTab(for: action.windowUUID) self.handleUpdatingTranslationIcon(windowUUID: action.windowUUID, with: .loading, on: originatingTab) - retrieveTranslations(windowUUID: action.windowUUID, targetLanguage: language, isPrivate: isPrivate, on: originatingTab) + retrieveTranslations( + windowUUID: action.windowUUID, + targetLanguage: language, + isPrivate: isPrivate, + on: originatingTab + ) } private func handleLanguageSelected(for action: TranslationLanguageSelectedAction, and state: AppState) { @@ -387,7 +392,13 @@ final class TranslationsMiddleware: FeatureFlaggable { translationFlowIds[windowUUID] = newFlowId selectedTargetLanguages[windowUUID] = targetLanguage handleUpdatingTranslationIcon(windowUUID: windowUUID, with: .loading, on: tab) - retrieveTranslations(windowUUID: windowUUID, targetLanguage: targetLanguage, isPrivate: isPrivate, autoTranslate: true, on: tab) + retrieveTranslations( + windowUUID: windowUUID, + targetLanguage: targetLanguage, + isPrivate: isPrivate, + autoTranslate: true, + on: tab + ) return true }