Skip to content
This repository has been archived by the owner on Aug 14, 2019. It is now read-only.

Bug Fix #1449 #1472

Closed
wants to merge 6 commits into from
Closed

Bug Fix #1449 #1472

wants to merge 6 commits into from

Conversation

GuyKahlon
Copy link
Contributor

Hey Jesse,

Pull request checklist

What's in this pull request?

Fix very small bug from the open issues.

Thanks,
Guy

GuyKahlon and others added 2 commits February 26, 2016 17:57
…Toolbar preferredDefaultHeight.

It doesn't seem like you can change the height of the inputToolbar using the documented property preferredDefaultHeight since it is used to configure the height constraint in viewDidLoad.
In order to fix the bug I added the line of code:

self.toolbarHeightConstraint.constant = self.inputToolbar.preferredDefaultHeight;

In the method:

viewWillAppear, and now the view load with the correct constraints.
@jessesquires jessesquires added this to the 7.2.1 milestone Mar 2, 2016
@@ -196,7 +196,8 @@ - (void)viewWillAppear:(BOOL)animated
[super viewWillAppear:animated];
[self.view layoutIfNeeded];
[self.collectionView.collectionViewLayout invalidateLayout];

self.toolbarHeightConstraint.constant = self.inputToolbar.preferredDefaultHeight;
Copy link
Owner

Choose a reason for hiding this comment

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

we should probably put this before layoutIfNeeded above, right?

@jessesquires
Copy link
Owner

Thanks @GuyKahlon ! Looks good. 😄

If you can, could we remove commit 701fd26 from this? (seems like an accident)

@GuyKahlon
Copy link
Contributor Author

Hey @jessesquires,

Commit 701fd26 has been removed.
Move update toolbarHeightConstraint.constant property before layoutIfNeeded.

@jessesquires
Copy link
Owner

👍

last thing, mind squashing these commits into 1? 😄

@GuyKahlon
Copy link
Contributor Author

Hey Jesse,

I tried to squash the committees into 1 with rebase (http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html) but it looks worse now, we have a 5 comments after the merge.

Do you prefer I'll send you a new Pull Request with just 1 commit?
Or maybe you know a better way for squashing.

Sorry about all the mess 😁

@jessesquires
Copy link
Owner

@GuyKahlon -- sometimes a new pull request is easiest 😁

@jessesquires jessesquires removed this from the 7.2.1 milestone Mar 3, 2016
@jessesquires
Copy link
Owner

subsumed by #1477

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