New ChatView: Support incoming updates of messages #1426
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -200,7 +200,9 @@ struct ChatView: View { | |
| guard let newMLMessage = MLXMPPManager.sharedInstance().sendMessageAndAddToHistory(message: draft.text, havingType: kMessageTypeText, toContact: self.contact.obj, isEncrypted: self.contact.isEncrypted, uploadInfo: nil) else { | ||
| return | ||
| } | ||
| messages.append(ChatViewMessage(ObservableKVOWrapper(newMLMessage))) | ||
| } messageBuilder: { message, viewModel, positionInUserGroup, positionInMessagesSection, positionInCommentsGroup, showContextMenuClosure, messageActionClosure, showAttachmentClosure in | ||
| MessageView(message: (message as! ChatViewMessage).message, viewModel: viewModel, positionInUserGroup: positionInUserGroup, positionInMessagesSection: positionInMessagesSection) | ||
| } | ||
| .showNetworkConnectionProblem(false) | ||
| // .enableLoadMore(pageSize: 3) { message in | ||
|
|
@@ -350,7 +352,7 @@ struct ChatView: View { | |
| if messages.isEmpty { | ||
| let dbMessages = DataLayer.sharedInstance().messages(forContact:contact.contactJid, forAccount:contact.accountID) as! [MLMessage] | ||
| for msg in dbMessages { | ||
| messages.append(ChatViewMessage(ObservableKVOWrapper(msg))) | ||
| } | ||
| } | ||
| ChatViewHelpers.refreshCounter(for: self.contact.obj) | ||
|
|
@@ -386,7 +388,7 @@ struct ChatView: View { | |
| MLNotificationManager.sharedInstance().currentContact = nil | ||
| } | ||
| } | ||
| .onReceive(NotificationCenter.default.publisher(for: NSNotification.Name(kMonalOmemoFetchingStateUpdate)).receive(on: RunLoop.main)) { notification in | ||
| if let xmppAccount = notification.object as? xmpp, let notificationJid = notification.userInfo?["jid"] as? String { | ||
| if xmppAccount.accountID == contact.accountID && notificationJid == contact.contactJid { | ||
| DDLogDebug("Got omemo fetching update: \(contact) --> \(String(describing:notification.userInfo))") | ||
|
|
@@ -398,7 +400,7 @@ struct ChatView: View { | |
| } | ||
| } | ||
| } | ||
| .onReceive(NotificationCenter.default.publisher(for: NSNotification.Name(kMonalNewMessageNotice)).receive(on: RunLoop.main)) { notification in | ||
| DDLogVerbose("chat view got new message notice \(String(describing:notification.userInfo))") | ||
|
|
||
| guard let message = notification.userInfo?["message"] as? MLMessage else { | ||
|
|
@@ -408,23 +410,24 @@ struct ChatView: View { | |
| if message.isEqual(to: self.contact.obj) { | ||
| // Do not insert based on delay timestamp because that | ||
| // would make it possible to fake history entries. | ||
| messages.append(ChatViewMessage(ObservableKVOWrapper(message))) | ||
| } | ||
| ChatViewHelpers.refreshCounter(for: self.contact.obj) | ||
| } | ||
| .onReceive(NotificationCenter.default.publisher(for: NSNotification.Name(kMonalRefresh)).receive(on: RunLoop.main)) { notification in | ||
| ChatViewHelpers.refreshCounter(for: self.contact.obj) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| class ChatViewMessage: ExyteChat.Message { | ||
| let message: ObservableKVOWrapper<MLMessage> | ||
|
|
||
| init(_ message: ObservableKVOWrapper<MLMessage>) { | ||
| self.message = message | ||
| let messageText = message.retracted ? NSLocalizedString("This message got retracted", comment: "") : message.messageText | ||
| let user = ExyteChat.User(id: message.senderID, name: message.contactDisplayName, avatarURL: nil, isCurrentUser: !message.inbound) | ||
| super.init(id: message.id, user: user, createdAt: message.timestamp, text: messageText) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -481,3 +484,32 @@ public extension ExyteChat.MessageView { | |
| // } | ||
| } | ||
| }*/ | ||
| struct MessageView: View { | ||
| @StateObject var message: ObservableKVOWrapper<MLMessage> | ||
| @ObservedObject var viewModel: ExyteChat.ChatViewModel | ||
| let positionInUserGroup: PositionInUserGroup | ||
| let positionInMessagesSection: PositionInMessagesSection | ||
| init(message: ObservableKVOWrapper<MLMessage>, viewModel: ChatViewModel, positionInUserGroup: PositionInUserGroup, positionInMessagesSection: PositionInMessagesSection) { | ||
|
Member
There was a problem hiding this comment. Why don't you pass in the
Member
Author
There was a problem hiding this comment. I tested this suggestion and the view auto updates stopped working.
Member
There was a problem hiding this comment. They are going to be picked up by the
Member
Author
There was a problem hiding this comment.
The
Member
Author
There was a problem hiding this comment. We need to write our custom MessageView, and use MLMessage properties in it directly, in order for it to work how you said
Member
There was a problem hiding this comment. Yes, I alreay said in some review that you should overwrite the properties of See #1426 (comment) That means something like this (maybe you could try a proof of concept first, before updating this PR to save some work in case it doesn't work like depicted below):
Okay, step 4 is a problem here, apparently you cannot overwrite a stored property with a computed one :( Solution:
I think introducing a new protocol is the way to go here, because it makes the original ExyteChat stuff more extensible but doesn't change any behavior. I think we can even upstream that change (together with making btw: the
Member
There was a problem hiding this comment. @matthewrfennell what do you think, would that protocol be upstreamable? |
||
| _message = StateObject(wrappedValue: message) | ||
| self.viewModel = viewModel | ||
| self.positionInUserGroup = positionInUserGroup | ||
| self.positionInMessagesSection = positionInMessagesSection | ||
| } | ||
| var body: some View { | ||
| ExyteChat.MessageView( | ||
| viewModel: viewModel, | ||
| message: ChatViewMessage(message), | ||
| positionInUserGroup: positionInUserGroup, | ||
| positionInMessagesSection: positionInMessagesSection, | ||
| chatType: .conversation, | ||
| avatarSize: 32, | ||
| tapAvatarClosure: nil, | ||
| messageStyler: AttributedString.init, | ||
| shouldShowLinkPreview: { _ in true }, | ||
| isDisplayingMessageMenu: false, | ||
| showMessageTimeView: true, | ||
| messageLinkPreviewLimit: 8, | ||
| font: UIFontMetrics.default.scaledFont(for: UIFont.systemFont(ofSize: 15)) | ||
| ) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe don't wrap it into
ObservableKVOWrapperoutside, but wrap it inside the constructor ofChatViewMessage. That seems a bit cleaner because it doesn't expose these implementation details to the caller.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing that causes the following error on L502:
If I replace
messagewithmessage.obj, the automatic view updates won't work anymoreThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you should change the code to "bubble that change up", aka: only wrap the MLMessage object into the kvo observer once and pass around the kvo observer or some object containing it rather then recreating things over and over again on every render.