GetStream / stream-chat-swift

💬 iOS Chat SDK in Swift - Build your own app chat experience for iOS using the official Stream Chat API
https://getstream.io/chat/sdk/ios/
Other
853 stars 205 forks source link

Error when using `ChatMessageListVC` with `ChatMessageSearchController` #2654

Closed brianmwadime closed 1 year ago

brianmwadime commented 1 year ago

What did you do?

Tried using the ChatMessageListVC in custom ViewController to display messages from a ChatMessageSearchController

What did you expect to happen?

The messages fetched from ChatMessageSearchController would be updated in the custom ChatMessageListVC and messages would be visible depending on the search parameters passed.

What happened instead?

When using the breakpoint navigator, the channelController( _ channelController: ChatChannelController, didUpdateMessages changes: [ListChange<ChatMessage>] ) gets called from the ChatChannelVC (even though it's not referenced in my custom view controller), then my attached delegate calls ChatMessageSearchControllerDelegate.controller(_ controller: ChatMessageSearchController, didChangeMessages changes: [ListChange<ChatMessage>])

then lastly the error below is thrown and the messages are not displayed

[TableView] Warning once only: UITableView was told to layout its visible cells and other contents without being in the view hierarchy (the table view or one of its superviews has not been added to a window). This may cause bugs by forcing views inside the table view to load and perform layout without accurate information (e.g. table view bounds, trait collection, layout margins, safe area insets, etc), and will also cause unnecessary performance overhead due to extra layout passes. Make a symbolic breakpoint at UITableViewAlertForLayoutOutsideViewHierarchy to catch this in the debugger and see what caused this to occur, so you can avoid this action altogether if possible, or defer it until the table view has been added to a window. Table view: <StreamChatUI.ChatMessageListView: 0x10b13fc00; baseClass = UITableView; frame = (0 0; 375 474); transform = [1, 0, 0, -1, 0, 0]; clipsToBounds = YES; gestureRecognizers = <NSArray: 0x2826320d0>; backgroundColor = <UIDynamicSystemColor: 0x283c24780; name = systemBackgroundColor>; layer = <CALayer: 0x28285e280>; contentOffset: {0, 0}; contentSize: {375, 1034.5}; adjustedContentInset: {0, 0, 8, 0}; dataSource: <StreamChatUI.ChatMessageListVC: 0x10b1c2000>>

GetStream Environment

GetStream Chat version: 4.31.0 GetStream Chat frameworks: StreamChat, StreamChatSwiftUI iOS version: 16.3.1 Swift version: 5 Xcode version: Xcode 14.3 Device: Iphone 8

Additional context

nuno-vieira commented 1 year ago

Hi @brianmwadime,

The ChatMessageListVC is not meant to be used as a Searching functionality, so this will probably produce unexpected issues. The ChatMessageListVC is meant to be used as a standalone component if, for some reason, you want to handle the composer yourself, and so the ChatChannelVC and the ChatThreadVC does not work for you.

Either way, can you share your implementation? Are you implementing the ChatMessageListVCDataSource and ChatMessageListVCDelegate?

You can ignore that console error for now, that is just a warning, and it won't have any impact on this one.

Best, Nuno

brianmwadime commented 1 year ago

Hi @brianmwadime,

The ChatMessageListVC is not meant to be used as a Searching functionality, so this will probably produce unexpected issues. The ChatMessageListVC is meant to be used as a standalone component if, for some reason, you want to handle the composer yourself, and so the ChatChannelVC and the ChatThreadVC does not work for you.

Either way, can you share your implementation? Are you implementing the ChatMessageListVCDataSource and ChatMessageListVCDelegate?

You can ignore that console error for now, that is just a warning, and it won't have any impact on this one.

Best, Nuno

Hey @nuno-vieira, we are using it as a standalone component since we don't need the composer. I have also implemented both ChatMessageListVCDataSource and ChatMessageListVCDelegate.

I also noticed initially when performing a search on ViewDidLoad the messages aren't displayed but if I move the app to the background and them back to foreground the messages appear in the list.

brianmwadime commented 1 year ago

@nuno-vieira does this mean if I wanted to create screens that show for example a list of only the attachments (media), I can't use the ChatStream components? I'll have to build out the UI?

nuno-vieira commented 1 year ago

Hi @brianmwadime,

And are you using the ChatMessageSearchControllerDelegate.didChangeMessages to populate the ChatMessageListVC and refresh the data? It is easier if you send your implementation so that we can review it. If you don't want to send it here, you can send it to nuno@getstream.io

Best, Nuno

nuno-vieira commented 1 year ago

Hi, @brianmwadime!

For the list of only the attachments, yes, you will need to build the UI yourself at the moment. We are constantly improving the SDK and adding new out-of-the-box components, but as of now, we don't have an ETA to provide a UI Component for showing the media files only. But, for the search functionality we will provide components out of the box very soon.

Best, Nuno

brianmwadime commented 1 year ago

Sent via email @nuno-vieira

nuno-vieira commented 1 year ago

Hi @brianmwadime!

So looking at the code, I see 2 things that need to be changed.

messagesController.search(query: query, completion: { [weak self] _ in
   guard let self = self else { return }
   debugPrint("Messages Count: ", messagesController.messages.count, separator: "\n")
   self.messages = Array(self.messagesController.messages)
})

This part should be changed to just this:

messagesController.search(query: query)

You only need the completion closure if you want to display an error. The data is only updated through the delegate.

Then, in the delegate, your current implementation does not do anything, unfortunately. This would be the correct implementation:

func controller(_ controller: ChatMessageSearchController, didChangeMessages changes: [ListChange<ChatMessage>]) {
  messageListVC.setPreviousMessagesSnapshot(messages)
  messageListVC.setNewMessagesSnapshot(Array(controller.messages))
  messageListVC.listView.updateMessages(with: changes)
}

The problem is that all these APIs are private at the moment. So we might need to change this. But for now, as a workaround just to check if it works, you can do it like this:

func controller(_ controller: ChatMessageSearchController, didChangeMessages changes: [ListChange<ChatMessage>]) {
  messages = Array(controller.messages)
  messageListVC.listView.reloadData()
}

Let me know if, with this, you can see the data.

Best, Nuno

brianmwadime commented 1 year ago

@nuno-vieira yes, accessing reloadData() fixes the issue. Thank you. Also is there a way to load the messages normally (from the top) not like the Chat message screen?

nuno-vieira commented 1 year ago

Hi @brianmwadime

What do you mean by that? Do you want the most recent messages to render at the top?

brianmwadime commented 1 year ago

@nuno-vieira for example if I m displaying a single message, I want it to be at the top of the screen (like a regular list) not start from the bottom.

nuno-vieira commented 1 year ago

Ah, for that, you can do this:

Components.default.shouldMessagesStartAtTheTop = true

But keep in mind, if the message list is big, it will still display the bottom messages first. The shouldMessagesStartAtTheTop applies only when the message list has a few messages.

Let me know if this is what you want

brianmwadime commented 1 year ago

@nuno-vieira yes, but just for the knowledge how can I reverse the order as well?

nuno-vieira commented 1 year ago

Currently, we do not support this. The Chat UI is an inverted table view at the moment and is meant for rendering a conversation where newer messages are rendered at the bottom. For now, we are not planning on changing this.

This is why we do not recommend using ChatMessageListVC for message search since it was not designed for that.

Usually, people render the message search in a regular Table View or Collection View, and then you click on that message and you jump to the conversation at that location. This is also how our built-in Search component will work. This searching functionality is usually done in the Channel List Screen, but it can be done also in other places.

brianmwadime commented 1 year ago

@nuno-vieira yeah, thought we'd be able to reuse the message cell and all the logic it comes with out of the box by using the ChatMessageListVC, but I haven't seen examples of this with a custom tableview/collectionview.

nuno-vieira commented 1 year ago

We don't have our own examples yet, since we will soon have built-in components for this. This is planned for the end of Q2. For now, we don't have any customers doing a search of messages with the ChatMessageCell design. It is more with the Channel List Item View design. This is how our builtin search component will look like:

image

On the Search List View, the results are displayed like a Channel List, and then you click on the result and jump to the conversation.

Using the ChatMessageCell for doing searches is not something we are supporting for now. If you want this to do a search like WhatsApp does, I believe this won't work very well. The reason is that WhatsApp has all messages available locally, so in this case, it is easy to implement this. But in our case, when doing a search, most of the messages are not available locally, only remotely. So there will be gaps if you render the search results in the message list.

I don't have access to your designs, so I'm not sure what you are trying to implement, but if it is to mimic WhatsApp search inside the conversation, I'm afraid that won't work very well.

brianmwadime commented 1 year ago

@nuno-vieira got it. I have one last question, is it possible to filter for just for lets say [.images, .video] for a specific channel in the ChatChannelController or that is just available on the ChatMessageSearchController?

nuno-vieira commented 1 year ago

Yes that is correct, only ChatMessageSearchController allows that

nuno-vieira commented 1 year ago

I'm closing this one for now; feel free to open new issues if you have any troubles 👍

Best, Nuno