Skip to content

fix intrinsic content size for MessageLabel#889

Merged
nathantannar4 merged 2 commits intoMessageKit:developmentfrom
monzo:bugfix/messagelabel-autolayout
Oct 5, 2018
Merged

fix intrinsic content size for MessageLabel#889
nathantannar4 merged 2 commits intoMessageKit:developmentfrom
monzo:bugfix/messagelabel-autolayout

Conversation

@marius-serban
Copy link
Contributor

@marius-serban marius-serban commented Oct 2, 2018

What does this implement/fix? Explain your changes.

The intrinsicContentSize of MessageLabel isn't correct because it doesn't take the textInsets property into account; it just relies on the UILabel implementation. This is a problem when using MessageLabels in custom cells that use autolayout.

Does this close any currently open issues?

nope

Any relevant logs, error output, etc?

Any other comments?

Where has this been tested?

Devices/Simulators:

iOS Version:

Swift Version:

MessageKit Version:

Copy link
Member

@SD10 SD10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good for me 👍 @nathantannar4 did you ever look into CI failing? I noticed you were playing with it in your Swift 4.2 PR. Think this is fine to merge without it passing though.

@nathantannar4
Copy link
Member

@SD10 ya this should build fine on Swift 4.2.

@marius-serban please change the destination branch and add a CHANGELOG entry for upcoming release then it LGTM 👍

@SD10 SD10 changed the base branch from master to development October 4, 2018 22:48
@SD10
Copy link
Member

SD10 commented Oct 4, 2018

I changed this to point at development and git isn't complaining nor are there excess commits. We just need a CHANGELOG entry, probably under bugfix?

@nathantannar4 nathantannar4 merged commit fa08526 into MessageKit:development Oct 5, 2018
@SD10
Copy link
Member

SD10 commented Oct 5, 2018

Thank you for contributing to MessageKit! I've invited you to join the MessageKit GitHub organization - no pressure to accept! If you'd like more information on what that means, check out our contributing guidelines and join the MessageKit Slack channel. Feel free to reach out if you have any questions! 😃

@SD10
Copy link
Member

SD10 commented Oct 6, 2018

@nathantannar4 You didn't check the CHANGELOG format before merging this 😓

@nathantannar4
Copy link
Member

I did, what wasn't right about the format?

Sent with GitHawk

@SD10
Copy link
Member

SD10 commented Oct 6, 2018

It's not in the CHANGELOG.md file. It's a new file 😂

@nathantannar4
Copy link
Member

Oh 😂 I thought it was the git diff just showing the difference. I'll fix it when I merge dev into master

Sent with GitHawk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments