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

Text is truncated when using UITableViewCellAccessoryDisclosureIndicator #15

Closed
koenvanderdrift opened this issue Dec 24, 2014 · 16 comments

Comments

@koenvanderdrift
Copy link

First of all, many thanks for this sample project (and PureLayout).

In my app, the cells have a disclosure indicator, and in some cases this causes the text to be truncated. I tried this in your sample code by setting self.accessoryType = UITableViewCellAccessoryDisclosureIndicator; inside the init method of the cell, and I see the same at the end of the body text. I'm guessing the indicator interferes with the autolayout constraints, but I don't know how and where to correct for this. I tried increasing kLabelHorizontalInsets, but that didn't change anything. Removing the lines that set the lineBreakMode also didn't help.

Any suggestions?

@smileyborg
Copy link
Owner

You're likely running into the issue where the cell accessory view (disclosure indicator) is causing the cell's contentView to be smaller than the table view width, which breaks the height calculation. To fix this you'd need to set the cell size to the "correct" size (table view width minus width of accessory views and other things like a section index). Here is one of many posts about this issue: http://stackoverflow.com/questions/902096/uitableviewcells-contentviews-width-with-a-given-accessory-type There is no good way to do this that avoids hardcoding the values though.

The real solution is to use the self-sizing cell mechanism introduced in iOS 8. This of course is only available if your deployment target is 8.0 or higher. However, the self sizing cells will correctly take into account these cases.

Hope that helps.

@koenvanderdrift
Copy link
Author

So based on your answer and the link I changed

cell.bounds = CGRectMake(0.0f, 0.0f, CGRectGetWidth(tableView.bounds), CGRectGetHeight(cell.bounds));

to

cell.bounds = CGRectMake(0.0f, 0.0f, CGRectGetWidth(tableView.bounds) - 20, CGRectGetHeight(cell.bounds));

using the hardcoded value of 20.

The text is no longer truncated, but for some cells there is now a gap at the top of the body label.

So, did I make the width correction in the wrong place, or is something else going on as well?

I'm targeting iOS 7 and higher so unfortunately I cannot use the self-sizing cell mechanism introduced in iOS 8.

@smileyborg
Copy link
Owner

That is the correct place/way to do it. You'll probably have to debug further, but it is possible that the disclosure indicator does not take up 20 pt (I'm not sure what the value is -- you can try setting a breakpoint at runtime and checking the frame of the contentView to see how much less than the table view's width it is).

If that's not the issue, it might have to do with your constraints. Hard to say for sure.

@koenvanderdrift
Copy link
Author

Thanks, I'll explore with other values. Just note, this is all in your sample code, so I'm assuming the constraints are ok :)

@smileyborg
Copy link
Owner

Oh, if you haven't modified the sample code, then yes the constraints should be fine :) Please feel free to share your fork of the project on GitHub if you can't figure out what's going on.

@koenvanderdrift
Copy link
Author

Allright, I made a fork, and added some code adjustments to try to get the cell's dimensions to work. What I have currently is not yet working.

@smileyborg
Copy link
Owner

OK I took at look at your fork. Definitely see the sizing issue.

I first ran the project on the iPhone 6 sim, and checked what the actual sizes of the table view cells and their content views were:

(lldb) po [self.tableView visibleCells][0]
<RJTableViewCell: 0x7ff228786ec0; baseClass = UITableViewCell; frame = (0 0; 375 393); autoresize = W; layer = <CALayer: 0x7ff228786ac0>>

(lldb) po [[self.tableView visibleCells][0] contentView]
<UITableViewCellContentView: 0x7ff228785b30; frame = (0 0; 341 392.5); gestureRecognizers = <NSArray: 0x7ff228787310>; layer = <CALayer: 0x7ff228786720>>

Note the width of the table view cell is the width of the screen (as expected): 375 pt. But the width of the contentView is 341 pt in this case (with the disclosure indicator).

So that means we want to factor in this 375 - 341 = 34 pt difference for the sizing operation.

Instead of setting the table view cell's bounds, I have found it more reliable to add a constraint to the contentView directly. (The reason setting the cell's bounds works in some cases is because the table view cell internally resizes and fixes the contentView to fit inside of it. I think Apple has changed the internal implementation between iOS 7 & 8 though.)

Anyways, here are both changes together:

CGFloat width = 0.0f;

if (cell.accessoryType == UITableViewCellAccessoryNone && cell.accessoryView == nil)
    width = CGRectGetWidth(tableView.bounds);
else
    width = CGRectGetWidth(tableView.bounds) - 34.0f;

//    cell.bounds = CGRectMake(0.0f, 0.0f, width, CGRectGetHeight(cell.bounds));

// Instead of setting the cell.bounds, add a constraint to the contentView directly that enforces the final width.
// Use a slightly less than Required priority to avoid any unintentional conflicts with the internal UITableViewCell constraints.
[UIView autoSetPriority:UILayoutPriorityRequired - 1 forConstraints:^{
    [cell.contentView autoSetDimension:ALDimensionWidth toSize:width];
}];

This seems to be working well for me on the iPhone 6 sim. Give it a try and let me know how that goes.

Note that you'll probably need to measure and hardcode the revised values for the accessory view width, for all the scenarios/devices as needed.

@smileyborg
Copy link
Owner

One other note: that code above is incomplete, you'll want to make sure you store that constraint when you add it, so that you only add it once, and when the device rotates (or the table view otherwise changes width), you can simply update the constant of the constraint to the new width as needed.

@koenvanderdrift
Copy link
Author

Interestingly, on iOS 7 the difference is 33 pt. I have added that to my fork. Looks good now on both iOS versions, as well as on the iPhone 5(s) simulators.

@koenvanderdrift
Copy link
Author

I've noticed that adding

[UIView autoSetPriority:UILayoutPriorityRequired - 1 forConstraints:^{ [cell.contentView autoSetDimension:ALDimensionWidth toSize:width]; }];

decreases the performance of the table view, especially with long tables, scrolling becomes choppy. I have confirmed this with Instruments, there is at least a three-fold difference (in my app). My hunch is the slowdown is happening because this is called for every row and I am going to try to move this code to the updateConstraints method of the cell, so it will be called only once. I'll post back later with an update.

@smileyborg
Copy link
Owner

That's interesting. You definitely don't want to duplicate that constraint more than once per cell, let me know how that goes. Can you also try testing the performance if you make the constraint Required priority (e.g. omit the autoSetPriority... call)? I know that optional constraints are more expensive to solve than Required ones, and theoretically it should work fine if the constraint is Required here.

@koenvanderdrift
Copy link
Author

Yeah, it's much faster if I comment out the autoSetPriority: call. Maybe it's an iOS7 vs iOS8 issue, still looking into that. If I understand correctly, I don't need heightForRowAtIndexPath: and estimatedHeightForRowAtIndexPath: in iOS8, but my app should work on iOS7 too so I need to keep it.

@koenvanderdrift
Copy link
Author

Allright, it's an iOS 7/8 issue.

If I run the code in iOS7 (simulator), there are no hiccups. If I run the code in iOS8 there are hiccups. But if I completely comment out heightForRowAtIndexPath: and estimatedHeightForRowAtIndexPath:, and set the values for the rowHeight and estimatedRowHeight in viewDidLoad, the hiccups in iOS8 are gone.

So I need to make my code work in both iOS7 and iOS8, and asked about that on SO: http://stackoverflow.com/questions/27944801/skip-ignore-method-in-ios/27945778

@smileyborg
Copy link
Owner

Yeah, the iOS 8 self-sizing cells should be overall better than custom implementing that work yourself on iOS 7. So if possible you should use the self-sizing cell mechanism. This question asked the same question on SO already: http://stackoverflow.com/questions/26022113/how-to-only-override-a-method-depending-on-the-runtime-system-ios-version

I recommend using two different data source classes though, as opposed to conditionally making your view controller respond to that selector. But either should work.

@koenvanderdrift
Copy link
Author

Hope it's ok to ask this question here.

I've been trying to align the two UILabel views next to each other, and added that to my fork. What I see is:

  1. they are lined up correctly next to each other, so that part is working
  2. in iOS 7, I see all the text for the bodyLabel, but in some cases there is a lot of whitespace above and under it.
  3. in iOS 8, the text of the every bodyLabel is truncated.

Note, that I shortened the string length for the bodyLabel for clarity.

See my fork here https://github.com/koenvanderdrift/TableViewCellWithAutoLayout

Any suggestions how I can fix this?

@koenvanderdrift
Copy link
Author

I solved the iOS 7 issue by adding:

    [UIView autoSetPriority:UILayoutPriorityFittingSizeLevel forConstraints:^{
        [self.bodyLabel autoSetContentHuggingPriorityForAxis: ALAxisHorizontal];
    }];

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