Skip to content

Commit

Permalink
SwiftUI timeline refactor + TimelineViewState decoupling (#1392)
Browse files Browse the repository at this point in the history 
* swiftUI conversion + scroll to bottom

* remove unused

* back pagination implemented, however when a lot of elements are in the scroll the perfomance diminishes not sure what may be causing it

* scroll adapter solution

* way better

* works but height animation in cells are broken

* code improvement

* everything implemented

* rebase fix

* fix

* doc

* fix test compilation

* code improvement and animation improvement

* code improvement

* code improvements

* tests updated.

* pr comments

* better identifiers

* fix

* some PR comments

* pr comments
  • Loading branch information
@Velin92
Velin92 authored Jul 25, 2023
1 parent 43dafda commit 7cf32ba
Show file tree
Hide file tree
Showing 17 changed files with 280 additions and 476 deletions.
14 changes: 5 additions & 9 deletions ElementX.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@
206F0DBAB6AF042CA1FF2C0D /* SettingsViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D487C1185D658F8B15B8F55 /* SettingsViewModelTests.swift */; };
208C19811613F9A10F8A7B75 /* MediaLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AFCE895ECFFA53FEE64D62B /* MediaLoader.swift */; };
2185C1F6724C78FFF355D6FA /* WelcomeScreenScreenUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8AB10FA6570DD08B3966C159 /* WelcomeScreenScreenUITests.swift */; };
2198B4458AFF69102BBCC370 /* RoomTimelineItemViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = FA7D3C686C214470F4911618 /* RoomTimelineItemViewModel.swift */; };
21BF2B7CEDFE3CA67C5355AD /* test_image.png in Resources */ = {isa = PBXBuildFile; fileRef = C733D11B421CFE3A657EF230 /* test_image.png */; };
22882C710BC99EC34A5024A0 /* UITestsScreenIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6CEBE5EA91E8691EDF364EC2 /* UITestsScreenIdentifier.swift */; };
2352C541AF857241489756FF /* MockRoomSummaryProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8F7D42E66E939B709C1EC390 /* MockRoomSummaryProvider.swift */; };
Expand Down Expand Up @@ -172,6 +171,7 @@
40B79D20A873620F7F128A2C /* UserPreference.swift in Sources */ = {isa = PBXBuildFile; fileRef = 35FA991289149D31F4286747 /* UserPreference.swift */; };
414F50CFCFEEE2611127DCFB /* RestorationToken.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3558A15CFB934F9229301527 /* RestorationToken.swift */; };
4166A7DD2A4E2EFF0EB9369B /* FormRowLabelStyle.swift in Sources */ = {isa = PBXBuildFile; fileRef = D1897720266C036471AD9D1B /* FormRowLabelStyle.swift */; };
41CE5E1289C8768FC5B6490C /* RoomTimelineItemViewState.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0C2D52E36AD614B3C003EF6 /* RoomTimelineItemViewState.swift */; };
41DFDD212D1BE57CA50D783B /* Kingfisher in Frameworks */ = {isa = PBXBuildFile; productRef = 0DD568A494247444A4B56031 /* Kingfisher */; };
41F553349AF44567184822D8 /* APNSPayload.swift in Sources */ = {isa = PBXBuildFile; fileRef = 94D670124FC3E84F23A62CCF /* APNSPayload.swift */; };
4219391CD2351E410554B3E8 /* AggregratedReaction.swift in Sources */ = {isa = PBXBuildFile; fileRef = B858A61F2A570DFB8DE570A7 /* AggregratedReaction.swift */; };
Expand Down Expand Up @@ -296,7 +296,6 @@
6F2D5D4F2590310DFAE973E4 /* WaitingDialog.swift in Sources */ = {isa = PBXBuildFile; fileRef = F6D698BFD68B061350553930 /* WaitingDialog.swift */; };
6FC10A00D268FCD48B631E37 /* ViewFrameReader.swift in Sources */ = {isa = PBXBuildFile; fileRef = EFF7BF82A950B91BC5469E91 /* ViewFrameReader.swift */; };
6FF51EB400DBA0668FC38B97 /* TimelineStartRoomTimelineView.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9ED8E731E21055F728E5FED /* TimelineStartRoomTimelineView.swift */; };
702694459B649B9D3A3C34F8 /* TimelineTableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9212AE02CBDD692C56A879F /* TimelineTableViewController.swift */; };
70394ECD2DCC70741538620D /* AccessibilityIdentifiers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 04BB8DDE245ED86C489BA983 /* AccessibilityIdentifiers.swift */; };
70558528EF68CAAEF09972D5 /* RoomTimelineItemFixtures.swift in Sources */ = {isa = PBXBuildFile; fileRef = E96ED747FF90332EA1333C22 /* RoomTimelineItemFixtures.swift */; };
706289B086B0A6B0C211763F /* UITestsSignalling.swift in Sources */ = {isa = PBXBuildFile; fileRef = B7F0192CE2F891141A25B49F /* UITestsSignalling.swift */; };
Expand Down Expand Up @@ -1372,6 +1371,7 @@
D071F86CD47582B9196C9D16 /* UserDiscoverySection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserDiscoverySection.swift; sourceTree = "<group>"; };
D09A267106B9585D3D0CFC0D /* ClientError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClientError.swift; sourceTree = "<group>"; };
D0A45283CF1DB96E583BECA6 /* ImageRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageRoomTimelineView.swift; sourceTree = "<group>"; };
D0C2D52E36AD614B3C003EF6 /* RoomTimelineItemViewState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineItemViewState.swift; sourceTree = "<group>"; };
D1897720266C036471AD9D1B /* FormRowLabelStyle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FormRowLabelStyle.swift; sourceTree = "<group>"; };
D263254AFE5B7993FFBBF324 /* NSE.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = NSE.entitlements; sourceTree = "<group>"; };
D316BB02636AF2174F2580E6 /* SoftLogoutScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SoftLogoutScreenViewModelProtocol.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1472,10 +1472,8 @@
F875D71347DC81EAE7687446 /* NavigationRootCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NavigationRootCoordinatorTests.swift; sourceTree = "<group>"; };
F899D02CF26EA7675EEBE74C /* UserSessionScreenTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserSessionScreenTests.swift; sourceTree = "<group>"; };
F8CEB4634C0DD7779C4AB504 /* CreateRoomScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateRoomScreenUITests.swift; sourceTree = "<group>"; };
F9212AE02CBDD692C56A879F /* TimelineTableViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineTableViewController.swift; sourceTree = "<group>"; };
F9E785D5137510481733A3E8 /* TextRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TextRoomTimelineView.swift; sourceTree = "<group>"; };
F9ED8E731E21055F728E5FED /* TimelineStartRoomTimelineView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineStartRoomTimelineView.swift; sourceTree = "<group>"; };
FA7D3C686C214470F4911618 /* RoomTimelineItemViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomTimelineItemViewModel.swift; sourceTree = "<group>"; };
FB0D6CB491777E7FC6B5BA12 /* CreateRoomScreen.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateRoomScreen.swift; sourceTree = "<group>"; };
FB7BAD55A4E2B8E5828CD64C /* SoftLogoutScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SoftLogoutScreenViewModel.swift; sourceTree = "<group>"; };
FBC776F301D374A3298C69DA /* AppCoordinatorProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppCoordinatorProtocol.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2590,7 +2588,6 @@
4A4AD793D50748F8997E5B15 /* TimelineItemMacContextMenu.swift */,
A1BF12A5E7C76777C4BF0F2B /* TimelineItemMenu.swift */,
0BC588051E6572A1AF51D738 /* TimelineSenderAvatarView.swift */,
F9212AE02CBDD692C56A879F /* TimelineTableViewController.swift */,
874A1842477895F199567BD7 /* TimelineView.swift */,
1D8572B713A11CFDBF009B2F /* Replies */,
A312471EA62EFB0FD94E60DC /* Style */,
Expand Down Expand Up @@ -2892,7 +2889,7 @@
7D25A35764C7B3DB78954AB5 /* RoomTimelineItemFactoryProtocol.swift */,
ED1D792EB82506A19A72C8DE /* RoomTimelineItemProtocol.swift */,
90F2F8998E5632668B0AD848 /* RoomTimelineItemView.swift */,
FA7D3C686C214470F4911618 /* RoomTimelineItemViewModel.swift */,
D0C2D52E36AD614B3C003EF6 /* RoomTimelineItemViewState.swift */,
75D1D02F7F3AC1122FCFB4F3 /* Items */,
);
path = TimelineItems;
Expand Down Expand Up @@ -3627,7 +3624,7 @@
path = Timeline;
sourceTree = "<group>";
};
"TEMP_62CE5EF1-8768-4933-9547-E5F60DAC11F4" /* element-x-ios */ = {
"TEMP_43147A32-A06C-4EEF-8C45-FDEA43A25C1E" /* element-x-ios */ = {
isa = PBXGroup;
children = (
41553551C55AD59885840F0E /* secrets.xcconfig */,
Expand Down Expand Up @@ -4532,7 +4529,7 @@
878070573C7BF19E735707B4 /* RoomTimelineItemProperties.swift in Sources */,
1AE4AEA0FA8DEF52671832E0 /* RoomTimelineItemProtocol.swift in Sources */,
AD55E245FE686D7DB4C86406 /* RoomTimelineItemView.swift in Sources */,
2198B4458AFF69102BBCC370 /* RoomTimelineItemViewModel.swift in Sources */,
41CE5E1289C8768FC5B6490C /* RoomTimelineItemViewState.swift in Sources */,
9BD3A773186291560DF92B62 /* RoomTimelineProvider.swift in Sources */,
77D7DAA41AAB36800C1F2E2D /* RoomTimelineProviderProtocol.swift in Sources */,
B2F8E01ABA1BA30265B4ECBE /* RoundedCornerShape.swift in Sources */,
Expand Down Expand Up @@ -4631,7 +4628,6 @@
6FF51EB400DBA0668FC38B97 /* TimelineStartRoomTimelineView.swift in Sources */,
69BCBB4FB2DC3D61A28D3FD8 /* TimelineStyle.swift in Sources */,
FFD3E4FF948E06C7585317FC /* TimelineStyler.swift in Sources */,
702694459B649B9D3A3C34F8 /* TimelineTableViewController.swift in Sources */,
500CB65ED116B81DA52FDAEE /* TimelineView.swift in Sources */,
36AC963F2F04069B7FF1AA0C /* UIConstants.swift in Sources */,
A37EED79941AD3B7140B3822 /* UIDevice.swift in Sources */,
Expand Down
4 changes: 2 additions & 2 deletions ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/pointfreeco/xctest-dynamic-overlay",
"state" : {
"revision" : "245d527002f29c5a616c5b7131f6a50ac7f41cbf",
"version" : "0.8.6"
"revision" : "50843cbb8551db836adec2290bb4bc6bac5c1865",
"version" : "0.9.0"
}
}
],
Expand Down
12 changes: 11 additions & 1 deletion ElementX/Sources/Other/ScrollViewAdapter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ class ScrollViewAdapter: NSObject, UIScrollViewDelegate {
scrollView?.delegate = self
}
}


var shouldScrollToTopClosure: ((UIScrollView) -> Bool)?

private let didScrollSubject = PassthroughSubject<Void, Never>()
var didScroll: AnyPublisher<Void, Never> {
didScrollSubject.eraseToAnyPublisher()
Expand Down Expand Up @@ -73,6 +75,14 @@ class ScrollViewAdapter: NSObject, UIScrollViewDelegate {
func scrollViewDidScrollToTop(_ scrollView: UIScrollView) {
updateDidScroll(scrollView)
}

func scrollViewShouldScrollToTop(_ scrollView: UIScrollView) -> Bool {
guard let shouldScrollToTopClosure else {
// Default behaviour
return true
}
return shouldScrollToTopClosure(scrollView)
}

// MARK: - Private

Expand Down
35 changes: 20 additions & 15 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ enum RoomScreenComposerMode: Equatable {

enum RoomScreenViewAction {
case displayRoomDetails
case paginateBackwards
case itemAppeared(itemID: TimelineItemIdentifier)
case itemDisappeared(itemID: TimelineItemIdentifier)
case itemTapped(itemID: TimelineItemIdentifier)
Expand Down Expand Up @@ -86,18 +85,14 @@ struct RoomScreenViewState: BindableState {
var roomId: String
var roomTitle = ""
var roomAvatarURL: URL?
var itemsDictionary = OrderedDictionary<String, RoomTimelineItemViewModel>()
var members: [String: RoomMemberState] = [:]
var canBackPaginate = true
var isBackPaginating = false
var showLoading = false
var timelineStyle: TimelineStyle
var readReceiptsEnabled: Bool
var isEncryptedOneToOneRoom = false

var timelineViewState = TimelineViewState() // check the doc before changing this
var composerMode: RoomScreenComposerMode = .default
let scrollToBottomPublisher = PassthroughSubject<Void, Never>()


var bindings: RoomScreenViewStateBindings

/// A closure providing the actions to show when long pressing on an item in the timeline.
Expand All @@ -106,14 +101,6 @@ struct RoomScreenViewState: BindableState {
var sendButtonDisabled: Bool {
bindings.composerText.count == 0
}

var timelineIDs: [String] {
itemsDictionary.keys.elements
}

var itemViewModels: [RoomTimelineItemViewModel] {
itemsDictionary.values.elements
}
}

struct RoomScreenViewStateBindings {
Expand Down Expand Up @@ -184,3 +171,21 @@ struct RoomMemberState {
let displayName: String?
let avatarURL: URL?
}

/// Used as the state for the TimelineView, to avoid having the context continuously refresh the list of items on each small change.
/// Is also nice to have this as a wrapper for any state that is directly connected to the timeline.
struct TimelineViewState {
var canBackPaginate = true
var isBackPaginating = false
var itemsDictionary = OrderedDictionary<String, RoomTimelineItemViewState>()

var timelineIDs: [String] {
itemsDictionary.keys.elements
}

var itemViewStates: [RoomTimelineItemViewState] {
itemsDictionary.values.elements
}

var paginateAction: (() -> Void)?
}
63 changes: 34 additions & 29 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol

private var canCurrentUserRedact = false

private var paginateBackwardsTask: Task<Void, Never>?

init(timelineController: RoomTimelineControllerProtocol,
mediaProvider: MediaProviderProtocol,
roomProxy: RoomProxyProtocol,
Expand All @@ -60,6 +62,10 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
bindings: .init(composerText: "", composerFocused: false, reactionsCollapsed: [:])),
imageProvider: mediaProvider)

state.timelineViewState.paginateAction = { [weak self] in
self?.paginateBackwards()
}

setupSubscriptions()

state.timelineItemMenuActionProvider = { [weak self] itemId -> TimelineItemMenuActions? in
Expand All @@ -84,8 +90,6 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
switch viewAction {
case .displayRoomDetails:
callback?(.displayRoomDetails)
case .paginateBackwards:
Task { await paginateBackwards() }
case .itemAppeared(let id):
Task { await timelineController.processItemAppearance(id) }
case .itemDisappeared(let id):
Expand Down Expand Up @@ -151,12 +155,12 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
case .updatedTimelineItems:
self.buildTimelineViews()
case .canBackPaginate(let canBackPaginate):
if self.state.canBackPaginate != canBackPaginate {
self.state.canBackPaginate = canBackPaginate
if self.state.timelineViewState.canBackPaginate != canBackPaginate {
self.state.timelineViewState.canBackPaginate = canBackPaginate
}
case .isBackPaginating(let isBackPaginating):
if self.state.isBackPaginating != isBackPaginating {
self.state.isBackPaginating = isBackPaginating
if self.state.timelineViewState.isBackPaginating != isBackPaginating {
self.state.timelineViewState.isBackPaginating = isBackPaginating
}
}
}
Expand Down Expand Up @@ -215,12 +219,23 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
.store(in: &cancellables)
}

private func paginateBackwards() async {
switch await timelineController.paginateBackwards(requestSize: Constants.backPaginationEventLimit, untilNumberOfItems: Constants.backPaginationPageSize) {
case .failure:
displayError(.toast(L10n.errorFailedLoadingMessages))
default:
break
private func paginateBackwards() {
guard paginateBackwardsTask == nil else {
return
}

paginateBackwardsTask = Task { [weak self] in
guard let self else {
return
}

switch await timelineController.paginateBackwards(requestSize: Constants.backPaginationEventLimit, untilNumberOfItems: Constants.backPaginationPageSize) {
case .failure:
displayError(.toast(L10n.errorFailedLoadingMessages))
default:
break
}
paginateBackwardsTask = nil
}
}

Expand All @@ -245,7 +260,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
}

private func buildTimelineViews() {
var timelineItemsDictionary = OrderedDictionary<String, RoomTimelineItemViewModel>()
var timelineItemsDictionary = OrderedDictionary<String, RoomTimelineItemViewState>()

let itemsGroupedByTimelineDisplayStyle = timelineController.timelineItems.chunked { current, next in
canGroupItem(timelineItem: current, with: next)
Expand All @@ -259,36 +274,26 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol

if itemGroup.count == 1 {
if let firstItem = itemGroup.first {
timelineItemsDictionary.updateValue(updateViewModel(item: firstItem, groupStyle: .single),
timelineItemsDictionary.updateValue(RoomTimelineItemViewState(item: firstItem, groupStyle: .single),
forKey: firstItem.id.timelineID)
}
} else {
for (index, item) in itemGroup.enumerated() {
if index == 0 {
timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .first),
timelineItemsDictionary.updateValue(RoomTimelineItemViewState(item: item, groupStyle: .first),
forKey: item.id.timelineID)
} else if index == itemGroup.count - 1 {
timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .last),
timelineItemsDictionary.updateValue(RoomTimelineItemViewState(item: item, groupStyle: .last),
forKey: item.id.timelineID)
} else {
timelineItemsDictionary.updateValue(updateViewModel(item: item, groupStyle: .middle),
timelineItemsDictionary.updateValue(RoomTimelineItemViewState(item: item, groupStyle: .middle),
forKey: item.id.timelineID)
}
}
}
}

state.itemsDictionary = timelineItemsDictionary
}

private func updateViewModel(item: RoomTimelineItemProtocol, groupStyle: TimelineGroupStyle) -> RoomTimelineItemViewModel {
if let timelineItemViewModel = state.itemsDictionary[item.id.timelineID] {
timelineItemViewModel.groupStyle = groupStyle
timelineItemViewModel.type = .init(item: item)
return timelineItemViewModel
} else {
return RoomTimelineItemViewModel(item: item, groupStyle: groupStyle)
}
state.timelineViewState.itemsDictionary = timelineItemsDictionary
}

private func canGroupItem(timelineItem: RoomTimelineItemProtocol, with otherTimelineItem: RoomTimelineItemProtocol) -> Bool {
Expand Down Expand Up @@ -666,7 +671,7 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
// MARK: - Reactions

private func showEmojiPicker(for itemID: TimelineItemIdentifier) {
guard let item = state.itemsDictionary[itemID.timelineID], item.isReactable else { return }
guard let item = state.timelineViewState.itemsDictionary[itemID.timelineID], item.isReactable else { return }
callback?(.displayEmojiPicker(itemID: itemID))
}

Expand Down
Loading

0 comments on commit 7cf32ba

Please sign in to comment.