TTFlowLayout doesn't correctly handle variable-height rows #428

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

mmorearty commented Feb 24, 2011

TTFlowLayout sets each row's height to the height of the last subview on that row. This seems like a bug to me -- it seems to me that it should set each row's height to the height of the tallest subview on that row.

For example, if a row contains subviews with heights (25, 25, 50), then the resulting row will have a height of 50, which is good; but if the row contains subviews of heights (25, 50, 25), then the resulting row will have a height of 25, which is bad.

Here is a sample: Make a project, add a new UIViewController, and then put this code in the UIViewController -- this will create a container view with a light gray background, which has three subviews of heights (25, 25, 50):

- (void) viewDidLoad {
  // 'top' is top container view with light gray background, width 100
  UIView* top = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 100, 0)] autorelease];
  top.backgroundColor = [UIColor lightGrayColor];

  // give it a border so it's easy to see how big it actually is
  top.layer.borderColor = [UIColor blackColor].CGColor;
  top.layer.borderWidth = 1;

  [self.view addSubview:top];

  // 'red' is just a red square, 25x25
  UIView* red = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 25, 25)] autorelease];
  red.backgroundColor = [UIColor redColor];
  [top addSubview:red];

  // 'green' is just a green square, 25x25
  UIView* green = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 25, 25)] autorelease];
  green.backgroundColor = [UIColor greenColor];
  [top addSubview:green];

  // 'blue' is a blue square, 50x50
  UIView* blue = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 50, 50)] autorelease];
  blue.backgroundColor = [UIColor blueColor];
  [top addSubview:blue];

  // lay out the colored-square subviews, and then resize the top
  // container view based on the resulting size
  TTFlowLayout* fl = [[[TTFlowLayout alloc] init] autorelease];
  CGSize size = [fl layoutSubviews:top.subviews forView:top];
  top.frame = CGRectMake(0, 0, size.width, size.height);
}

When you run it, you will see that it renders as expected -- the resulting row has a height of 50:

good

Now swap the 'green' and the 'blue' blocks of code, so that the blue square (the tallest one) is no longer the last subview on the row -- it is the second subview instead of the third:

- (void) viewDidLoad {
  // 'top' is top container view with light gray background, width 100
  UIView* top = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 100, 0)] autorelease];
  top.backgroundColor = [UIColor lightGrayColor];

  // give it a border so it's easy to see how big it actually is
  top.layer.borderColor = [UIColor blackColor].CGColor;
  top.layer.borderWidth = 1;

  [self.view addSubview:top];

  // 'red' is just a red square, 25x25
  UIView* red = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 25, 25)] autorelease];
  red.backgroundColor = [UIColor redColor];
  [top addSubview:red];

  // 'blue' is a blue square, 50x50
  UIView* blue = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 50, 50)] autorelease];
  blue.backgroundColor = [UIColor blueColor];
  [top addSubview:blue];

  // 'green' is just a green square, 25x25
  UIView* green = [[[UIView alloc] initWithFrame:CGRectMake(0, 0, 25, 25)] autorelease];
  green.backgroundColor = [UIColor greenColor];
  [top addSubview:green];

  // lay out the colored-square subviews, and then resize the top
  // container view based on the resulting size
  TTFlowLayout* fl = [[[TTFlowLayout alloc] init] autorelease];
  CGSize size = [fl layoutSubviews:top.subviews forView:top];
  top.frame = CGRectMake(0, 0, size.width, size.height);
}

When you run it, you get a messed-up layout -- the row height is only 25 instead of 50:

bad

Finally, run this again after applying my patch, and you'll see that the row height matches the height of the tallest subview on the row:

fixed

Contributor

jverkoey commented Feb 26, 2011

Testing this right now. This is one of the best pull requests I've seen, thanks so much for making this easy to verify and test.

Contributor

jverkoey commented Feb 26, 2011

Merge branch 'mmorearty-three20-flowlayout-bug' into development

Closed by ea82e78.

This issue was closed.

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