Skip to content

Refactor FXIOS-13953 [Swift 6 migration] Toolbar dispatch legacy #30418

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+71 −57
Open

Refactor FXIOS-13953 [Swift 6 migration] Toolbar dispatch legacy #30418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions BrowserKit/Sources/ToolbarKit/AddressToolbar/LocationView/LocationView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ final class LocationView: UIView,

private var urlAbsolutePath: String?
private var searchTerm: String?
private var onTapLockIcon: ((UIButton) -> Void)?
private var onLongPress: (() -> Void)?
private var onTapLockIcon: (@MainActor (UIButton) -> Void)?
private var onLongPress: (@MainActor () -> Void)?
private weak var delegate: LocationViewDelegate?
private var theme: Theme?
private var isUnifiedSearchEnabled = false
Expand Down
8 changes: 4 additions & 4 deletions BrowserKit/Sources/ToolbarKit/AddressToolbar/LocationView/LocationViewState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ public struct LocationViewConfiguration {
public let didStartTyping: Bool
public let shouldShowKeyboard: Bool
public let shouldSelectSearchTerm: Bool
public var onTapLockIcon: ((UIButton) -> Void)?
public var onLongPress: (() -> Void)?
public var onTapLockIcon: (@MainActor (UIButton) -> Void)?
public var onLongPress: (@MainActor () -> Void)?

public init(
searchEngineImageViewA11yId: String,
Expand All @@ -46,8 +46,8 @@ public struct LocationViewConfiguration {
didStartTyping: Bool,
shouldShowKeyboard: Bool,
shouldSelectSearchTerm: Bool,
onTapLockIcon: ((UIButton) -> Void)? = nil,
onLongPress: (() -> Void)? = nil
onTapLockIcon: (@MainActor (UIButton) -> Void)? = nil,
onLongPress: (@MainActor () -> Void)? = nil
) {
self.searchEngineImageViewA11yId = searchEngineImageViewA11yId
self.searchEngineImageViewA11yLabel = searchEngineImageViewA11yLabel
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4711,7 +4711,7 @@ extension BrowserViewController: KeyboardHelperDelegate {
)
}

store.dispatchLegacy(
store.dispatch(
ToolbarAction(
shouldShowKeyboard: false,
windowUUID: windowUUID,
Expand Down
2 changes: 1 addition & 1 deletion firefox-ios/Client/Frontend/Browser/TabScrollController/TabScrollHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ final class TabScrollHandler: NSObject,
if (lastContentOffsetY > 0 && contentOffset.y <= 0) ||
(lastContentOffsetY <= 0 && contentOffset.y > 0) {
lastContentOffsetY = contentOffset.y
store.dispatchLegacy(
store.dispatch(
GeneralBrowserMiddlewareAction(
scrollOffset: contentOffset,
windowUUID: windowUUID,
Expand Down
6 changes: 3 additions & 3 deletions firefox-ios/Client/Frontend/Browser/Toolbars/AddressBarPanGestureHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ final class AddressBarPanGestureHandler: NSObject, StoreSubscriber {
enablePanGestureRecognizer()
if shouldCompleteTransition {
webPagePreview.isHidden = true
store.dispatchLegacy(
store.dispatch(
ToolbarAction(
shouldAnimate: false,
windowUUID: windowUUID,
Expand All @@ -286,8 +286,8 @@ final class AddressBarPanGestureHandler: NSObject, StoreSubscriber {
if let nextTab {
tabManager.selectTab(nextTab)
} else {
store.dispatchLegacy(GeneralBrowserAction(windowUUID: windowUUID,
actionType: GeneralBrowserActionType.addNewTab))
store.dispatch(GeneralBrowserAction(windowUUID: windowUUID,
actionType: GeneralBrowserActionType.addNewTab))
}
} else {
statusBarOverlay.restoreOverlay(animated: !UIAccessibility.isReduceMotionEnabled,
Expand Down
25 changes: 13 additions & 12 deletions firefox-ios/Client/Frontend/Browser/Toolbars/AddressToolbarContainer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ final class AddressToolbarContainer: UIView,
/// We want to check here if the keyboard accessory view state has changed
/// To avoid spamming redux actions.
guard hasAccessoryView != shoudShowKeyboard else { return accessoryViewOffset }
store.dispatchLegacy(
store.dispatch(
ToolbarAction(
shouldShowKeyboard: hasAccessoryView,
windowUUID: windowUUID,
Expand All @@ -215,7 +215,7 @@ final class AddressToolbarContainer: UIView,
)

if shouldAdjustForAccessory {
store.dispatchLegacy(
store.dispatch(
ToolbarAction(
scrollAlpha: 0,
windowUUID: windowUUID,
Expand Down Expand Up @@ -264,7 +264,7 @@ final class AddressToolbarContainer: UIView,
let action = ScreenAction(windowUUID: windowUUID,
actionType: ScreenActionType.showScreen,
screen: .toolbar)
store.dispatchLegacy(action)
store.dispatch(action)

store.subscribe(self, transform: {
$0.select({ appState in
Expand All @@ -273,6 +273,7 @@ final class AddressToolbarContainer: UIView,
})
}

// Laurie - This is never called?
Copy link
Contributor Author

@lmarceau lmarceau Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same @thatswinnie should we call this in deinit?

Copy link
Collaborator

@thatswinnie thatswinnie Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lmarceau yes, definitely.

func unsubscribeFromRedux() {
guard let windowUUID else {
store.unsubscribe(self)
Expand All @@ -282,7 +283,7 @@ final class AddressToolbarContainer: UIView,
let action = ScreenAction(windowUUID: windowUUID,
actionType: ScreenActionType.closeScreen,
screen: .toolbar)
store.dispatchLegacy(action)
store.dispatch(action)
store.unsubscribe(self)
}

Expand Down Expand Up @@ -501,13 +502,13 @@ final class AddressToolbarContainer: UIView,
let toolbarState = store.state.screenState(ToolbarState.self, for: .toolbar, window: windowUUID) {
if searchTerm.isEmpty, !toolbarState.addressToolbar.isEmptySearch {
let action = ToolbarAction(windowUUID: windowUUID, actionType: ToolbarActionType.didDeleteSearchTerm)
store.dispatchLegacy(action)
store.dispatch(action)
} else if !searchTerm.isEmpty, toolbarState.addressToolbar.isEmptySearch {
let action = ToolbarAction(windowUUID: windowUUID, actionType: ToolbarActionType.didEnterSearchTerm)
store.dispatchLegacy(action)
store.dispatch(action)
} else if !toolbarState.addressToolbar.didStartTyping {
let action = ToolbarAction(windowUUID: windowUUID, actionType: ToolbarActionType.didStartTyping)
store.dispatchLegacy(action)
store.dispatch(action)
}
}
self.searchTerm = searchTerm
Expand All @@ -522,7 +523,7 @@ final class AddressToolbarContainer: UIView,

let action = ToolbarMiddlewareAction(windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didClearSearch)
store.dispatchLegacy(action)
store.dispatch(action)
}

func openBrowser(searchTerm: String) {
Expand Down Expand Up @@ -573,7 +574,7 @@ final class AddressToolbarContainer: UIView,

let action = ToolbarMiddlewareAction(windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didStartDragInteraction)
store.dispatchLegacy(action)
store.dispatch(action)
}

func addressToolbarDidBeginDragInteraction() {
Expand Down Expand Up @@ -601,15 +602,15 @@ final class AddressToolbarContainer: UIView,
windowUUID: windowUUID,
actionType: ToolbarActionType.didPasteSearchTerm
)
store.dispatchLegacy(action)
store.dispatch(action)

delegate?.openSuggestions(searchTerm: locationText ?? "")
} else {
let action = ToolbarAction(searchTerm: locationText,
shouldAnimate: true,
windowUUID: windowUUID,
actionType: ToolbarActionType.didStartEditingUrl)
store.dispatchLegacy(action)
store.dispatch(action)
}
}

Expand All @@ -624,7 +625,7 @@ final class AddressToolbarContainer: UIView,

if toolbarState.addressToolbar.isEditing {
let action = ToolbarAction(windowUUID: windowUUID, actionType: ToolbarActionType.cancelEdit)
store.dispatchLegacy(action)
store.dispatch(action)
}
}

Expand Down
42 changes: 24 additions & 18 deletions firefox-ios/Client/Frontend/Browser/Toolbars/Models/AddressToolbarContainerModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,14 @@ final class AddressToolbarContainerModel: Equatable {
gestureType: .tap,
windowUUID: self.windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
store.dispatch(action)
},
onLongPress: {
let action = ToolbarMiddlewareAction(buttonType: .locationView,
gestureType: .longPress,
windowUUID: self.windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
store.dispatch(action)
})
return AddressToolbarConfiguration(
locationViewConfiguration: locationViewConfiguration,
Expand Down Expand Up @@ -285,34 +285,40 @@ final class AddressToolbarContainerModel: Equatable {

private static func getA11yCustomAction(action: ToolbarActionConfiguration, windowUUID: UUID) -> (() -> Void)? {
return action.a11yCustomActionName != nil ? {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.customA11yAction)
store.dispatchLegacy(action)
ensureMainThread {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.customA11yAction)
store.dispatch(action)
}
} : nil
}

private static func getOnSelected(action: ToolbarActionConfiguration, windowUUID: UUID) -> ((UIButton) -> Void)? {
return { button in
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .tap,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
ensureMainThread {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .tap,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatch(action)
}
}
}

private static func getOnLongPress(action: ToolbarActionConfiguration,
windowUUID: UUID,
isShowingTopTabs: Bool) -> ((UIButton) -> Void)? {
return action.canPerformLongPressAction(isShowingTopTabs: isShowingTopTabs) ? { button in
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .longPress,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
ensureMainThread {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .longPress,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatch(action)
}
} : nil
}

Expand Down
38 changes: 22 additions & 16 deletions firefox-ios/Client/Frontend/Browser/Toolbars/Models/NavigationToolbarContainerModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,34 +53,40 @@ struct NavigationToolbarContainerModel: Equatable {

private static func getA11yCustomAction(action: ToolbarActionConfiguration, windowUUID: WindowUUID) -> (() -> Void)? {
return action.a11yCustomActionName != nil ? {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.customA11yAction)
store.dispatchLegacy(action)
ensureMainThread {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.customA11yAction)
store.dispatch(action)
}
} : nil
}

private static func getOnSelected(action: ToolbarActionConfiguration, windowUUID: WindowUUID) -> ((UIButton) -> Void)? {
return { button in
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .tap,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
ensureMainThread {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .tap,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatch(action)
}
}
}

private static func getOnLongPress(action: ToolbarActionConfiguration,
state: ToolbarState,
windowUUID: WindowUUID) -> ((UIButton) -> Void)? {
return action.canPerformLongPressAction(isShowingTopTabs: state.isShowingTopTabs) ? { button in
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .longPress,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatchLegacy(action)
ensureMainThread {
let action = ToolbarMiddlewareAction(buttonType: action.actionType,
buttonTapped: button,
gestureType: .longPress,
windowUUID: windowUUID,
actionType: ToolbarMiddlewareActionType.didTapButton)
store.dispatch(action)
}
} : nil
}
}
1 change: 1 addition & 0 deletions firefox-ios/Client/Frontend/Browser/Toolbars/NavigationToolbarContainer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ final class NavigationToolbarContainer: UIView, ThemeApplicable, StoreSubscriber
})
}

// Laurie - This is never called?
Copy link
Contributor Author

@lmarceau lmarceau Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thatswinnie Should we call this in deinit?

Copy link
Collaborator

@thatswinnie thatswinnie Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lmarceau yes, we should 🙏

func unsubscribeFromRedux() {
store.unsubscribe(self)
}
Expand Down