Feels dangerous to add a type to a project where the exact intention is it'll immediately crash?

Am I missing something?

@Sherlouk It's to allow people to make custom cells, basically if messageForItem(at: IndexPath) returns a MessageType with MessageData.custom(Any?) I expect them to handle it by overriding cellForItem(at:) and any other sizing methods and then calling super for any other types

Is that documented somewhere? If you use the Custom MessageType then you are expected to override the ...

fatalError documentation, best documentation

Nah that just tells you that you fucked up! Not how to fix it!

_{Sent with GitHawk}

This would be super useful so I gave the changes a quick go locally, but hit a snag 😅: Declarations from extensions cannot be overridden yet when trying to subclass MessagesCollectionViewFlowLayout and overriding messageContainerSize(). I can see its still in progress, so just an FYI 🙂

Yeah @ash30 this will require the next release, MessageKit 0.14.0, where we already made this changes

_{Sent with GitHawk}

Can you show a example of implementation of .custom(Any)

@stephtelolahy We'll definitely have some example documentation but I can't get to that right now. You can do a lot with .custom(Any). You basically just need to handle cellForItem(at: IndexPath) in MessagesViewController and messageContainerSize(for:at:) in MessagesCollectionViewFlowLayout.

@SD10 I tried to override messageContainerSize(for:at:) using the subclass of MessagesCollectionViewFlowLayout but I got Declarations from extensions cannot be overridden yet error. Apparently, I don't have enough knowledge about how to override this function. Could you explain how I can do this?

@SD10 , Oops , we can only put non-override method in extentions.

Wow that is surprising because I'm allowed to mark them as open 😅 We can fix this

Suggestion:

open func messageContainerSize(for message: MessageType, at indexPath: IndexPath) -> CGSize {
  // ...
  case .custom:
    return messageContainerSizeForCustom()
  // ...
}

@objc open func messageContainerSizeForCustom() -> CGSize {
   fatalError(MessageKitError.customDataUnresolvedSize)
}

This way we could add our custom behavior by overriding messageContainerSizeForCustom without touching other MessageTypes.

By marking as @objc you would solve the non-override problem I guess.

Same thing could work for collectionView(_:cellForItemAt indexPath: IndexPath) -> UICollectionViewCell.

wdyt?

@yokomizor That's an interesting suggestion. Why not just override and call super though?

@SD10 calling super will check MessageType again, and throw the fatalError.

If you remove the case .custom, then calling super would be fine, but we will have to check message.data twice. Super will check first. Later our override will check it again (we just want to add custom behavior for .custom).

Maybe it would be even better if you delegate this to messagesLayoutDelegate.messageContainerSize(...).

wdyt?

@yokomizor I was expecting users to override, handle the .custom case. And then call super afterwards to handle the remaining cases.

@yokomizor Also keep in mind, you really should not be using development. I will break a ton of things on this branch all the time. I may revert all these changes to the layout until a later release.

@SD10 Oh! true. We don't need messageContainerSizeForDefault.

How do you expect users will set a custom collectionViewLayout?

I suppose users could pass the custom one through init:

To do so, they need to set messagesCollectionView again on MessagesViewController since the default one is created on init phase:

So, the default instance will be created anyway, even if users would not use it.

I understand this is wip. Anyway, it would be nice to know which path you are planning to follow.

btw, wdyt about the delegate suggestion? Users are used to override default behavior using MessagesDisplayDelegate. It might be less disruptive.

Cheers!

@SD10 btw, back to the override topic. Even calling super later, MessageType will be checked twice. First to check if it is .custom and then later super will check it again. Using messageContainerSizeForDefault MessageType will be checked just once. Am I missing something?

@yokomizor , If you want to use custom layout, I think override messagesCollectionView and implement its set、get method would be a good way.

@yokomizor You would return before super is called in the case of .custom. The switch will not be executed a second time

_{Sent with GitHawk}

@SD10 for .custom just once. for .text twice.

@zhongwuzw cool. So delegates won't be an option? Less disruptive maybe 👼

@yokomizor

override func messageContainerSize(for message: MessageType, at indexPath: IndexPath) -> CGSize {
    switch message.data {
   case .custom(_): 
      // handle sizing
   default: 
      super.messageContainerSize(for: message, at: indexPath)
   }
}

No double checks

@SD10

message = .text(...)

1. switch message.data will check message type once. Then default: case will be called.

2. Inside super, switch message.data will check message type again. Then case .text(_): will be called.

My point is that we could check it just once in order to address it to the right handler. Makes sense? Calling switch twice would't be a problem, though.

It shouldn’t make a difference in production, especially since a switch will exit at the first match. It is a good point to bring up though 👍👍

_{Sent with GitHawk}