Add support for emoji-only messages by SirArkimedes · Pull Request #222 · MessageKit/MessageKit

SirArkimedes · 2017-10-03T22:35:27Z

Adds a new message type that handles text that doubles the font size and removes the message background to model how iMessage displays emoji-only messages.

Issue: #182

Previews of how they look:

SirArkimedes · 2017-10-03T22:40:45Z

I do have a question, though. Should I force the "text" MessageData to use this "emoji" MessageData or should I leave it up to the user?

I know the issue talks about parsing, but was unsure if I should parse the message at all.

SD10 · 2017-10-03T22:53:24Z

@SirArkimedes This is a good start!! 👍 I was hoping we didn't have to create another subclass of MessagesCollectionViewCell and could just use TextMessageCell (I'm trying to decrease the number of different cell types we have).

I'll be able to review this a little more in-depth later.

About parsing, nothing to do there. It should be the user's responsibility to determine if a message contains only emojis or not.

cc @cwalo for some extra eyes

fbartho · 2017-10-03T23:01:14Z

Re: parsing, you could add a utility method that trivially detects emoji (minus spaces), and add an example in documentation of how to use wire this together to return the right message type.

Adds a new message type that handles text that doubles the font size and removes the message background to model how iMessage displays emoji-only messages.

eliburke · 2017-10-04T00:02:45Z

If you don't want to embed emoji detection code, I'd suggest a comment pointing users at:
https://stackoverflow.com/a/39425959/1758224

Using these two extensions, the appropriate logic is:

if messageText.containsOnlyEmoji, messageText.glyphCount < 5 {
    // do the thing
}

SD10 · 2017-10-04T00:22:48Z

@fbartho @eliburke Both great points, thank you!!

cwalo · 2017-10-04T00:55:12Z

Seconded @eliburke. Here's a gist with our extension (mostly lifted from the referenced SO answer). I'll check out what's been done so far.

Removes EmojiTextCell and takes what it used to doand adds it to TextMessageCell.

SirArkimedes · 2017-10-04T02:20:11Z

I can understand that adding a comment to make it clear how to use the emoji type, but where would I put that comment?

SD10 · 2017-10-04T03:47:30Z

@SirArkimedes This would be reflected in external docs. Don't worry about merging v0.9.0 too much. It's changing quickly. We'll rebase when I review this

SD10 · 2017-10-04T06:32:36Z

    // MARK: - Properties

    open var messageLabelFont: UIFont
+    open var emojiLabelFont: UIFont


I think this should be private if we're going to override it by doubling messageLabelFont

SD10 · 2017-10-04T06:33:32Z

@@ -30,6 +30,8 @@ open class MessagesCollectionViewFlowLayout: UICollectionViewFlowLayout {
    // MARK: - Properties

    open var messageLabelFont: UIFont


This probably needs a didSet to reflect the changes in emojiLabelFont

SD10 · 2017-10-04T06:34:54Z

-        case .text, .attributedText:
-            let cell = messagesCollectionView.dequeueReusableCell(TextMessageCell.self, for: indexPath)
+        case .text, .attributedText, .emoji:
+            let cell = collectionView.dequeueReusableCell(TextMessageCell.self, for: indexPath)


this should be messagesCollectionView but maybe its because this branch is not current with v0.9.0

SD10 · 2017-10-04T06:37:00Z

        case .attributedText(let text):
            messageContentView.attributedText = text
+        case .emoji(let text):
+            messageContainerView.backgroundColor = .clear


Yeah, I was thinking how to handle the style. Maybe we shouldn't set the color to .clear here and expect the user to apply a style of .none to the .emoji case.

I was originally thinking the same thing, by just setting the style to .none here. But the MessageCollectionViewCell sets the background color from the delegate, which creates just a colored rectangle behind the message.

Tough one. Can we just do this in the delegate then? You'll have to do an if case let or switch on the MessageType. If it is emoji return a .clear background color. Until we find a better way to handle it.

Moved to the delegate.

Also adds a didSet to the main font, to set the emoji’s font to double the set font.

SD10 · 2017-10-04T22:12:15Z

@SirArkimedes This looks good to me! We need to add a CHANGELOG entry under Added. This would be a breaking change notifying the users that we added a new .emoji(String) case to the MessageData type.

Please take care to follow the existing CHANGELOG in v0.9.0. We also have a CHANGELOG_GUIDELINES file in that PR

Adds .emoji(String) breaking change to CHANGELONG.md.

SirArkimedes · 2017-10-04T22:56:25Z

Done!

SD10

@SirArkimedes Thank you for your contribution to MessageKit 💯 🎉 I've invited you to join the MessageKit organization. No pressure if you aren't feeling up to it 😎

Add support for emoji-only messages

ed1d1e5

Adds a new message type that handles text that doubles the font size and removes the message background to model how iMessage displays emoji-only messages.

SirArkimedes force-pushed the v0.9.0 branch from 3d659a4 to ed1d1e5 Compare October 3, 2017 23:10

Remove EmojiCell and consolidate into TextCell

dd7c831

Removes EmojiTextCell and takes what it used to doand adds it to TextMessageCell.

SirArkimedes added 2 commits October 3, 2017 21:25

Merge branch 'v0.9.0' into v0.9.0

afa93c1

Merge branch 'v0.9.0' into v0.9.0

0ba2c7d

SD10 reviewed Oct 4, 2017

View reviewed changes

SirArkimedes added 2 commits October 4, 2017 10:16

Merge branch 'v0.9.0' into v0.9.0

11aa695

Move background color set to delegate

0f00bd3

Also adds a didSet to the main font, to set the emoji’s font to double the set font.

Add .emoji(String) to CHANGELONG.md

96733e1

Adds .emoji(String) breaking change to CHANGELONG.md.

SD10 added under review ready for merge and removed under review labels Oct 4, 2017

SD10 approved these changes Oct 4, 2017

View reviewed changes

SD10 merged commit c55db41 into MessageKit:v0.9.0 Oct 4, 2017

This was referenced Oct 4, 2017

Add emoji message type #182

Closed

v0.9.0 #157

Merged

		@@ -30,6 +30,8 @@ open class MessagesCollectionViewFlowLayout: UICollectionViewFlowLayout {
		// MARK: - Properties

		open var messageLabelFont: UIFont

Conversation

SirArkimedes commented Oct 3, 2017

Uh oh!

SirArkimedes commented Oct 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

fbartho commented Oct 3, 2017

Uh oh!

eliburke commented Oct 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SD10 commented Oct 4, 2017

Uh oh!

cwalo commented Oct 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SirArkimedes commented Oct 4, 2017

Uh oh!

SD10 commented Oct 4, 2017

Uh oh!

SD10 Oct 4, 2017

Choose a reason for hiding this comment

Uh oh!

SD10 Oct 4, 2017

Choose a reason for hiding this comment

Uh oh!

SD10 Oct 4, 2017

Choose a reason for hiding this comment

Uh oh!

SD10 Oct 4, 2017

Choose a reason for hiding this comment

Uh oh!

SirArkimedes Oct 4, 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 Oct 4, 2017

Choose a reason for hiding this comment

Uh oh!

SirArkimedes Oct 4, 2017

Choose a reason for hiding this comment

Uh oh!

SD10 commented Oct 4, 2017

Uh oh!

SirArkimedes commented Oct 4, 2017

Uh oh!

SD10 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SirArkimedes commented Oct 3, 2017 •

edited

Loading

SD10 commented Oct 3, 2017 •

edited

Loading

eliburke commented Oct 4, 2017 •

edited

Loading

cwalo commented Oct 4, 2017 •

edited

Loading

SirArkimedes Oct 4, 2017 •

edited

Loading