Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[self.contentView layoutSubviews] should be [self.contentView layoutIfNeeded] #1

Closed
tomabuct opened this issue Jan 16, 2014 · 2 comments

Comments

@tomabuct
Copy link

[self.contentView layoutSubviews] at line 103 should be [self.contentView layoutIfNeeded]. (you shouldn't call layoutSubviews directly according to the UIKit docs.)

it's required because the [super layoutSubviews] (on line 98) only sets the frame for self.contentView and causes it to need layout (self.contentView.layer.needsLayout becomes YES) but doesn't actually cause contentView's subviews to be laid out. this means that self.bodyLabel doesn't have its frame set at this point. while this usually happens later, we add the [self.contentView layoutIfNeeded] on line 103 to force this to occur immediately.

additionally, I noticed you didn't have a [super layoutSubviews] at the end of your layoutSubviews.

having done the above, you should be able to get rid of lines 172 and 173.

@smileyborg
Copy link
Owner

You are correct that layoutSubviews shouldn't be called directly, that's why the comment was there above it. I just re-tested using setNeedsLayout and layoutIfNeeded instead and it is working fine. I thought I had originally tested this and determined that only calling layoutSubviews directly on the contentView was actually working reliably, but I must have been mistaken.

As far as calling [super layoutSubviews] at the end, I haven't found this to be necessary. The act of setting the preferredMaxLayoutWidth causes the bodyLabel to need another layout pass, which appears to happen automatically by the system. You can test and see that Lines 172 & 173 are still required whether or not it is there.

I'll update the code now for the first point, but please feel free to fork the project and test on your own for the second. If you figure anything out don't hesitate to open a pull request!

@smileyborg
Copy link
Owner

Changes made in this commit.

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

No branches or pull requests

2 participants