Skip to content

Commit

Permalink
Bugfix FXIOS-8657 Add operation queue to perform batch updates (#19670)
Browse files Browse the repository at this point in the history
* Remove collectionView updates in newState of TabDisplayView

* Make performChainedOperations var private and add template of methods to TabDisplayView

* Add updateWith for moving the tab

* Add updateWith

* WIP

* didAddTab called in TabDisplayView

* Clean up operation queue

* Remove commented code

* Fix  crash when there's  no sections

---------

Co-authored-by: Sophie Amin <samin@KPQ47DG99L.lan>
  • Loading branch information
OrlaM and Sophie Amin authored Apr 30, 2024
1 parent 27af704 commit 86bd353
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 23 deletions.
4 changes: 4 additions & 0 deletions firefox-ios/Client.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@
5A9F83442B2B8CE900272819 /* TabPeekModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5A9F83432B2B8CE900272819 /* TabPeekModel.swift */; };
5A9FF8492942454600DF9FBB /* Common in Frameworks */ = {isa = PBXBuildFile; productRef = 5A9FF8482942454600DF9FBB /* Common */; };
5AA0CC662A4B8F6100014E2A /* PasswordManagerCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5AA0CC652A4B8F6100014E2A /* PasswordManagerCoordinator.swift */; };
5AA1D8272BC09ECB00957516 /* TabTrayAnimationQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5AA1D8262BC09ECB00957516 /* TabTrayAnimationQueue.swift */; };
5AA75A652A46274A00533F8D /* MockThemeManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5AA75A622A46272000533F8D /* MockThemeManager.swift */; };
5AB4237C28A1947A003BC40C /* MockNotificationCenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5AB4237B28A1947A003BC40C /* MockNotificationCenter.swift */; };
5AB4237E28A2BA9C003BC40C /* HistoryHighlightsDataAdaptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5AB4237D28A2BA9C003BC40C /* HistoryHighlightsDataAdaptor.swift */; };
Expand Down Expand Up @@ -5363,6 +5364,7 @@
5A9F83412B2B796800272819 /* TabPeekState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabPeekState.swift; sourceTree = "<group>"; };
5A9F83432B2B8CE900272819 /* TabPeekModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabPeekModel.swift; sourceTree = "<group>"; };
5AA0CC652A4B8F6100014E2A /* PasswordManagerCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PasswordManagerCoordinator.swift; sourceTree = "<group>"; };
5AA1D8262BC09ECB00957516 /* TabTrayAnimationQueue.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TabTrayAnimationQueue.swift; sourceTree = "<group>"; };
5AA75A622A46272000533F8D /* MockThemeManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockThemeManager.swift; sourceTree = "<group>"; };
5AB4237B28A1947A003BC40C /* MockNotificationCenter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockNotificationCenter.swift; sourceTree = "<group>"; };
5AB4237D28A2BA9C003BC40C /* HistoryHighlightsDataAdaptor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HistoryHighlightsDataAdaptor.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -8478,6 +8480,7 @@
21C5B3582AF2A7130093F366 /* Views */,
21E77E512AA8BE5C00FABA10 /* TabTrayFlagManager.swift */,
21B548942B1E5F1400DC1DF8 /* InactiveTabsManager.swift */,
5AA1D8262BC09ECB00957516 /* TabTrayAnimationQueue.swift */,
);
path = Tabs;
sourceTree = "<group>";
Expand Down Expand Up @@ -14243,6 +14246,7 @@
1D2F68AB2ACB262900524B92 /* RemoteTabsPanelAction.swift in Sources */,
8A5D1CB92A30DBDB005AD35C /* ChinaSyncServiceSetting.swift in Sources */,
1D8487B42AD0C6C100F7527C /* RemoteTabsPanelMiddleware.swift in Sources */,
5AA1D8272BC09ECB00957516 /* TabTrayAnimationQueue.swift in Sources */,
C849E46526B9C3DD00260F0B /* SlideoverPresentationController.swift in Sources */,
E18F44072A951C330056160F /* FakespotHighlightGroup.swift in Sources */,
D5D0532E2645B3A700759F85 /* ExperimentsTableView.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ struct TabDisplayOrder: Codable {

class LegacyTabDisplayManager: NSObject, FeatureFlaggable {
// MARK: - Variables
var performingChainedOperations = false
private var performingChainedOperations = false
var inactiveViewModel: LegacyInactiveTabViewModel?
var isInactiveViewExpanded = false
var dataStore = WeakList<Tab>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ struct TabsPanelState: ScreenState, Equatable {
var toastType: ToastType?
var windowUUID: WindowUUID
var scrollToIndex: Int?
var didTapAddTab: Bool
var urlRequest: URLRequest?

var isPrivateTabsEmpty: Bool {
guard isPrivateMode else { return false }
Expand All @@ -33,7 +35,9 @@ struct TabsPanelState: ScreenState, Equatable {
inactiveTabs: panelState.inactiveTabs,
isInactiveTabsExpanded: panelState.isInactiveTabsExpanded,
toastType: panelState.toastType,
scrollToIndex: panelState.scrollToIndex)
scrollToIndex: panelState.scrollToIndex,
didTapAddTab: panelState.didTapAddTab,
urlRequest: panelState.urlRequest)
}

init(windowUUID: WindowUUID, isPrivateMode: Bool = false) {
Expand All @@ -43,7 +47,9 @@ struct TabsPanelState: ScreenState, Equatable {
tabs: [TabModel](),
inactiveTabs: [InactiveTabsModel](),
isInactiveTabsExpanded: false,
toastType: nil)
toastType: nil,
didTapAddTab: false,
urlRequest: nil)
}

init(windowUUID: WindowUUID,
Expand All @@ -52,14 +58,18 @@ struct TabsPanelState: ScreenState, Equatable {
inactiveTabs: [InactiveTabsModel],
isInactiveTabsExpanded: Bool,
toastType: ToastType? = nil,
scrollToIndex: Int? = nil) {
scrollToIndex: Int? = nil,
didTapAddTab: Bool = false,
urlRequest: URLRequest? = nil) {
self.isPrivateMode = isPrivateMode
self.tabs = tabs
self.inactiveTabs = inactiveTabs
self.isInactiveTabsExpanded = isInactiveTabsExpanded
self.toastType = toastType
self.windowUUID = windowUUID
self.scrollToIndex = scrollToIndex
self.didTapAddTab = didTapAddTab
self.urlRequest = urlRequest
}

static let reducer: Reducer<Self> = { state, action in
Expand Down Expand Up @@ -119,6 +129,17 @@ struct TabsPanelState: ScreenState, Equatable {
isInactiveTabsExpanded: state.isInactiveTabsExpanded,
toastType: type)

// case TabPanelAction.didTapAddTab:
// let didTapNewTab = context.didTapAddTab
// let urlRequest = context.urlRequest
// let isPrivateMode = context.isPrivate
// return TabsPanelState(windowUUID: state.windowUUID,
// isPrivateMode: isPrivateMode,
// tabs: state.tabs,
// inactiveTabs: state.inactiveTabs,
// isInactiveTabsExpanded: state.isInactiveTabsExpanded,
// didTapAddTab: didTapNewTab)

default:
return TabsPanelState(windowUUID: state.windowUUID,
isPrivateMode: state.isPrivateMode,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/

import UIKit

protocol TabTrayAnimationQueue {
func addAnimation(for collectionView: UICollectionView,
animation: @escaping (() -> Void))
}

class TabTrayAnimationQueueImplementation: TabTrayAnimationQueue {
private var animations = [() -> Void]()
private var performingChainedOperations = false

func addAnimation(for collectionView: UICollectionView,
animation: @escaping (() -> Void)) {
animations.append(animation)
performChainedOperations(for: collectionView)
}

private func performChainedOperations(for collectionView: UICollectionView) {
guard !performingChainedOperations,
let animation = animations.first
else { return }

performingChainedOperations = true
animations.removeFirst()
/// Fix crash related to bug from `collectionView.performBatchUpdates` when the
/// collectionView is not visible the dataSource section/items differs from the actions to be perform
/// which causes the crash
if collectionView.numberOfSections != 0 {
collectionView.numberOfItems(inSection: 0)
}
collectionView.performBatchUpdates({
animation()
}, completion: { [weak self] (done) in
collectionView.reloadData()
self?.performingChainedOperations = false
self?.performChainedOperations(for: collectionView)
})
}
}
53 changes: 34 additions & 19 deletions firefox-ios/Client/Frontend/Browser/Tabs/Views/TabDisplayView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ class TabDisplayView: UIView,

let panelType: TabTrayPanelType
private(set) var tabsState: TabsPanelState
private var performingChainedOperations = false
private var inactiveTabsSectionManager: InactiveTabsSectionManager
private var tabsSectionManager: TabsSectionManager
private let windowUUID: WindowUUID
private let animationQueue: TabTrayAnimationQueue
var theme: Theme?

private var shouldHideInactiveTabs: Bool {
Expand Down Expand Up @@ -82,12 +84,14 @@ class TabDisplayView: UIView,

public init(panelType: TabTrayPanelType,
state: TabsPanelState,
windowUUID: WindowUUID) {
windowUUID: WindowUUID,
animationQueue: TabTrayAnimationQueue = TabTrayAnimationQueueImplementation()) {
self.panelType = panelType
self.tabsState = state
self.inactiveTabsSectionManager = InactiveTabsSectionManager()
self.tabsSectionManager = TabsSectionManager()
self.windowUUID = windowUUID
self.animationQueue = animationQueue
super.init(frame: .zero)
self.inactiveTabsSectionManager.delegate = self
setupLayout()
Expand All @@ -100,18 +104,22 @@ class TabDisplayView: UIView,
func newState(state: TabsPanelState) {
tabsState = state

updateCollectionViewLayout()
collectionView.reloadData()

if let index = state.scrollToIndex {
scrollToTab(index)
}
}

private func updateCollectionViewLayout() {
collectionView.reloadData()
collectionView.collectionViewLayout.invalidateLayout()
collectionView.setNeedsLayout()
collectionView.layoutIfNeeded()
if state.didTapAddTab {
animationQueue.addAnimation(for: collectionView) { [weak self] in
guard let self else { return }

let action = TabPanelViewAction(panelType: self.panelType,
windowUUID: self.windowUUID,
actionType: TabPanelViewActionType.addNewTab)
store.dispatch(action)
}
}
}

private func scrollToTab(_ index: Int) {
Expand Down Expand Up @@ -327,11 +335,14 @@ class TabDisplayView: UIView,

// MARK: - TabCellDelegate
func tabCellDidClose(for tabUUID: TabUUID) {
let action = TabPanelViewAction(panelType: panelType,
tabUUID: tabUUID,
windowUUID: windowUUID,
actionType: TabPanelViewActionType.closeTab)
store.dispatch(action)
animationQueue.addAnimation(for: collectionView) { [weak self] in
guard let self else { return }
let action = TabPanelViewAction(panelType: panelType,
tabUUID: tabUUID,
windowUUID: windowUUID,
actionType: TabPanelViewActionType.closeTab)
store.dispatch(action)
}
}

// MARK: - SwipeAnimatorDelegate
Expand Down Expand Up @@ -388,14 +399,18 @@ extension TabDisplayView: UICollectionViewDragDelegate, UICollectionViewDropDele
let moveTabData = MoveTabData(originIndex: start.row,
destinationIndex: end.row,
isPrivate: tabsState.isPrivateMode)
let action = TabPanelViewAction(panelType: panelType,
moveTabData: moveTabData,
windowUUID: windowUUID,
actionType: TabPanelViewActionType.moveTab)

store.dispatch(action)
coordinator.drop(dragItem, toItemAt: destinationIndexPath)

collectionView.moveItem(at: start, to: end)
animationQueue.addAnimation(for: collectionView) { [weak self] in
guard let self else { return }
let action = TabPanelViewAction(panelType: panelType,
moveTabData: moveTabData,
windowUUID: windowUUID,
actionType: TabPanelViewActionType.moveTab)

store.dispatch(action)
collectionView.moveItem(at: start, to: end)
}
}
}

0 comments on commit 86bd353

Please sign in to comment.