Open up MessagesCollectionViewFlowLayout cell sizing API by SD10 · Pull Request #491 · MessageKit/MessageKit

SD10 · 2018-01-28T19:26:52Z

Summary of Pull Request:

Resolves: #489, #487, #453, #396, #331, #234

Moves MessageCollectionViewCell subview frame calculations to methods of the cell Move all cell subview origin calculations into MessagesCollectionViewCell #487

open func layoutAvatarView(with attributes: MessagesCollectionViewLayoutAttributes)
open func layoutMessageContainerView(with attributes: MessagesCollectionViewLayoutAttributes)
open func layoutTopLabel(with attributes: MessagesCollectionViewLayoutAttributes)
open func layoutBottomLabel(with attributes: MessagesCollectionViewLayoutAttributes)

This allows users to control the layout of subviews by an override to one of these methods. Theoretically, they could override and make the functions a no-op, handling layout with something like autolayout.

Opens up cell sizing API

// MARK: - MessageContainerView
open func messageLabelInsets(for message: MessageType, at indexPath: IndexPath) -> UIEdgeInsets
open func messageContainerPadding(for message: MessageType, at indexPath: IndexPath) -> UIEdgeInsets
open func messageContainerMaxWidth(for message: MessageType, at indexPath: IndexPath) -> CGFloat 
open func messageContainerSize(for message: MessageType, at indexPath: IndexPath) -> CGSize

// MARK: - Cell Top Label
open func cellTopLabelAlignment(for message: MessageType, at indexPath: IndexPath) -> LabelAlignment
open func cellTopLabelSize(for message: MessageType, at indexPath: IndexPath) -> CGSize
open func cellTopLabelMaxWidth(for message: MessageType, at indexPath: IndexPath) -> CGFloat

// MARK: - Cell Bottom Label
open func cellBottomLabelAlignment(for message: MessageType, at indexPath: IndexPath) -> LabelAlignment
open func cellBottomLabelSize(for message: MessageType, at indexPath: IndexPath) -> CGSize
open func cellBottomLabelMaxWidth(for message: MessageType, at indexPath: IndexPath) -> CGFloat

// MARK: - AvatarView
open func avatarPosition(for message: MessageType, at indexPath: IndexPath) -> AvatarPosition
open func avatarSize(for message: MessageType, at indexPath: IndexPath) -> CGSize

// MARK: - Cell Size
open func cellContentHeight(for message: MessageType, at indexPath: IndexPath) -> CGFloat
open func sizeForItem(at indexPath: IndexPath) -> CGSize

This also allows the MessagesCollectionViewFlowLayout to become more static. While a lot of these open methods will call into MessagesLayoutDelegate to retrieve the values, you can override the method that wraps the delegate call and return a static value instead.

Replaces MessageIntermediateLayoutAttributes with MessageCellLayoutContext

This is a lighter weight type and caches less information #489. We no longer hold a reference to any IndexPath or MessageType. We also no longer cache cell subview origins since the layout is now handled by the MessageCollectionViewCell. This fixes the annoying frame caching of cellTopLabel and cellBottomLabel in #234 and makes cell attribute caching much more useful again.

I expect that I messed at least one thing up with these changes. It will need some review. However, this API should be far easier to test now considering most things have been changed from private to internal or open.

TODO:

CHANGELOG
Fix tests

Release 0.13.1

SD10 · 2018-01-28T19:39:05Z

Sources/Layout/MessagesCollectionViewFlowLayout+Labels.swift

+    /// - Parameters:
+    ///   - attributedText: The `NSAttributedString` used to calculate a size that fits.
+    ///   - maxWidth: The max width available for the label.
+    internal func labelSize(for attributedText: NSAttributedString, considering maxWidth: CGFloat) -> CGSize {


These may be able to be combined into a single method for NSAttributedString

SD10 · 2018-01-28T19:40:02Z

Sources/Layout/MessagesCollectionViewLayoutAttributes.swift


-    var avatarFrame: CGRect = .zero
+    var avatarSize: CGSize = .zero
+    var avatarPosition = AvatarPosition(vertical: .cellBottom)


It would be better to maybe use IUO's here

SD10 · 2018-01-28T19:40:31Z

Sources/Views/Cells/MessageCollectionViewCell.swift

-            messageContainerView.frame = attributes.messageContainerFrame
-        }
+        guard let attributes = layoutAttributes as? MessagesCollectionViewLayoutAttributes else { return }
+        layoutMessageContainerView(with: attributes)


Add comment that this has to be called first

SD10 · 2018-01-28T19:40:42Z

Sources/Views/Cells/MessageCollectionViewCell.swift

+
+    // MARK: - Origin Calculations
+
+    func layoutAvatarView(with attributes: MessagesCollectionViewLayoutAttributes) {


SD10 · 2018-01-28T19:40:51Z

Sources/Views/Cells/MessageCollectionViewCell.swift

+        avatarView.frame = CGRect(origin: origin, size: attributes.avatarSize)
+    }
+
+    func layoutMessageContainerView(with attributes: MessagesCollectionViewLayoutAttributes) {


SD10 · 2018-01-28T19:40:56Z

Sources/Views/Cells/MessageCollectionViewCell.swift

+        messageContainerView.frame = CGRect(origin: origin, size: attributes.messageContainerSize)
+    }
+
+    func layoutTopLabel(with attributes: MessagesCollectionViewLayoutAttributes) {


SD10 · 2018-01-28T19:41:02Z

Sources/Views/Cells/MessageCollectionViewCell.swift

+        cellTopLabel.frame = CGRect(origin: origin, size: topLabelSize)
+    }
+
+    func layoutBottomLabel(with attributes: MessagesCollectionViewLayoutAttributes) {


SD10 · 2018-01-28T20:18:40Z

It's possible that none of these need to be open. They are already configured using the delegate. Having two would just cause confusion. I'll see if they have any benefits or can be removed.

open func avatarPosition(for message: MessageType, at indexPath: IndexPath) -> AvatarPosition
open func cellTopLabelAlignment(for message: MessageType, at indexPath: IndexPath) -> LabelAlignment
open func cellBottomLabelAlignment(for message: MessageType, at indexPath: IndexPath) -> LabelAlignment
open func messageLabelInsets(for message: MessageType, at indexPath: IndexPath) -> UIEdgeInsets
open func messageContainerPadding(for message: MessageType, at indexPath: IndexPath) -> UIEdgeInsets

nathantannar4 · 2018-01-28T21:52:28Z

Sources/Layout/MessageIntermediateLayoutAttributes.swift

-
-/// A intermediate context used to store recently calculated values used by
-/// the `MessagesCollectionViewFlowLayout` object to reduce redundant calculations.
-final class MessageIntermediateLayoutAttributes {


nathantannar4 · 2018-01-28T21:53:45Z

Sources/Layout/MessagesCollectionViewFlowLayout+Labels.swift

+        let avatarVertical = avatarPosition.vertical
+
+        switch (labelAlignment, avatarHorizontal) {
+


nathantannar4 · 2018-01-28T21:55:37Z

Sources/Views/Cells/MessageCollectionViewCell.swift

+    open func layoutMessageContainerView(with attributes: MessagesCollectionViewLayoutAttributes) {
+        guard attributes.messageContainerSize != .zero else { return }
+
+        var origin: CGPoint = .zero


var origin = CGPoint.zero for consistency

I think I'll flip flop them. I like explicitly typed better 👍

nathantannar4 · 2018-01-28T21:56:00Z

Sources/Views/Cells/MessageCollectionViewCell.swift

+        let bottomLabelPadding = bottomLabelAlignment.insets
+        let bottomLabelSize = attributes.bottomLabelSize
+
+        var origin: CGPoint = .zero


var origin = CGPoint.zero for consistency

zhongwuzw · 2018-01-29T06:34:06Z

Sources/Layout/MessagesCollectionViewFlowLayout.swift

-        }
-        return messagesCollectionView
-    }
+    fileprivate var layoutContextCache: [MessageID: MessageCellLayoutContext] = [:]


I prefer to choose NSCache , it can control the number of object in memory.

No layoutContextCache empty operation when prepareLayout?

Do you think it's necessary? I don't even use the prepareLayout method 🤔

@SD10 , I think it's necessary, dictionary has two weakness, one is occupy memory, another is performance penalty , the more number of data, the more time consumption when get object because of hash conflict.

Yep, we don't use prepareLayout, I mean we should override it that prepare something like empty cache when layout needs recalculation.

I currently empty the cache inside of here:

MessageKit/Sources/Layout/MessagesCollectionViewFlowLayout.swift

Line 118 in 2493217

open override func shouldInvalidateLayout(forBoundsChange newBounds: CGRect) -> Bool {

I'm fine with changing this to a more efficient cache 👍

SD10 · 2018-01-31T05:57:24Z

Sources/Extensions/UIEdgeInsets+Extensions.swift

@@ -0,0 +1,21 @@
+//
+//  UIEdgeInsets+Extensions.swift
+//  MessageKit


Replace with LICENSE

SD10 added 4 commits January 26, 2018 10:11

Merge pull request #486 from MessageKit/development

5f4fdd8

Release 0.13.1

Move origin calculations to MessageCollectionViewCell

e1bc35d

MessageIntermediateLayoutAttributes -> MessageCellLayoutContext

7239d49

Open up custom cell API and split into files

e6b08ea

SD10 changed the base branch from master to development January 28, 2018 19:26

SD10 requested review from cwalo, nathantannar4 and zhongwuzw January 28, 2018 19:27

SD10 added the in progress label Jan 28, 2018

SD10 added this to the 0.14.0 milestone Jan 28, 2018

SD10 commented Jan 28, 2018

View reviewed changes

SD10 added 3 commits January 28, 2018 14:28

Gardening + fix failing tests

4e123c3

Open cell layout methods

2493217

Fix small comment

8699a6b

nathantannar4 reviewed Jan 28, 2018

View reviewed changes

zhongwuzw reviewed Jan 29, 2018

View reviewed changes

This was referenced Jan 29, 2018

Use NSCache instead of Swift dictionary to cache attributes #495

Closed

MessageKit v1.0 Requirements #456

Closed

How to add event message other than bubble? #463

Closed

Replace dictionary with NSCache #495

a8a96d6

SD10 commented Jan 31, 2018

View reviewed changes

SD10 added a commit that referenced this pull request Jan 31, 2018

Add CHANGELOG entry for #491

40eb589

Add CHANGELOG entry for #491

f77100c

SD10 force-pushed the enhancement/custom-cell branch from 40eb589 to f77100c Compare January 31, 2018 06:09

Gardening + add in source docs

b4051be

SD10 added under review and removed in progress labels Jan 31, 2018

SD10 merged commit 1d660b0 into development Jan 31, 2018

SD10 deleted the enhancement/custom-cell branch January 31, 2018 07:23

This was referenced Jan 31, 2018

Move all cell subview origin calculations into MessagesCollectionViewCell #487

Closed

Enable creating custom cell types #453

Closed

SD10 removed this from the 0.14.0 milestone Mar 28, 2018


		// MARK: - Origin Calculations

		func layoutAvatarView(with attributes: MessagesCollectionViewLayoutAttributes) {

		let avatarVertical = avatarPosition.vertical

		switch (labelAlignment, avatarHorizontal) {

Comments

Conversation

SD10 commented Jan 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Pull Request:

TODO:

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!

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!

Choose a reason for hiding this comment

Uh oh!

SD10 commented Jan 28, 2018

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhongwuzw Jan 29, 2018 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SD10 commented Jan 28, 2018 •

edited

Loading

zhongwuzw Jan 29, 2018 •

edited

Loading