Skip to content
This repository was archived by the owner on Oct 30, 2018. It is now read-only.

Center right button and update constraints when its size is changed #186

Merged
merged 2 commits into from
Aug 7, 2015

Conversation

jmreyes
Copy link
Contributor

@jmreyes jmreyes commented May 7, 2015

We noticed, just like described in #57, that increasing the font size for the right button didn't update the already set constraints. That had the following effect in our case:

ios 8 3 12f69 2015-05-07 12-01-31

This fix uses KVO as proposed in the closed issue, but please feel free to give feedback.

Thanks a lot for this great library!

@@ -535,11 +538,8 @@ - (void)slk_didChangeTextViewContentSize:(NSNotification *)notification

- (void)slk_setupViewConstraints
{
UIImage *leftButtonImg = [self.leftButton imageForState:UIControlStateNormal];

Copy link

Choose a reason for hiding this comment

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

Why remove these?
This is still necessary to detect and adjust the left button.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are they needed there? The leftVerMargin generated from the UIImage size is not used when the constraints are set. It looks like the real adjustment is made in slk_updateConstraintConstants. Not sure, this was something I noticed while implementing the proper fix, not really needed for it to work.

Copy link

Choose a reason for hiding this comment

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

You're right. It's no longer used here anymore.

dzenbot pushed a commit that referenced this pull request Aug 7, 2015
Center right button and update constraints when its size is changed
@dzenbot dzenbot merged commit d2ca89d into slackhq:master Aug 7, 2015
@dzenbot
Copy link

dzenbot commented Aug 7, 2015

Sorry for taking so much time to merge this.
Works great and is backwards compatible. Thanks @jmreyes! 🙌

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants