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

Remove sizeToFit() from SizeCalculable protocol #164

Merged
merged 10 commits into from Sep 26, 2018

Conversation

antoinelamy
Copy link
Contributor

As per the documentation, we should not directly override sizeToFit() but rather always only implement sizeThatFits(_:) for auto-sizing needs. This PR aim to remove the sizeToFit() requirement in the SizeCalculable protocol.

You should not override this method. If you want to change the default sizing information for your view, override the sizeThatFits(_:) instead. That method performs any needed calculations and returns them to this method, which then makes the change.

Internally, UIView has a very basic implementation of the sizeToFit() method which is the equivalent of calling sizeThatFits with the view bounds so instead of calling it directly, we replicate its behaviour inside PinLayout (the implementation is not gonna change anytime soon anyway).

The original class look like this in assembly pseudo-code:
screen shot 2018-08-13 at 18 53 14

Once converted into a more human readable form, the implementation source code look like this:

- (void)sizeToFit {
  CGAffineTransform *transform = [self transform];

  if (CGAffineTransformIsIdentity(transform)) {
    CGRect frame = [self frame];
    CGSize size = [self sizeThatFits: CGSizeMake(frame.width, frame.height)];
    [self setFrame: CGRectMake(frame.x, frame.y, size.width, size.height)];
  }
  else {
    CGRect bounds = [self bounds];
    CGSize size = [self sizeThatFits: CGSizeMake(bounds.width, bounds.height)];
    [self setBounds: CGRectMake(bounds.x, bounds.y, size.width, size.height)];
  }
}

@antoinelamy antoinelamy changed the title Remove sizeToFit() from SizeCalculable protocol Remove sizeToFit() from SizeCalculable protocol Aug 14, 2018
@@ -278,6 +278,8 @@ import AppKit
@discardableResult func aspectRatio() -> PinLayoutObjC
#endif

@discardableResult func sizeToFit() -> PinLayoutObjC
Copy link
Member

Choose a reason for hiding this comment

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

Good catch 👍 We should eventually add more unit tests for objective-c.

} else {
size.width = sizeThatFits.width
}
if case .fitTypeContent = adjustSizeType {} else {
Copy link
Member

Choose a reason for hiding this comment

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

Icchh if case .fitTypeContent = adjustSizeType {} else { 😬. I would prefer a switch case here, even if it is longer, it is cleaner.

Sources/Types.swift Show resolved Hide resolved
@lucdion
Copy link
Member

lucdion commented Aug 14, 2018

@antoinelamy does this match also the size computed by UILabel.sizeToFit()?

@antoinelamy
Copy link
Contributor Author

antoinelamy commented Sep 15, 2018

@lucdion sorry it took forever but I finally got the chance to make the requested changes. About your question wether or not it match the size computed by UILabel.sizeToFit(), the answer is yes. I also added tests to ensure they both produce the same result with or without a transform applied.

@lucdion
Copy link
Member

lucdion commented Sep 15, 2018

Thanks @antoinelamy 🙂

@lucdion lucdion merged commit 963ea84 into layoutBox:master Sep 26, 2018
@antoinelamy antoinelamy deleted the feature/sizeToFit branch June 16, 2021 10:30
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.

None yet

2 participants