Messagekit: Explore using IGListKit as a dependency

[SD10]

I don't take adding any dependency to the project lightly, but I think there could be a lot to gain from adding IGListKit to this project. Possibly after the Swift bridge is completed.

Development has gone stale lately because the current architecture is pushing back at us. I definitely made some design decisions I regret and have learned some really painful lessons through this project. It's really difficult to build flexible abstractions that everyone can use. Especially for a UI library.

The reason MessageKit is struggling is the following areas:

1. Building a complex UICollectionViewCell
   
   
   
   • All the views are in a single cell. This has the following problems:
     
     
     
     
     
     • The layout logic is complex because one view's position depends on another view
     
     
     
     • More features == more views. Not everyone wants all of the views in the cell and want to turn some of them off. This results in unnecessary sizing calculations.
     
     
     
   
   
2. Performance of dynamically sizing a cell
   
   
   
   • Dynamically sizing a cell with 3+ NSAttributedStrings and other views is a _huge_ cost.
   
   
   • A cell with less views would only perform sizing operations when absolutely necessary.
   
   
3. Allowing more than 1 cell per section
   
   
   
   • MessageKit currently has a limitation of 1 message : 1 cell : 1 section
   
   

Here is my opinion on the pros and cons:

Pros

• Simplify existing cell view hierarchy
• Greater flexibility for creating complex cells and cell structures
• Decoupled diffing algorithm out of the box

Cons

• Adopting a dependency (and an Obj-C dependency)
• Potentially very API breaking
• Could increase the barrier of entry to the project

I've explored mimicking a small subset of the IGListKit architecture without adopting the full dependency. It just doesn't make sense to re-invent the wheel for something like this. There is a lot to more to gain from using a dedicated solution. Feel free to leave any input or concerns below 😅.

cc @nathantannar4 @zhongwuzw @cwalo @Sherlouk

[ndonald2]

Simplify existing cell view hierarchy

I'm not familiar with IGListKit and from a cursory read of the docs I'm not sure how it would help with the issue of cell view hierarchies. Not seeing anything in the APIs regarding view layout within cells. Could you please explain?

Greater flexibility for creating complex cells and cell structures

Same with this - it's not clear to me how IGListKit would help with dynamic sizing of collection view items. It'd be great to have a more detailed explanation of how it would be used to solve these problems.

[SD10]

Hey @ndonald2, good questions.

Simplify existing cell view hierarchy

If you reference the image above, creating a layout like that in MessageKit would require all those views to be in a single cell. With IGListKit, we would be turning each vertical component into its own cell.

Greater flexibility for creating complex cells and cell structures

We'd be splitting the layout up into to smaller components. This allows you to stack a bunch of different cell types and create new layouts via cell composition.

Same with this - it's not clear to me how IGListKit would help with dynamic sizing of collection view items.

It won't help directly with dynamic sizing. This will always be an issue with UICollectionView. However, the smaller cells will.

All text message cells currently have a messageLabel, cellTopLabel, and cellBottomLabel. The logic for the cell needs to consider the label sizes regardless of whether the user wants them in the cell or not (ie. only showing a bottom label on the last message in a group of messages). Moving them into their own cells allows the layout to focus solely on what will be visible.

[ndonald2]

@SD10

If you reference the image above, creating a layout like that in MessageKit would require all those views to be in a single cell. With IGListKit, we would be turning each vertical component into its own cell.

I get what you're saying, but is the type of layout you're referencing in the image (the IG post layout) really an example of something that MessageKit strives to eventually support? I was interested in MessageKit because it provides a relatively straightforward "conversation" UI abstraction (akin to iMessage) and IMO the image you've attached represents a _significantly_ different UI task and seems out of scope, unless I'm misunderstanding the goals of the project.

Regardless, the diffing and list management pieces of IGListKit do seem valuable as a way to minimize and simplify collection view reloading and updating.

[hyouuu]

Thanks for posting this up for discussion first! (Guess I'll hold on from adding the accessory view ;)

Have to spend time looking into IGListView and play with it to get the feeling, but from first glance it still have bugs open from 2016 (https://github.com/Instagram/IGListKit/issues/260), as well as many old bugs not dealt with yet, which is a bit concerning. Also FB probably will not consider updating their codebase to Swift in another 5 years, and that's just annoying :) Look at that "IG" prefix lol

For the problem "More features == more views. Not everyone wants all of the views in the cell and want to turn some of them off. This results in unnecessary sizing calculations.", have we thought about other approaches, e.g. provide bare bone MessageKit with more generic container views + extension libraries as needed?

[SD10]

@ndonald2 Thanks for the feedback 👍

... IMO the image you've attached represents a significantly different UI task and seems out of scope, unless I'm misunderstanding the goals of the project.

The goal was always to have easy to use defaults but also be highly customizable. When starting the project, Jesse Squires told us not to put the extra labels in the cell. Instead, he suggested we make them header and footer views. In hindsight, I can see why he made this recommendation, however, that architecture is really limiting on custom cells.

I was interested in MessageKit because it provides a relatively straightforward "conversation" UI abstraction (akin to iMessage)

I can appreciate that. This is all still under discussion so hopefully, we can all come about the right solution. You'll notice that even iMessage is using an architecture where the extra labels for "Read", "Sent" "Delivered" are not part of the cell. You can see this by swiping to the left on the bubbles. You'll see that the status labels stay in place.

[SD10]

@hyouuu

as well as many old bugs not dealt with yet, which is a bit concerning.

There isn't many bugs to be honest. A lot of those are trivial or in the example app. The biggest concern is they are having trouble for self sizing cells. I don't even think that MessageKit currently supports that. Self-sizing is always an issue even in the standard UIKit. It's important that we don't confuse dynamic cell sizes with self-sizing. MessageKit has dynamic cells, but we size them manually.

have we thought about other approaches, e.g. provide bare bone MessageKit with more generic container views + extension libraries as needed?

I'm open to any ideas you may have 😇 We certainly want to find a balance between having a robust set of features and yet not doing everything. The key is that the library should be easily extendable but not too simple as to merit not using it at all.

[SD10]

It really comes down to making cells simpler. The messageLabel, bottomLabel, and topLabel, is too much for one cell. You can see issue #519. Also related is scroll performance in #504. @ndonald2 should be familiar with that already.

JSQMVC struggled with a similar issue for years https://github.com/jessesquires/JSQMessagesViewController/issues/492

[ndonald2]

@SD10 I appreciate the detailed response.

The goal was always to have easy to use defaults but also be highly customizable.

I'm 100% with you there. My issue is more with using the instagram post example in the screenshot to drive the conversation; I don't think many people would look at that and think "I need a messaging UI library to recreate this". It seems like a wholly different use case.

It really comes down to making cells simpler. The messageLabel, bottomLabel, and topLabel, is too much for one cell.

This, as you've stated, seems to be the essential problem - it makes perfect sense to break extra labels, custom views, etc out of the cell. If IGListKit can help solve this problem without introducing too fragile or heavy-handed a dependency, then it seems worth considering. :+1:

[hyouuu]

So I just used Instagram app a bit more to pay attention to its collectionView in both my profile and explore tab, and when pull to refresh there are very bizarre glitches going on - have you guys experienced that? (maybe those parts are not using IGListView?)

[SD10]

@ndonald2 I can understand how the image above seems out of place. Maybe this will provide more context:

@hyouuu Maybe what you are seeing is the diffing 🤔 Every app has bugs but I don't think Instagram would be using this abstraction if they had significant issues. IGListKit gets used by hundreds of millions of Instagram users.

[Sherlouk]

I'll add my two cents given I've contributed to IGLK since it went open source so I know a little bit about it

First off I definitely wouldn't start introducing this before the nice Swift work Ryan has been working on, it feels like a lot of wasted effort to start that transition now when a lot of it will be changing!

MessageKit would definitely benefit from some sort of diffing to make insertion of new data a lot easier irrespective of where it is within the list.

IGLK does have a nice encapsulation with "SectionControllers" but I would be interested in what your thoughts are as to what a section controller is to MessageKit. It can obviously support multiple cells, but where do you draw the line?

You gave a brief image above which helps towards that, but I query where things like grouped messages come into this, how do you decide which one has the authors image, and where you can insert non-bubble content like timestamps, delivery information, and more

Instagram have and continue to use IGLK throughout their application so it is well tested and they're always using the master build which is designed to be stable. There's a pretty nice community around it and Ryan is still active at responding to questions though I am aware that as Instagram have no new requirements for IGLK that development has slowed down and is now primarily things contributors want to support and add. This is obviously speculation as we don't know what the plans are with the project moving forward - it does raise questions though if you had to, could you maintain both MessageKit and IGLK or quickly move away from it? (As is always the risk with any dependency!)

I also feel like IGLK isn't really something you "bolt on", it definitely pushes your data flow and structure in a certain way which is something that I'd argue MessageKit doesn't already match, so this definitely wouldn't be an insignificant change to essentially re-architect the same view.

You mentioned that you might like to make it a subspec, so people could choose to not use IGLK but this again goes against at least in my opinion of how IGLK works. You can't just bolt it on and get the benefits, you have to change the way you build things!

I also don't think that this will help with the performance issues you've been facing with a large number of views, but simply make them a bit easier to handle and with better patterns in place. You will still need to look into optimisations in other places, especially as you will likely still need a custom and high performance UICV layout. Let alone the height calculations of a given section controller, which will still need to concern itself with it's cells and their attributed strings.

Happy to expand if needed 😉

[SD10]

@Sherlouk Thank you for the thoughtful write up ❤️ 😈

MessageKit would definitely benefit from some sort of diffing to make insertion of new data a lot easier irrespective of where it is within the list.

Yeah, this isn't the core focus but it is a nice added benefit.

... but I query where things like grouped messages come into this, how do you decide which one has the authors image, and where you can insert non-bubble content like timestamps, delivery information, and more

We haven't found a good answer to this yet, to be honest. Every solution seems to be an array of models where non-bubble content is peppered in between messages. Open to suggestions 😖

I also feel like IGLK isn't really something you "bolt on", it definitely pushes your data flow and structure in a certain way which is something that I'd argue MessageKit doesn't already match, so this definitely wouldn't be an insignificant change to essentially re-architect the same view.

I agree, it is definitely not a trivial change in any sense.

I also don't think that this will help with the performance issues you've been facing with a large number of views, but simply make them a bit easier to handle and with better patterns in place.

Pretty much. It's not like IGListKit is a free lunch in terms of a performance boost. We ultimately need a better pattern for the cells, simpler view hierarchies. Focus on sizing a single attributed string at a time or even let some of them become static.

[Sherlouk]

We haven't found a good answer to this yet

Ultimately I think that's where we can't jump to conclusions then, this isn't something you can "just do" and hope for the best especially given what you've already built and decisions you've already made. I think you really do need to think about your different section controllers, what their responsibilities are, and what ins/outs they need!

Focus on sizing a single attributed string at a time or even let some of them become static.

Ultimately static content will have the same hit on performance whether it's in it's own section controller, or nested with other cells! It's just the adding values together! Ultimately though your nested hierarchy won't change too much, but more you'll get away from one cell having multiple responsibilities which is always bad code smell!

[hyouuu]

Great to have the 3rd most contributed person joining the discussion :D

[SD10]

@Sherlouk

Ultimately I think that's where we can't jump to conclusions then, this isn't something you can "just do" and hope for the best especially given what you've already built and decisions you've already made.

We're definitely not rushing into anything 😆 I learned that lesson once already

Ultimately static content will have the same hit on performance whether it's in it's own section controller, or nested with other cells! It's just the adding values together!

Static content is a lesser performance hit. If I know the height of a NSAttributedString is constant then I don't have to size it dynamically with the rect(for bounds) method. Calling that method is so expensive because it needs to consider all the attributes of the String. I'm trying to make this change regardless of using IGListKit 😕

[cwalo]

Late to the discussion, but some great points here. According to this IGListKit issue it sounds like it could make MessageKit a non-starter when used with some persistence frameworks/data architectures, namely Realm. It's a game of trade-offs, I suppose - trying to support everyone and everything, while being as performant as possible.

[jyounus]

I know this is a completely different suggestion, but would considering one of those "async UI libs" help here? If we could off-load calculations onto a background thread, that would have a positive performance impact and thus help with the scrolling/"heavy cells" issue.

The two big libs that come into mind are:
-From Facebook/Pintrest: https://texturegroup.org/
-From LinkedIn: http://layoutkit.org/

Not sure, just thought I'd throw this out here. Maybe this could be useful and solve some problems. I personally haven't used these libs before but I'm sure someone here could add their input with any experience they might have.

EDIT
eBay have previously open sourced a chat UI written using ASDK (which is now called Texture): https://github.com/eBay/NMessenger
Unfortunately it doesn't seem to be very well maintained.

[SD10]

@jyounus Yeah I've thought about it too, but IMO using those are far less safe in terms of a dependency than IGListKit. I have a feeling those will stop being maintained. I'm also kind of hesitant to "cover up" the problems

[SD10]

We don't need to do this anymore. After the changes from #580, #579 most of the problems will be resolved and we can have better opt-in IGListKit support.