v0.1.0 by SD10 · Pull Request #5 · MessageKit/MessageKit

SD10 · 2017-07-22T19:29:09Z

No description provided.

SD10 · 2017-07-22T19:31:20Z

Sources/MessageBubble.swift

+    public let highlightedImage: UIImage
+
+    public func image(highlighted: Bool) -> UIImage {
+        return highlighted ? image : highlightedImage


This will probably be removed/repurposed considering we most likely won't use images for bubbles. The return values are flipped though 😛

So why don't we remove this now? I feel like this a key point in this projects development where we have an opportunity to remove any cruft.

codecov-io · 2017-07-22T19:37:51Z

Codecov Report

❗ No coverage uploaded for pull request base (master@bbc9884). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             master     #5   +/-   ##
=======================================
  Coverage          ?   100%           
=======================================
  Files             ?      1           
  Lines             ?      7           
  Branches          ?      0           
=======================================
  Hits              ?      7           
  Misses            ?      0           
  Partials          ?      0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbc9884...32265a4. Read the comment docs.

bojanstef · 2017-07-23T02:39:05Z

Sources/MessageData.swift

+
+    case video(file: NSURL, thumbnail: UIImage)
+
+    case system(String)


What is MessageData.system? or rather; what is the difference with .text and .system

I think a little code comment will be helpful for people to understand.

I'm actually not sure because this is code leftover from the old repo. The only thing to be concerned about for the moment is MessageData.text. v1 should be basic messages and then expand to Media data types from there.

@towry eventually the code will be fully commented by v1. right now I'm just hacking things together 😅

IMO I would suggest removing anything that does not make any since. I would rather not have things then to start out with bloat right off the bat. But that might just be me.

SD10 · 2017-07-23T17:04:52Z

Some things are shaping up but the code is still very rough. A lot is subject to change. I'm struggling over using a UICollectionViewController opposed to a regular UIViewController. I'd like my collectionView to be typed as MessagesCollectionView.

Type casting everywhere 😭

MacMeDan

Remove anything that is not understood or that is not fully implemented.

MacMeDan · 2017-07-23T21:08:00Z

Sources/MessageBubble.swift

+    public let highlightedImage: UIImage
+
+    public func image(highlighted: Bool) -> UIImage {
+        return highlighted ? image : highlightedImage


So why don't we remove this now? I feel like this a key point in this projects development where we have an opportunity to remove any cruft.

MacMeDan · 2017-07-23T21:19:27Z

Sources/MessageType.swift

+
+    var sentDate: Date { get }
+
+    var data: MessageData { get }


I don't know why we would not have the sentDate as a part of the messageData. Can we think of a reason we would want this?

I also get confused with naming. Here "MessageData" is the enum of different message category, while the protocol is holding the message content. It's better to name the protocol as "MessageData" and the enum as "MessageType".

I had similar concerns. I'm considering:
MessageType -> MessageRepresentable
MessageData -> MessageType

SD10 · 2017-07-23T22:54:33Z

v0.1.0 Features:

Message Bubbles Incoming/Outgoing
Message Avatars
Basic Text Messages Support
Resizing Messages Based on Text Length
Input Bar With Send Button
Move Input Bar When Showing Keyboard

ArtSabintsev · 2017-07-24T00:49:14Z

Sources/MessageCollectionViewCell.swift

+
+    override func apply(_ layoutAttributes: UICollectionViewLayoutAttributes) {
+        guard let attributes = layoutAttributes as? MessagesCollectionViewLayoutAttributes else { return }
+        print(attributes.avatarSize)


For print statements, you should probably forward them to a print statement that only works in DEBUG mode.

@ArtSabintsev I left those by accident 👍 Thanks. I would never allow them in a release

I guess the print statements is temporary here, I would add a comment to such thing as to be removed. Even in debug mode, other people may not want to see all kind print messages in the console. But it is good to decorate print statement in a DEBUG util in case the code being merged to master branch or we can use swift lint tool to check that.

All the code will be cleaned up before any merge to master. Having this PR open is just to show the public that there is work being done.

towry · 2017-07-25T00:06:28Z

Sources/MessagesCollectionViewLayoutAttributes.swift

+        return attributes.messageFont == messageFont && attributes.avatarSize == avatarSize &&
+            attributes.avatarToEdgePadding == avatarToEdgePadding && attributes.avatarBottomPadding == avatarBottomPadding &&
+            attributes.avatarToContainerPadding == avatarToContainerPadding && attributes.messageLeftRightPadding == messageLeftRightPadding &&
+            attributes.direction == direction


Better formatting.

return attributes.messageFont == messageFont && attributes.avatarSize == avatarSize && attributes.avatarToEdgePadding == avatarToEdgePadding && attributes.avatarBottomPadding == avatarBottomPadding && attributes.avatarToContainerPadding == avatarToContainerPadding && attributes.messageLeftRightPadding == messageLeftRightPadding && attributes.direction == direction

lzyraining · 2017-07-25T04:03:39Z

Sources/MessageData.swift

+
+    case photo(UIImage)
+
+    case video(file: NSURL, thumbnail: UIImage)


Will the video type apply a UIButton to as its internal property? Or is it autoplay when video is sent?

These aren't commented out yet but v1 will only support basic types like text(String). More details to come when I write up a Vision.md

lzyraining · 2017-07-25T04:22:00Z

Sources/MessageCollectionViewCell.swift

+            let containerX = contentView.frame.width - attributes.messageLeftRightPadding - containerWidth
+            let containerY: CGFloat = 0
+            messageContainerView.frame = CGRect(x: containerX, y: containerY, width: containerWidth, height: containerHeight)
+        }


The logic from the AvatarSizeCalculator is blocking incoming avatar. And there is no difference between incoming and outgoing case. Recommend to add a todo list to remove this constraint.

It's half implemented, not done yet. This branch is really outdated.

nunogoncalves · 2017-07-25T22:14:15Z

Sources/MessagesViewController.swift

+    func setupConstraints() {
+
+        messagesCollectionView.translatesAutoresizingMaskIntoConstraints = false
+        view.addConstraint(NSLayoutConstraint(item: messagesCollectionView, attribute: .top, relatedBy: .equal, toItem: topLayoutGuide, attribute: .bottom, multiplier: 1, constant: 0))


Sorry, just here sniffing around.
Why not using anchors? Minimum targets?

Yes, minimum targets. I struggled with the decision to support iOS 8. I figured why not unless it becomes a hassle.

nunogoncalves · 2017-07-25T22:19:45Z

Sources/AvatarSizeCalculator.swift

+
+class AvatarSizeCalculator {
+
+    func avatarSizeFor(messageType: MessageType, with layout: MessagesCollectionViewFlowLayout) -> CGSize {


If I may,
func avatarSize(for messageType: MessageType, with layout: MessagesCollectionViewFlowLayout) would make the calling site read better. :)

nunogoncalves · 2017-07-25T22:25:22Z

Sources/MessageContainerSizeCalculator.swift

+
+fileprivate extension String {
+
+    func height(considering width: CGFloat, font: UIFont) -> CGFloat {


Just another tiny suggestion:

func height(considering width: CGFloat, and font: UIFont)

would produce a calling site reading experience a bit smoother.

height(considering: width, and: UIFont.systemFont...)

Also, this is nice for views with number of lines = 0. If you want to have a label with max number of lines = 2 for example, this won't do it as it will only consider the text. That way I would prefer the sizeThatFits from the label.

SD10 · 2017-07-26T00:41:53Z

Hi @ everyone, first and foremost sorry for the delay! I ran into a nasty bug that ate up most of my time.

I'm sure that most of you have gotten bored looking for some way to contribute, but let's not take this PR too seriously. It was primarily to show people this repo is active.

It's a WIP so you may see some hacky code or half implemented solutions. It doesn't mean I have the intention of leaving them this way 😅

lzyraining · 2017-07-27T05:47:53Z

Example/Sources/ConversationViewController.swift

+
+    func avatarForMessage(_ message: MessageType, at indexPath: IndexPath, in collectionView: UICollectionView) -> Avatar {
+        let image = isFromCurrentSender(message: message) ? #imageLiteral(resourceName: "Steve-Jobs") : #imageLiteral(resourceName: "Tim-Cook")
+        return Avatar(placeholderImage: image)


Why not let Sender own the Avatar? To make the API more user friendly. We don't need to map the avatar to message manually.

func avatarForMessage(_ message: MessageType, at indexPath: IndexPath, in collectionView: UICollectionView) -> Avatar { return message.sender.avatar }

Hey @lzyraining thanks for the feedback!

Deciding how to expose an API that allows for easy customization will be a difficult task. I hope to release a really basic v0.1.0 beta in the next few hours and will be looking for feedback/suggestions.

As for your suggestion, I don't think Sender is a good fit for the responsibility of Avatar because some people may not even want Avatars. I'd like to keep this types responsibility minimal. I've even considered making it a protocol.

@SD10 I agree that it's difficult to build easy customized API. Thanks for the optional type in swift, we can simple handle who don't want Avatars by putting a question mark.

I like the idea using protocol for some models. Swift is basically protocol oriented programming language. It's amazing that protocol allows inheritance and extension to make customize and default values easy. But my concern is that we are build a framework for all potential use. From a user's perspective, I don't want to build all the models which are declared as protocol in the library. I just want to it easy to use.

@lzyraining Thanks, I had considered making Avatar optional here as well. It will take a bit of time to decide on a nice API. The great thing is we don't have to worry about API stability at this moment 😃

Thanks for raising the concerns of having to build the models required by protocols. I was originally thinking that people would probably already have a User model that they could conform Sender to -- but I see this could also be difficult and intrusive.

Solves issue #546 ( https://github.com/MessageKit/MessageKit/issues/5…

Let cell support diary quote message type

SD10 added 2 commits July 22, 2017 14:01

Update file headers and organize project

1e4671c

Add MessagesCollectionView

f1d7b0a

SD10 commented Jul 22, 2017

View reviewed changes

SD10 added 3 commits July 22, 2017 14:46

Fix return value logic

e14bcb6

Setup MessagesViewController constraints

59f0b81

Setup basic MessageInputBar with constraints

7c4eed8

bojanstef reviewed Jul 23, 2017

View reviewed changes

SD10 added 4 commits July 23, 2017 00:52

Add basic MessagesCollectionViewFlowLayout for height resizing cells

a185063

Update LICENSE.md

0132ae5

Add MessagesCollectionViewLayoutAttributes (rough draft)

83a0832

Round corner edges for demo

058b4fd

MacMeDan reviewed Jul 23, 2017

View reviewed changes

ArtSabintsev reviewed Jul 24, 2017

View reviewed changes

towry reviewed Jul 25, 2017

View reviewed changes

lzyraining reviewed Jul 25, 2017

View reviewed changes

nunogoncalves reviewed Jul 25, 2017

View reviewed changes

SD10 added 5 commits July 25, 2017 22:20

In progress

c5ad5cb

Fix layout bug & cleanup code

383002d

Remove test colors start to clean up UI

0d11179

Add MessageInputBar as inputAccessoryView

f1eeb6c

In progress

9ffc10c

lzyraining reviewed Jul 27, 2017

View reviewed changes

Move collectionview bottom constraint when keyboard shows/hides

6cf5056

SD10 added 2 commits July 27, 2017 02:00

Clean up access control

cb90c7f

Update Podspec and clean up code

db2e929

SD10 changed the title ~~[WIP] v0.1.0~~ v0.1.0 Jul 27, 2017

Update MessageKit.podspec

32265a4

SD10 merged commit 7efca7b into master Jul 27, 2017

SD10 deleted the v0.1.0 branch August 1, 2017 05:04

BasThomas referenced this pull request Mar 6, 2018

Merge pull request #551 from Sepho/development

f48315c

Solves issue #546 ( https://github.com/MessageKit/MessageKit/issues/5…

result0924 pushed a commit to result0924/MessageKit that referenced this pull request Oct 5, 2023

Merge pull request MessageKit#5 from Health2Sync/main-justin

9f9e65e

Let cell support diary quote message type


		case video(file: NSURL, thumbnail: UIImage)

		case system(String)


		case photo(UIImage)

		case video(file: NSURL, thumbnail: UIImage)


		class AvatarSizeCalculator {

		func avatarSizeFor(messageType: MessageType, with layout: MessagesCollectionViewFlowLayout) -> CGSize {


		fileprivate extension String {

		func height(considering width: CGFloat, font: UIFont) -> CGFloat {

Conversation

SD10 commented Jul 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Jul 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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 Jul 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MacMeDan left a comment

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 Jul 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

lzyraining Jul 25, 2017 • 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!

nunogoncalves Jul 25, 2017 • 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!

nunogoncalves Jul 25, 2017 • 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!

SD10 commented Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Reviewers

codecov-io commented Jul 22, 2017 •

edited

Loading

SD10 commented Jul 23, 2017 •

edited

Loading

SD10 commented Jul 23, 2017 •

edited

Loading

lzyraining Jul 25, 2017 •

edited

Loading

nunogoncalves Jul 25, 2017 •

edited

Loading

nunogoncalves Jul 25, 2017 •

edited

Loading

SD10 commented Jul 26, 2017 •

edited

Loading