Implement message actions and message status in the new ChatView #1471

lissine0 · 2025-08-30T18:46:45Z

Support for replies or quotes will come later in the future, as part of another PR.

lissine0 · 2025-08-30T19:07:11Z

Continuing the discussion from this comment thread #1465 (comment):

the only thing I'd write in the main thread would be new sent messages because we want them in our db the instance we show them on screen [otherwise users could think the message was written, force close the app and loose the message if the db was heavily congested]

I agree that we want the newly sent message in the DB the moment it's shown on screen. But, we can delay displaying the new message until after the DB write is finished.

In fact, that is the case already for message sending.

So, by running the DB write on a separate thread, we avoid blocking the UI: the user can scroll up and down; and they know their message wasn't sent yet, as it still didn't show up in a chat bubble (and it probably is still visible in the input view)

I think this same reasoning applies to other message actions like correction, retraction and local deletion, so I adhered to it in this PR: run the DB write on a separate thread, and only update the UI after the write is finished.

lissine0 · 2025-09-01T21:57:12Z

I ended up choosing an empty icon for .sent message status, like the UIKit ChatView, as it looks more natural in public channels, or when the contact has disabled sending read receipts and delivery receipts.

lissine0 · 2025-09-01T22:07:41Z

Monal/Classes/ChatView.swift

+                    case .copy:
+                        defaultActionClosure(message, .copy)
+                    case .edit:
+                        defaultActionClosure(message, .edit { editedText in


This .edit closure is defined as Sendable in ExyteChat.

This results in the following warning on the sendMessage call a couple of lines below :

Capture of 'mlMessage' with non-sendable type 'MLMessage' in a `@Sendable` closure; this is an error in the Swift 6 language mode Class 'MLMessage' does not conform to the 'Sendable' protocol (monalxmpp.MLMessage)

We can't make the MLMessage class Sendable (that would require it to contain only stored properties that are immutable and sendable), so I silenced the warning by declaring it @unchecked Sendable in this source file.

One of the reasons why I din't like the direction Swift is currently moving to, it makes easy things much much harder or even impossible, even if the programmer knows what they are doing :(
Same goes for the stupid actor stuff that should ensure that ui things run on the main thread, but not detecting that we are already on the main thread.

That said, I'm not sure why the closure was defined as Sendable in the first place. @matthewrfennell do you have any insights here?

@matthewrfennell Do you know why this closure was defined as sendable?
Would it be possible to remove that sendable requirement upstream? That would spare us this dirty MLMessage as Sendable hack

lissine0 · 2025-09-01T22:28:47Z

Note that for messages with .error status, if the user clicks the status icon, a new message with the same contents is sent. (i.e. the failed message remains visible)
This is built-in to ExyteChat.

On the other hand, the "Resend" message action I added resends the same message (it reuses the same IDs)

This means we have two, slightly different ways to resend a failed message.
I don't think it's that big of a problem, but if you want, we can comment out these couple of lines in ExyteChat to disable the built-in way https://github.com/exyte/Chat/blob/114f42ab9e1ff6/Sources/ExyteChat/Views/MessageView/MessageView.swift#L129,L131

tmolitor-stud-tu · 2025-09-03T02:04:14Z

I agree that we want the newly sent message in the DB the moment it's shown on screen. But, we can delay displaying the new message until after the DB write is finished.

Oh yes, good idea!

So, by running the DB write on a separate thread, we avoid blocking the UI: the user can scroll up and down; and they know their message wasn't sent yet, as it still didn't show up in a chat bubble (and it probably is still visible in the input view)
I think this same reasoning applies to other message actions like correction, retraction and local deletion, so I adhered to it in this PR: run the DB write on a separate thread, and only update the UI after the write is finished.

Yes, great! :)

tmolitor-stud-tu · 2025-09-03T02:07:04Z

Note that for messages with .error status, if the user clicks the status icon, a new message with the same contents is sent. (i.e. the failed message remains visible) This is built-in to ExyteChat.

On the other hand, the "Resend" message action I added resends the same message (it reuses the same IDs)

This means we have two, slightly different ways to resend a failed message. I don't think it's that big of a problem, but if you want, we can comment out these couple of lines in ExyteChat to disable the built-in way https://github.com/exyte/Chat/blob/114f42ab9e1ff6/Sources/ExyteChat/Views/MessageView/MessageView.swift#L129,L131

Why did you add a "resend" action in the first place?

I think removing the old message having the error when resending using the status icon would be better, if that's easily achievable.

tmolitor-stud-tu · 2025-09-03T02:12:18Z

I ended up choosing an empty icon for .sent message status, like the UIKit ChatView, as it looks more natural in public channels, or when the contact has disabled sending read receipts and delivery receipts.

Sounds sensible :)

lissine0 · 2025-09-03T04:36:31Z

Why did you add a "resend" action in the first place?

I did so because I thought the error status icon was too small, and that it's not obvious that it's a clickable button. (Note however that some of the area neighboring the icon to the top, right, and top-right is also clickable)

Here's a screenshot

lissine0 · 2025-09-03T05:07:18Z

I think removing the old message having the error when resending using the status icon would be better, if that's easily achievable.

That may be easily achievable on the ExyteChat side, but the message won't be removed from the DB, meaning it'll be shown the next time the ChatView is entered.

A proper solution is to modify ExyteChat, allowing library users to provide a custom closure that is triggered when the error status icon is clicked.

If you prefer clicking the error status icon over using the message actions, I can try to contribute this change to ExyteChat. (A custom make it possible to also remove the "Show Error" message action.)

tmolitor-stud-tu · 2025-09-19T23:33:20Z

A proper solution is to modify ExyteChat, allowing library users to provide a custom closure that is triggered when the error status icon is clicked.

If you prefer clicking the error status icon over using the message actions, I can try to contribute this change to ExyteChat. (A custom make it possible to also remove the "Show Error" message action.)

yes please :)

lissine0 · 2025-10-13T06:49:45Z

Here's how the UI for resending messages is currently implemented:

Simulator_resending_failed_message.mp4

Also, note that I tested the localization extraction script, and the following string is extracted successfully:
https://github.com/lissine0/Monal/blob/4ee957b4cc81c64c78c01433fb77c6f03fa2ed43/Monal/Classes/ChatView.swift#L334

tmolitor-stud-tu · 2025-10-14T23:14:14Z

Sure, I've updated the extraction script a few months ago to really build the project, so everything taking a LocalizedStringKey gets properly extracted (even when used as parameter for our own functions etc.) :)

tmolitor-stud-tu

Only small comments

Monal/Classes/ChatView.swift

tmolitor-stud-tu · 2025-10-14T23:22:14Z

Monal/Classes/ChatView.swift

+                    case .copy:
+                        defaultActionClosure(message, .copy)
+                    case .edit:
+                        defaultActionClosure(message, .edit { editedText in


@matthewrfennell Do you know why this closure was defined as sendable?
Would it be possible to remove that sendable requirement upstream? That would spare us this dirty MLMessage as Sendable hack

Monal/Classes/ChatView.swift

tmolitor-stud-tu · 2025-10-14T23:58:01Z

Monal/Classes/ChatView.swift

                }
            }))
        }
+        .alert("Moderating message", isPresented: $showModeratingMessageAlert, actions: {


Maybe richAlert would be a better fit than alert? I don't know how both would look and what would look better.

I didn't try the richAlert yet

This is how it looks like with alert:

Though I admit it might look a bit ugly on macOS:

I didn't try richAlert because it will require a bit of refactoring to use .optionalMappedToBool() with it:
richAlert expects isPresented to be a Binding<T?> but optionalMappedToBool returns a Binding<Bool>, and the Swift compiler can't convert automatically.

The error is:

Cannot convert value of type 'Binding<Bool>' to expected argument type 'Binding<()?>' Arguments to generic parameter 'Value' ('Bool' and '()?') are expected to be equal

You can check the invite creation view to get a feel for richAlert: its background is blurred, it has a small shadow (try it in dark mode!), and it would look the same on iOS and macOS.

No, you don't need the optionalMappedToBool at all. The rich alert will do this internally already :) You can just use the value directly and the alert will pop up if it is not nil.

Monal/Classes/ChatView.swift

…tQueue

by updating the DB and posting the deletion notification inside xmpp retractMessage: and by doing so asynchronously on the receiveQueue, in the same block where the retraction is sent

Support for replies or quotes will come later

lissine0 · 2025-10-22T07:05:34Z

Monal/Classes/ChatView.swift

+                                                                 to: self.contact.obj,
+                                                                 isEncrypted: isEncrypted,
+                                                                 isUpload: isUpload,
+                                                                 andMessageId: mlMessage.messageId)


This is how Xcode automatically indents multiline function arguments:

self.account.sendMessage(mlMessage.messageText, to: self.contact.obj, isEncrypted: isEncrypted, isUpload: isUpload, andMessageId: mlMessage.messageId)

But if you prefer we could use 4-space indentation for the argument list instead:

self.account.sendMessage(editedText, to: self.contact.obj, isEncrypted: self.contact.isEncrypted || mlMessage.encrypted, isUpload: false, andMessageId: UUID().uuidString, withLMCId: mlMessage.messageId )

Well, I somewhat lean to the 4-space version. But we for sure have the XCode version in our codebase, too.

Nearly all code written by me uses the 4-space version, though.

lissine0 force-pushed the message-actions branch from f5af703 to 5c59276 Compare August 31, 2025 14:01

lissine0 changed the title ~~Implement our supported message actions in the new ChatView~~ Implement message actions and message status in the new ChatView Sep 1, 2025

lissine0 force-pushed the message-actions branch from 5c59276 to 245e7c2 Compare September 1, 2025 21:29

lissine0 commented Sep 1, 2025

View reviewed changes

lissine0 force-pushed the message-actions branch from 245e7c2 to 1d75594 Compare September 1, 2025 22:37

lissine0 force-pushed the message-actions branch from 1d75594 to cccf314 Compare September 7, 2025 00:15

tmolitor-stud-tu force-pushed the develop branch 3 times, most recently from 4ce6b4e to 0679810 Compare September 17, 2025 01:35

lissine0 added 2 commits September 17, 2025 23:55

Ensure "This message got retracted" is localizable in the ChatView

e86bfac

Don't reorder active chats when receiving corrections or retractions

aee4926

lissine0 force-pushed the message-actions branch from cccf314 to 8753f19 Compare September 17, 2025 22:55

tmolitor-stud-tu force-pushed the develop branch from 5b3d3b0 to 0e6bc5d Compare September 18, 2025 21:57

tmolitor-stud-tu force-pushed the develop branch 8 times, most recently from f92692c to 6d1997e Compare September 25, 2025 01:40

tmolitor-stud-tu force-pushed the develop branch 6 times, most recently from 518a17d to eaba046 Compare October 11, 2025 02:43

lissine0 force-pushed the message-actions branch from 8753f19 to 4ee957b Compare October 13, 2025 06:39

lissine0 requested a review from tmolitor-stud-tu October 13, 2025 06:50

tmolitor-stud-tu reviewed Oct 15, 2025

View reviewed changes

lissine0 force-pushed the message-actions branch from 4ee957b to 0f0e9a3 Compare October 21, 2025 22:05

lissine0 added 6 commits October 22, 2025 06:24

Specify instancetype as the return type of MLNotificationQueue curren…

02fce74

…tQueue

Make message retraction more transactional

73e05a2

by updating the DB and posting the deletion notification inside xmpp retractMessage: and by doing so asynchronously on the receiveQueue, in the same block where the retraction is sent

Implement our supported message actions in the new ChatView

46b884f

Support for replies or quotes will come later

Ask the user for the message moderation reason

ffe49fa

Bump SwiftPM dependencies

b8f1c42

Display message status in the new ChatView

04f101b

lissine0 force-pushed the message-actions branch from 0f0e9a3 to 04f101b Compare October 22, 2025 07:01

lissine0 commented Oct 22, 2025

View reviewed changes

lissine0 requested a review from tmolitor-stud-tu October 22, 2025 07:10

tmolitor-stud-tu force-pushed the develop branch 7 times, most recently from bc01177 to 6e8eeac Compare October 26, 2025 07:15

tmolitor-stud-tu approved these changes Nov 5, 2025

View reviewed changes

tmolitor-stud-tu merged commit 628694e into monal-im:develop Nov 5, 2025
1 check passed

Uh oh!

Implement message actions and message status in the new ChatView #1471

Implement message actions and message status in the new ChatView #1471

Uh oh!

Conversation

lissine0 commented Aug 30, 2025

Uh oh!

lissine0 commented Aug 30, 2025

Uh oh!

lissine0 commented Sep 1, 2025

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!

lissine0 commented Sep 1, 2025

Uh oh!

tmolitor-stud-tu commented Sep 3, 2025

Uh oh!

tmolitor-stud-tu commented Sep 3, 2025

Uh oh!

tmolitor-stud-tu commented Sep 3, 2025

Uh oh!

lissine0 commented Sep 3, 2025

Uh oh!

lissine0 commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmolitor-stud-tu commented Sep 19, 2025

Uh oh!

lissine0 commented Oct 13, 2025

Uh oh!

tmolitor-stud-tu commented Oct 14, 2025

Uh oh!

tmolitor-stud-tu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

tmolitor-stud-tu Nov 5, 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!

Uh oh!

lissine0 Oct 22, 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lissine0 commented Sep 3, 2025 •

edited

Loading

tmolitor-stud-tu Nov 5, 2025 •

edited

Loading

lissine0 Oct 22, 2025 •

edited

Loading