Skip to content

Commit ddf4374

Browse files
author
sczs
committed
[ios] Fixes TableViewTextItem text color assignment.
TextItemColorBlack was being used as the default color even if textColor was not being set. This was causing the item label to skip the styler textColor for the label. This CL fixes that problem and introduces some unit_tests to check this. [email protected] (cherry picked from commit 7c48c74) Bug: 865808 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: Ic9e7d368d4acb5cab8f2585762c69363c1143749 Reviewed-on: https://chromium-review.googlesource.com/1145584 Reviewed-by: Kurt Horimoto <[email protected]> Commit-Queue: Sergio Collazos <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#577003} Reviewed-on: https://chromium-review.googlesource.com/1151529 Reviewed-by: Sergio Collazos <[email protected]> Cr-Commit-Position: refs/branch-heads/3497@{#111} Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
1 parent 0227fab commit ddf4374

File tree

7 files changed

+46
-17
lines changed

7 files changed

+46
-17
lines changed

ios/chrome/browser/ui/history/history_table_view_controller.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ - (TableViewItem*)statusItemWithMessage:(NSString*)statusMessage
769769
TableViewTextItem* entriesStatusItem =
770770
[[TableViewTextItem alloc] initWithType:ItemTypeEntriesStatus];
771771
entriesStatusItem.text = statusMessage;
772-
entriesStatusItem.textColor = TextItemColorBlack;
772+
entriesStatusItem.textColor = [UIColor blackColor];
773773
statusMessageItem = entriesStatusItem;
774774
}
775775
return statusMessageItem;

ios/chrome/browser/ui/settings/table_cell_catalog_view_controller.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ - (void)loadModel {
7777
[[TableViewTextItem alloc] initWithType:ItemTypeText];
7878
textItem.text = @"Simple Text Cell";
7979
textItem.textAlignment = NSTextAlignmentCenter;
80-
textItem.textColor = TextItemColorBlack;
80+
textItem.textColor = [UIColor blackColor];
8181
[model addItem:textItem toSectionWithIdentifier:SectionIdentifierText];
8282

8383
TableViewAccessoryItem* textAccessoryItem =

ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,7 @@ extern const CGFloat kUseDefaultFontSize;
3434
// Spacing between text label and cell contentView.
3535
extern const CGFloat kTableViewLabelVerticalTopSpacing;
3636

37+
// Hex Value for light gray label text color.
38+
extern const CGFloat kTableViewTextLabelColorLightGrey;
39+
3740
#endif // IOS_CHROME_BROWSER_UI_TABLE_VIEW_CELLS_TABLE_VIEW_CELLS_CONSTANTS_H_

ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.mm

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,5 @@
1717
const CGFloat kTableViewHighlightedCellColorAlpha = 0.5;
1818
const CGFloat kUseDefaultFontSize = 0.0;
1919
const CGFloat kTableViewLabelVerticalTopSpacing = 13.0;
20+
21+
const CGFloat kTableViewTextLabelColorLightGrey = 0x6D6D72;

ios/chrome/browser/ui/table_view/cells/table_view_text_item.h

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,16 @@
99

1010
#import "ios/chrome/browser/ui/table_view/cells/table_view_item.h"
1111

12-
// Defines the colors used for the cell text.
13-
typedef NS_ENUM(UInt32, TextItemColor) {
14-
TextItemColorLightGrey = 0x6D6D72,
15-
TextItemColorBlack = 0x000000,
16-
};
17-
1812
// TableViewTextItem contains the model data for a TableViewTextCell.
1913
@interface TableViewTextItem : TableViewItem
2014

2115
// Text Alignment for the cell's textLabel. Default is NSTextAlignmentLeft.
2216
@property(nonatomic, assign) NSTextAlignment textAlignment;
2317

24-
// Hex color for the cell's textLabel. Default is TextItemColorLightGrey.
25-
// ChromeTableViewStyler's |cellTitleColor| takes precedence over the default
26-
// color, but not over |textColor|.
27-
@property(nonatomic, assign) TextItemColor textColor;
18+
// UIColor for the cell's textLabel. Default is
19+
// kTableViewTextLabelColorLightGrey. ChromeTableViewStyler's |cellTitleColor|
20+
// takes precedence over the default color, but not over |textColor|.
21+
@property(nonatomic, assign) UIColor* textColor;
2822

2923
@property(nonatomic, readwrite, strong) NSString* text;
3024

ios/chrome/browser/ui/table_view/cells/table_view_text_item.mm

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@ - (void)configureCell:(UITableViewCell*)tableCell
4242
cell.textLabel.backgroundColor = styler.tableViewBackgroundColor;
4343
// This item's text color takes precedence over the global styler.
4444
// TODO(crbug.com/854249): redo the logic for this convoluted if clause.
45-
if (self.textColor == TextItemColorBlack ||
46-
self.textColor == TextItemColorLightGrey) {
47-
cell.textLabel.textColor = UIColorFromRGB(self.textColor);
45+
if (self.textColor) {
46+
cell.textLabel.textColor = self.textColor;
4847
} else if (styler.cellTitleColor) {
4948
cell.textLabel.textColor = styler.cellTitleColor;
5049
} else {
51-
cell.textLabel.textColor = UIColorFromRGB(TextItemColorLightGrey);
50+
cell.textLabel.textColor =
51+
UIColorFromRGB(kTableViewTextLabelColorLightGrey);
5252
}
5353
cell.textLabel.textAlignment =
5454
self.textAlignment ? self.textAlignment : NSTextAlignmentLeft;

ios/chrome/browser/ui/table_view/cells/table_view_text_item_unittest.mm

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
#import "ios/chrome/browser/ui/table_view/cells/table_view_text_item.h"
66

77
#include "base/mac/foundation_util.h"
8+
#import "ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.h"
89
#import "ios/chrome/browser/ui/table_view/chrome_table_view_styler.h"
10+
#import "ios/chrome/browser/ui/uikit_ui_util.h"
911
#include "testing/gtest/include/gtest/gtest.h"
1012
#include "testing/gtest_mac.h"
1113
#include "testing/platform_test.h"
@@ -42,9 +44,37 @@
4244
TableViewTextCell* cell = [[[item cellClass] alloc] init];
4345
ASSERT_TRUE([cell isMemberOfClass:[TableViewTextCell class]]);
4446

47+
ChromeTableViewStyler* styler = [[ChromeTableViewStyler alloc] init];
48+
UIColor* testTextColor = [UIColor redColor];
49+
styler.cellTitleColor = testTextColor;
50+
UIColor* testBackgroundColor = [UIColor blueColor];
51+
styler.tableViewBackgroundColor = testBackgroundColor;
52+
[item configureCell:cell withStyler:styler];
53+
EXPECT_NSEQ(testBackgroundColor, cell.textLabel.backgroundColor);
54+
EXPECT_NSEQ(testTextColor, cell.textLabel.textColor);
55+
}
56+
57+
TEST_F(TableViewTextItemTest, ConfigureLabelColorWithProperty) {
58+
TableViewTextItem* item = [[TableViewTextItem alloc] initWithType:0];
59+
UIColor* textColor = [UIColor blueColor];
60+
item.textColor = textColor;
61+
TableViewTextCell* cell = [[[item cellClass] alloc] init];
62+
ASSERT_TRUE([cell isMemberOfClass:[TableViewTextCell class]]);
63+
4564
ChromeTableViewStyler* styler = [[ChromeTableViewStyler alloc] init];
4665
UIColor* testColor = [UIColor redColor];
4766
styler.tableViewBackgroundColor = testColor;
4867
[item configureCell:cell withStyler:styler];
49-
EXPECT_NSEQ(testColor, cell.textLabel.backgroundColor);
68+
EXPECT_NSEQ(textColor, cell.textLabel.textColor);
69+
EXPECT_NSNE(testColor, cell.textLabel.textColor);
70+
}
71+
72+
TEST_F(TableViewTextItemTest, ConfigureLabelColorWithDefaultColor) {
73+
TableViewTextItem* item = [[TableViewTextItem alloc] initWithType:0];
74+
TableViewTextCell* cell = [[[item cellClass] alloc] init];
75+
ASSERT_TRUE([cell isMemberOfClass:[TableViewTextCell class]]);
76+
ChromeTableViewStyler* styler = [[ChromeTableViewStyler alloc] init];
77+
[item configureCell:cell withStyler:styler];
78+
EXPECT_NSEQ(UIColorFromRGB(kTableViewTextLabelColorLightGrey),
79+
cell.textLabel.textColor);
5080
}

0 commit comments

Comments
 (0)