Commit c1f77f2f authored by Yi Su's avatar Yi Su Committed by Commit Bot

Fix TableViewDetailIconItem layout defect.

This CL fixes the layout defect of TableViewDetailIconItem, which
causes an unsmooth animation when the labels' text are updated. The
solution is to use AutoLayout constraints instead of calculating width
of both labels during UIView.LayoutSubviews. It's hard to prove that the
constraints are correct, but it works :D

Screen shot:
https://drive.google.com/open?id=1UJoZ81NXE0gPQab80cGBVQFOVHqh1BBm

Bug: 927682
Change-Id: I516469c7bd1bcbdfeebd7d9ddfe492fa39cd28a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1554354
Commit-Queue: Yi Su <mrsuyi@chromium.org>
Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#648602}
parent 95038519
...@@ -181,11 +181,27 @@ typedef NS_ENUM(NSInteger, ItemType) { ...@@ -181,11 +181,27 @@ typedef NS_ENUM(NSInteger, ItemType) {
detailIconItem.detailText = @"Short"; detailIconItem.detailText = @"Short";
[model addItem:detailIconItem toSectionWithIdentifier:SectionIdentifierText]; [model addItem:detailIconItem toSectionWithIdentifier:SectionIdentifierText];
TableViewDetailIconItem* detailIconItemLong = TableViewDetailIconItem* detailIconItemLeftLong =
[[TableViewDetailIconItem alloc] initWithType:ItemTypeTextSettingsDetail]; [[TableViewDetailIconItem alloc] initWithType:ItemTypeTextSettingsDetail];
detailIconItemLong.text = @"Very long text eating the other detail label"; detailIconItemLeftLong.text =
detailIconItemLong.detailText = @"A bit less short"; @"Left label is very very very very very very very long";
[model addItem:detailIconItemLong detailIconItemLeftLong.detailText = @"R";
[model addItem:detailIconItemLeftLong
toSectionWithIdentifier:SectionIdentifierText];
TableViewDetailIconItem* detailIconItemRightLong =
[[TableViewDetailIconItem alloc] initWithType:ItemTypeTextSettingsDetail];
detailIconItemRightLong.text = @"L";
detailIconItemRightLong.detailText =
@"Right label is very very very very very very very long";
[model addItem:detailIconItemRightLong
toSectionWithIdentifier:SectionIdentifierText];
TableViewDetailIconItem* detailIconItemBothLong =
[[TableViewDetailIconItem alloc] initWithType:ItemTypeTextSettingsDetail];
detailIconItemBothLong.text = @"Left label occupy 75% of row space";
detailIconItemBothLong.detailText = @"Right label occupy 25% of row space";
[model addItem:detailIconItemBothLong
toSectionWithIdentifier:SectionIdentifierText]; toSectionWithIdentifier:SectionIdentifierText];
// SectionIdentifierSettings. // SectionIdentifierSettings.
......
...@@ -41,27 +41,6 @@ ...@@ -41,27 +41,6 @@
// the full width of the cell. // the full width of the cell.
- (void)setIconImage:(UIImage*)image; - (void)setIconImage:(UIImage*)image;
// The amount of horizontal space to provide to each of the labels, Exposed for
// testing. When the preferred ContentSize category chosen by the user isn't one
// of the accessibility categories, these values are determined with the
// following logic:
//
// - If there is sufficient room (after accounting for margins) for the full
// width of each label, use the current width of each label.
// - If not, use the current width of the main label and a clipped width for the
// detail label.
// - Unless the main label wants more than 75% of the available width and the
// detail label wants 25% or less of the available width, in which case use a
// clipped width for the main label and the current width of the detail label.
// - If both labels want more width than their guaranteed minimums (75% and
// 25%), use the guaranteed minimum amount for each.
//
// If the PreferredContentSizeCategory is an Accessibility category, those
// values aren't used. The content is displayed on two labels, one above the
// other, without limitation on the number of lines used by the labels.
@property(nonatomic, readonly) CGFloat textLabelTargetWidth;
@property(nonatomic, readonly) CGFloat detailTextLabelTargetWidth;
@end @end
#endif // IOS_CHROME_BROWSER_UI_TABLE_VIEW_CELLS_TABLE_VIEW_DETAIL_ICON_ITEM_H_ #endif // IOS_CHROME_BROWSER_UI_TABLE_VIEW_CELLS_TABLE_VIEW_DETAIL_ICON_ITEM_H_
...@@ -20,22 +20,16 @@ namespace { ...@@ -20,22 +20,16 @@ namespace {
// Padding used between the icon and the text labels. // Padding used between the icon and the text labels.
const CGFloat kIconTrailingPadding = 12; const CGFloat kIconTrailingPadding = 12;
// Size of the icon image. // Size of the icon image.
const CGFloat kIconImageSize = 28; const CGFloat kIconImageSize = 28;
// Proportion of Cell's textLabel/detailTextLabel. This guarantees that the
// textLabel occupies 75% of the row space and detailTextLabel occupies 25%.
const CGFloat kCellLabelsWidthProportion = 3.0f;
// Minimum proportion of the available width to guarantee to the main and detail
// labels.
const CGFloat kMinTextWidthRatio = 0.75f;
const CGFloat kMinDetailTextWidthRatio = 0.25f;
} // namespace } // namespace
@implementation TableViewDetailIconItem @implementation TableViewDetailIconItem
@synthesize iconImageName = _iconImageName;
@synthesize text = _text;
@synthesize detailText = _detailText;
- (instancetype)initWithType:(NSInteger)type { - (instancetype)initWithType:(NSInteger)type {
self = [super initWithType:type]; self = [super initWithType:type];
if (self) { if (self) {
...@@ -83,8 +77,6 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f; ...@@ -83,8 +77,6 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f;
UILayoutGuide* _labelContainerGuide; UILayoutGuide* _labelContainerGuide;
NSLayoutConstraint* _iconHiddenConstraint; NSLayoutConstraint* _iconHiddenConstraint;
NSLayoutConstraint* _iconVisibleConstraint; NSLayoutConstraint* _iconVisibleConstraint;
NSLayoutConstraint* _textLabelWidthConstraint;
NSLayoutConstraint* _detailTextLabelWidthConstraint;
} }
@synthesize detailTextLabel = _detailTextLabel; @synthesize detailTextLabel = _detailTextLabel;
...@@ -113,6 +105,7 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f; ...@@ -113,6 +105,7 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f;
_textLabel.adjustsFontForContentSizeCategory = YES; _textLabel.adjustsFontForContentSizeCategory = YES;
_textLabel.textColor = [UIColor blackColor]; _textLabel.textColor = [UIColor blackColor];
_textLabel.backgroundColor = [UIColor clearColor]; _textLabel.backgroundColor = [UIColor clearColor];
_textLabel.textAlignment = NSTextAlignmentLeft;
[contentView addSubview:_textLabel]; [contentView addSubview:_textLabel];
_detailTextLabel = [[UILabel alloc] init]; _detailTextLabel = [[UILabel alloc] init];
...@@ -122,15 +115,9 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f; ...@@ -122,15 +115,9 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f;
_detailTextLabel.adjustsFontForContentSizeCategory = YES; _detailTextLabel.adjustsFontForContentSizeCategory = YES;
_detailTextLabel.textColor = UIColorFromRGB(kSettingsCellsDetailTextColor); _detailTextLabel.textColor = UIColorFromRGB(kSettingsCellsDetailTextColor);
_detailTextLabel.backgroundColor = [UIColor clearColor]; _detailTextLabel.backgroundColor = [UIColor clearColor];
_detailTextLabel.textAlignment = NSTextAlignmentRight;
[contentView addSubview:_detailTextLabel]; [contentView addSubview:_detailTextLabel];
// Set up the width constraints. They are activated here and updated in
// layoutSubviews.
_textLabelWidthConstraint =
[_textLabel.widthAnchor constraintEqualToConstant:0];
_detailTextLabelWidthConstraint =
[_detailTextLabel.widthAnchor constraintEqualToConstant:0];
// Set up the constraints for when the icon is visible and hidden. One of // Set up the constraints for when the icon is visible and hidden. One of
// these will be active at a time, defaulting to hidden. // these will be active at a time, defaulting to hidden.
_iconHiddenConstraint = [_labelContainerGuide.leadingAnchor _iconHiddenConstraint = [_labelContainerGuide.leadingAnchor
...@@ -140,17 +127,45 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f; ...@@ -140,17 +127,45 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f;
constraintEqualToAnchor:_iconImageView.trailingAnchor constraintEqualToAnchor:_iconImageView.trailingAnchor
constant:kIconTrailingPadding]; constant:kIconTrailingPadding];
// Rules between |_textLabel| width and |_detailTextLabel| width:
// 1. Widths are represented by the percentage according to the
// available space inside |_labelContainerGuide|;
// 2. |_textLabel| has a width quota of 75%;
// 3. |_detailTextLabe| has a width quota of 25%;
// 4. When label A fits in A's quota, rest of A's quota can be used by
// the other label(i.e. B can exceed B's quota), and vice versa;
// 5. When both labels exceed their quota, they occupy their quotas.
//
// Expected space allocation result:
// Rows are |_textLabel|'s quota.
// Columns are |_detailTextLabel|'s quota.
// Pairs are actual width for (|_textLabel|, |_detailTextLabel|).
//
// 20% 60% 90%
//
// 20% (20%, 20%) (20%, 60%) (20%, 80%)
// 60% (60%, 20%) (60%, 40%) (60%, 30%)
// 90% (80%, 20%) (75%, 25%) (75%, 25%)
//
NSLayoutConstraint* widthConstraint = [_textLabel.widthAnchor
constraintEqualToAnchor:_detailTextLabel.widthAnchor
multiplier:kCellLabelsWidthProportion];
// Set low priority to the proportion constraint between |_textLabel| and
// |_detailTextLabel|, so that it won't break other layouts.
widthConstraint.priority = UILayoutPriorityDefaultLow;
_standardConstraints = @[ _standardConstraints = @[
_textLabelWidthConstraint,
_detailTextLabelWidthConstraint,
// Set up the vertical constraints and align the baselines of the two text // Set up the vertical constraints and align the baselines of the two text
// labels. // labels.
[_textLabel.centerYAnchor [_textLabel.centerYAnchor
constraintEqualToAnchor:contentView.centerYAnchor], constraintEqualToAnchor:contentView.centerYAnchor],
[_textLabel.trailingAnchor
constraintLessThanOrEqualToAnchor:_detailTextLabel.leadingAnchor
constant:-kTableViewHorizontalSpacing],
[_detailTextLabel.firstBaselineAnchor [_detailTextLabel.firstBaselineAnchor
constraintEqualToAnchor:_textLabel.firstBaselineAnchor], constraintEqualToAnchor:_textLabel.firstBaselineAnchor],
[_detailTextLabel.trailingAnchor [_detailTextLabel.trailingAnchor
constraintEqualToAnchor:_labelContainerGuide.trailingAnchor], constraintEqualToAnchor:_labelContainerGuide.trailingAnchor],
widthConstraint,
]; ];
_accessibilityConstraints = @[ _accessibilityConstraints = @[
...@@ -235,24 +250,6 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f; ...@@ -235,24 +250,6 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f;
} }
} }
// Updates the layout constraints of the text labels and then calls the
// superclass's implementation of layoutSubviews which can then take account of
// the new constraints.
- (void)layoutSubviews {
[super layoutSubviews];
// Size the labels in order to determine how much width they want.
[self.textLabel sizeToFit];
[self.detailTextLabel sizeToFit];
// Update the width constraints.
_textLabelWidthConstraint.constant = self.textLabelTargetWidth;
_detailTextLabelWidthConstraint.constant = self.detailTextLabelTargetWidth;
// Now invoke the layout.
[super layoutSubviews];
}
#pragma mark - UITableViewCell #pragma mark - UITableViewCell
- (void)prepareForReuse { - (void)prepareForReuse {
...@@ -281,34 +278,6 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f; ...@@ -281,34 +278,6 @@ const CGFloat kMinDetailTextWidthRatio = 0.25f;
} }
} }
- (CGFloat)textLabelTargetWidth {
CGFloat availableWidth = CGRectGetWidth(_labelContainerGuide.layoutFrame) -
kTableViewHorizontalSpacing;
CGFloat textLabelWidth = self.textLabel.frame.size.width;
CGFloat detailTextLabelWidth = self.detailTextLabel.frame.size.width;
if (textLabelWidth + detailTextLabelWidth <= availableWidth)
return textLabelWidth;
return std::max(
availableWidth - detailTextLabelWidth,
std::min(availableWidth * kMinTextWidthRatio, textLabelWidth));
}
- (CGFloat)detailTextLabelTargetWidth {
CGFloat availableWidth = CGRectGetWidth(_labelContainerGuide.layoutFrame) -
kTableViewHorizontalSpacing;
CGFloat textLabelWidth = self.textLabel.frame.size.width;
CGFloat detailTextLabelWidth = self.detailTextLabel.frame.size.width;
if (textLabelWidth + detailTextLabelWidth <= availableWidth)
return detailTextLabelWidth;
return std::max(availableWidth - textLabelWidth,
std::min(availableWidth * kMinDetailTextWidthRatio,
detailTextLabelWidth));
}
- (NSString*)accessibilityLabel { - (NSString*)accessibilityLabel {
return self.textLabel.text; return self.textLabel.text;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment