feat: improve swipe pt.1 - WPB-16787 #3019

dmitrysimkin · 2025-05-15T07:27:28Z

WPB-16787 Improve Swipe gesture

Issue

First part of the planned changes. Please take a look at the bigger picture and approach, this is the goal of the PR.
PR is targeting into feature branch. You can find this first PR a bit raw, it has some TODO's and dev code but it's too many things affected, we need to start with smth. I'll address all this dev code and TODO in next PRs

In this PR

Migrated sender name to SwiftUI:

show if needed
update dynamically if was renamed on other device without refreshing the cell, as self sufficient message view.
Also there known issue when sender name updated so line number is different, it does not update UI properly (will address later)

Migrated status view to SwiftUI:

show if needed
updates dynamically without refreshing a cell through table view

Introduced UI layer model to serve the UI needs only. Also to avoid issues with multiple threads
Removed ConversationCell to be generic, it's not needed as we set cell's content using contetConfigration for which you don't need to reuse cells. Another reason as that you can't create viewModel empty on the fly, we need ViewModels be created before and passed to a cell.
Introduced ViewModels factory to encapsulate initialisation to one place with single responsibility. Similarly to approach in WireAuthenication but without 3rd party DI which can be done as next transition phase
Move some UI related code like date formatters and fonts to WireUI to be able to reuse in main App and WireConversationUI module

What's not in this PR:

Sender's avatar
Count down timer for self deleting messages
message text content
some little UI things like images and some formatting logic

Testing

Describe how to test.

Optional: attachments like images, videos, etc.

Checklist

Title contains a reference JIRA issue number like [WPB-XXX].
Description is filled and free of optional paragraphs.
Adds/updates automated tests.

UI accessibility checklist

If your PR includes UI changes, please utilize this checklist:

Make sure you use the API for UI elements that support large fonts.
All colors are taken from WireDesign.ColorTheme or constructed using WireDesign.BaseColorPalette.
New UI elements have Accessibility strings for VoiceOver.

# Conflicts: # wire-ios/Wire-iOS/Sources/UserInterface/Conversation/Content/Cells/ConfigurationMessageCell/ConversationMessageSectionController.swift

dmitrysimkin · 2025-05-19T08:39:45Z

WireUI/Sources/WireConversationUI/ConversationCell/CellTypes/TextMessage/MessageModel.swift

+
+import Foundation
+
+public struct MessageModel: Equatable {


Created UI model to have it's own model and not depend on other layers Models.
Also it was requirement becasue this is WireConversationUI module where I can't import WireDataModel project.
As well it helps to have just pure struct which helps be safer with background thread calculations

question: I wonder if we want to move these models to the WireDomain package and have, as such, kind of a centralized place where we retrieve these models from wherever we need them. just concerned about duplicating WireDataModel objects spread a little bit everywhere in new code.

update: these look like more like UIModels so probably best to keep it here.

These are presentation layer models not domain, for me WireDomain is not a places for them but if it will be used in other modules and not in WireConversationUI only we'd need to find proper place to put

dmitrysimkin · 2025-05-19T08:40:52Z

WireUI/Sources/WireConversationUI/ConversationCell/CellTypes/TextMessage/MessageModel.swift

+public enum SystemMessageTypeModel: Int, Equatable {
+    case performedCall
+    case missedCall
+    case messageDeletedForEveryone
+}
+
+public enum DeliveryStateModel: Int, Sendable, Equatable, CaseIterable {
+    case invalid
+    case pending
+    case sent
+    case delivered
+    case read
+    case failedToSend
+}
+


I'll split them into its own files later

dmitrysimkin · 2025-05-19T08:54:43Z

...Sources/WireConversationUI/ConversationCell/CellTypes/TextMessage/TextMessageViewModel.swift

+//    private func startRandomStateTimer() {
+//        timer = Timer.publish(every: 1.0, on: .main, in: .common)
+//            .autoconnect()
+//            .sink { [weak self] _ in
+//                guard let self else { return }
+//                let (newText, lines) = self.randomMultilineText()
+//                if lines >= 2 {
+//                    self.significantChangeSubject.send(())
+//                } else {
+//                    self.text = newText
+//                }
+//            }
+//    }
+
+//    func randomMultilineText() -> (String, Int) {
+//        let lines = [


Removed in next PR

dmitrysimkin · 2025-05-19T08:58:57Z

WireUI/Sources/WireConversationUI/ConversationCell/ConversationCellPreview.swift

-final class ConversationCellsPreview: UITableViewController {
-
-    enum SectionIdentifier {
-        case single
-    }
-
-    typealias ItemIdentifier = ConversationCellModel


I've commented this out as it was not straight forward to make all models support identifiable for diffable data source. Will try to back that in on final stages

dmitrysimkin · 2025-05-19T09:00:03Z

WireUI/Sources/WireConversationUI/Resources/UImage+ImageResource.swift

There was typo in Resouces instead of Resources

dmitrysimkin · 2025-05-19T09:02:46Z

...ation/Content/Cells/ConfigurationMessageCell/Components/ConversationMessageToolboxCell.swift

-        var message: ZMConversationMessage
-        var deliveryState: ZMDeliveryState
+        var message: MessageModel
+        var deliveryState: DeliveryStateModel


Use UI model which is pure structs so we don't have issues with multithreaded access

thought: I don't think we have a convention on that yet however to be more specific about the intent of these models we could rename them with the suffix UIModel

agree, I'll ask in the channel about naming and will rename in next PRs accordingly

Will update in next PRs to avoid back and force merging

dmitrysimkin · 2025-05-19T09:05:55Z

.../Conversation/Content/Cells/ConfigurationMessageCell/Components/NewTextCellDescription.swift

+extension NewTextCellDescription: NewCellDescription {}
+extension BurstTimestampSenderMessageCellDescription: NewCellDescription {}
+
+final class NewTextCellDescription: ConversationMessageCellDescription {


New description for first step migration of text message into new way. It will support for now sender, status and just simple message content, without links, previews, search queries, quoted messages

dmitrysimkin · 2025-05-19T09:06:43Z

.../Conversation/Content/Cells/ConfigurationMessageCell/Components/NewTextCellDescription.swift

+    }
+}
+
+final class NewTextCell: UIView, ConversationMessageCell {


Not used but needed to compile with old approach

dmitrysimkin · 2025-05-19T09:08:40Z

...nterface/Conversation/Content/Cells/ConfigurationMessageCell/Components/SenderObserver.swift

+final class SenderObserver: NSObject, UserObserving, SenderObserverProtocol {
+
+    var observation: Any?
+
+    var author: String?
+    private let authorChangedSubject = PassthroughSubject<String, Never>()
+    var authorChangedPublisher: AnyPublisher<String, Never> {
+        authorChangedSubject
+            .removeDuplicates()
+            .eraseToAnyPublisher()
+    }
+
+    init(
+        messageID: NSManagedObjectID,
+        viewContext: NSManagedObjectContext
+    ) {
+        super.init()
+        viewContext.perform {
+            let message = try! viewContext.existingObject(with: messageID) as! ZMMessage
+            self.author = message.senderName
+            if let sender = message.senderUser {
+                self.observation = UserChangeInfo.add(observer: self, for: sender, context: viewContext)


Object that helps be smarter with what to observe for each specific part of the message.
Also it reduces amount of updates if nothing really changed (.removeDuplicates())

dmitrysimkin · 2025-05-19T09:16:35Z

...os/Wire-iOS/Sources/UserInterface/Conversation/Content/ConversationTableViewDataSource.swift

        _ controller: ConversationMessageSectionController,
        didRequestRefreshForMessage message: ZMConversationMessage
    ) {
+        guard !message.isText else { return }


For new approach it would be not needed to call that in case of small changes that affect only cell itself

github-actions · 2025-05-19T09:38:44Z

Test Results

2 tests 1 ✅ 12s ⏱️
1 suites 1 💤
1 files 0 ❌

Results for commit 54d41ad.

datadog-wireapp · 2025-05-19T09:41:23Z

Datadog Report

Branch report: feature/improve-swipe-WPB-16787
Commit report: 764ee49
Test service: wire-ios-mono

✅ 0 Failed, 1 Passed, 1 Skipped, 12s Total Time

jullianm

Nice work so far, left a couple of comments/questions before approving

jullianm · 2025-05-20T13:13:08Z

...rsationUI/ConversationCell/CellTypes/TextMessage/Message Sender/MessageSenderViewModel.swift

+    public enum State {
+        case none
+        case some(MessageSenderViewModel)
+    }


question: Can't we get rid of the wrapper and use the view model directly as @ObservedObject by removing the constraint of having it optional ? The view should always be tightly coupled to its view model so no need for an optional here, no?

I'm not sure I got the suggestion right, can you pls elaborate @jullianm ?
For some cases we don't need to show sender so we need some way to tell that, nil or some specific state

Why do we need a view model wrapper , can't we use a state inside the view model like for the MessageStatusViewModel ?

But I have case none for MessageStatusViewModel as well for a case when we don't need to show status. It's just happen that status view has 2 more states: fail and callList, 4 in total. So approach is the same, @jullianm hope that makes more clear now?

jullianm · 2025-05-20T13:21:14Z

...ConversationUI/ConversationCell/CellTypes/TextMessage/Message Sender/SenderMessageView.swift

+    @ObservedObject var model: MessageSenderViewModelWrapper
+
+    var body: some View {
+        switch model.state {


question: I think here instead of relying on the view model state itself (whether it's optional or not) the view model should always be tightly coupled to its view and provide some state inside that the view would check (i.e model.state..)

jullianm · 2025-05-20T13:21:59Z

...ConversationUI/ConversationCell/CellTypes/TextMessage/Message Status/MessageStatusView.swift

+    @ObservedObject var model: MessageStatusViewModel
+
+    var body: some View {
+        switch model.state {


yep like this using the view model state property directly 👍

jullianm · 2025-05-20T13:22:24Z

...ConversationUI/ConversationCell/CellTypes/TextMessage/Message Status/MessageStatusView.swift

+
+struct MessageStatusView: View {
+
+    @ObservedObject var model: MessageStatusViewModel


Suggested change

@ObservedObject var model: MessageStatusViewModel

@ObservedObject var viewModel: MessageStatusViewModel

will rename in next PRs

jullianm · 2025-05-20T13:40:07Z

WireUI/Sources/WireConversationUI/ConversationCell/CellTypes/TextMessage/MessageModel.swift

+
+import Foundation
+
+public struct MessageModel: Equatable {


question: I wonder if we want to move these models to the WireDomain package and have, as such, kind of a centralized place where we retrieve these models from wherever we need them. just concerned about duplicating WireDataModel objects spread a little bit everywhere in new code.

update: these look like more like UIModels so probably best to keep it here.

jullianm · 2025-05-20T13:46:47Z

...Sources/WireConversationUI/ConversationCell/CellTypes/TextMessage/TextMessageViewModel.swift

+    public typealias ContentView = TextMessageView
+
+    @ObservedObject var senderViewModelWrapper: MessageSenderViewModelWrapper
+    @ObservedObject var statusViewModel: MessageStatusViewModel


thought: not a big fan of using these view models inside another view model as, for me, a view should have ideally only one view model although I'm assuming the code here will evolve in next PRs and probably these's no strict rule about this

For me it's really good way to split things up and reuse. later we'd have another message types like assets and with separate sender and status it would easier to just add it to message content and that's it.
This is similar what we have with cell descriptions, we have each for status, sender, reactions, etc. But with VM's it's more self-sufficient in terms if reactive change

jullianm · 2025-05-20T13:50:26Z

WireUI/Sources/WireReusableUIComponents/DateFormatter/DateFormatter+Message.swift

+
+import Foundation
+
+public extension DateFormatter {


suggestion: this could go to WireFoundation/Utilities so we could reuse it from any packages

Can't move to WireUtilities because it's a project. But I think I can move it to WireFoundation in next PR. However I don't have clear understanding of separation and what should go in e and what in WireDesign or WireReusableComponents

jullianm · 2025-05-20T13:53:44Z

...ation/Content/Cells/ConfigurationMessageCell/Components/ConversationMessageToolboxCell.swift

-        var message: ZMConversationMessage
-        var deliveryState: ZMDeliveryState
+        var message: MessageModel
+        var deliveryState: DeliveryStateModel


thought: I don't think we have a convention on that yet however to be more specific about the intent of these models we could rename them with the suffix UIModel

jullianm · 2025-05-20T14:11:13Z

...nversation/Content/Cells/ConfigurationMessageCell/ConversationMessageSectionController.swift

+        shouldShowSender: Bool,
+        shouldShowStatus: Bool
+    ) -> TextMessageViewModel
+}


praise: nice, I think we could use Needle foundation which acts as factories in next PRs

github-actions · 2025-06-23T00:40:27Z

This PR is stale because it has been open 30 days with no activity. Please update it or close it in case is not relevant anymore.

dmitrysimkin added 12 commits May 7, 2025 22:40

Added text message skelleton in new way

11063f1

Added skeleton for sender and status in new way

978c8f7

Make author be auto updatable

ae06404

Enable swiping

169f6be

Extract making VM into DI

2b2b268

Port sender, first part

ff4ec51

Initial port of message toolbox datasource

dfd6845

Update message satus

18524f6

Do not reload all when messages just updated

5c0cae1

Make cell not reload if just inner update

4548f6c

Use status state

d17b475

Fixed missing state updates

28f8fba

github-actions bot assigned dmitrysimkin May 15, 2025

dmitrysimkin changed the title ~~Feature/improve swipe wpb 16787~~ Feat: improve swipe - WPB-16787 May 15, 2025

dmitrysimkin added 5 commits May 15, 2025 14:23

wip

8a29cff

format

d698036

Port toolbox UI

2a4b3ea

Do not show sender if not needed

e8d7ae7

Merge branch 'develop' into feature/improve-swipe-WPB-16787

cabd2a9

# Conflicts: # wire-ios/Wire-iOS/Sources/UserInterface/Conversation/Content/Cells/ConfigurationMessageCell/ConversationMessageSectionController.swift

dmitrysimkin changed the base branch from develop to feature/base-improve-swipe-WPB-16787 May 16, 2025 12:28

Code style

54d41ad

dmitrysimkin commented May 19, 2025

View reviewed changes

dmitrysimkin changed the title ~~Feat: improve swipe - WPB-16787~~ Feat: improve swipe pt.1 - WPB-16787 May 19, 2025

github-actions bot added the Fix PR Title 🤦‍♂️ label May 19, 2025

dmitrysimkin requested review from a team, netbe, samwyndham and jullianm and removed request for a team May 19, 2025 09:18

dmitrysimkin changed the title ~~Feat: improve swipe pt.1 - WPB-16787~~ feat: improve swipe pt.1 - WPB-16787 May 19, 2025

github-actions bot removed the Fix PR Title 🤦‍♂️ label May 19, 2025

dmitrysimkin marked this pull request as ready for review May 19, 2025 09:24

dmitrysimkin requested review from caldrian and johnxnguyen May 19, 2025 09:25

dmitrysimkin requested a review from KaterinaWire May 19, 2025 09:47

jullianm reviewed May 20, 2025

View reviewed changes

dmitrysimkin requested a review from jullianm May 22, 2025 06:52

github-actions bot added the stale label Jun 23, 2025

dmitrysimkin marked this pull request as draft July 28, 2025 10:00

netbe added the WIP label Jul 29, 2025


		struct MessageStatusView: View {

		@ObservedObject var model: MessageStatusViewModel

	@ObservedObject var model: MessageStatusViewModel
	@ObservedObject var viewModel: MessageStatusViewModel

feat: improve swipe pt.1 - WPB-16787 #3019

Are you sure you want to change the base?

feat: improve swipe pt.1 - WPB-16787 #3019

Uh oh!

Conversation

dmitrysimkin commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Testing

Checklist

UI accessibility checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jullianm May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmitrysimkin May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 19, 2025

Test Results

Uh oh!

datadog-wireapp bot commented May 19, 2025

Datadog Report

Uh oh!

jullianm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jullianm May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jullianm May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

dmitrysimkin commented May 15, 2025 •

edited

Loading

jullianm May 20, 2025 •

edited

Loading

dmitrysimkin May 19, 2025 •

edited

Loading

jullianm May 20, 2025 •

edited

Loading

jullianm May 20, 2025 •

edited

Loading