Skip to content

Commit

Permalink
[ios] Fixes TableViewTextItem text color assignment.
Browse files Browse the repository at this point in the history
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.

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 <kkhorimoto@chromium.org>
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577003}
  • Loading branch information
sczs authored and Commit Bot committed Jul 20, 2018
1 parent d530f68 commit 7c48c74
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ - (TableViewItem*)statusItemWithMessage:(NSString*)statusMessage
TableViewTextItem* entriesStatusItem =
[[TableViewTextItem alloc] initWithType:ItemTypeEntriesStatus];
entriesStatusItem.text = statusMessage;
entriesStatusItem.textColor = TextItemColorBlack;
entriesStatusItem.textColor = [UIColor blackColor];
statusMessageItem = entriesStatusItem;
}
return statusMessageItem;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ - (void)loadModel {
[[TableViewTextItem alloc] initWithType:ItemTypeText];
textItem.text = @"Simple Text Cell";
textItem.textAlignment = NSTextAlignmentCenter;
textItem.textColor = TextItemColorBlack;
textItem.textColor = [UIColor blackColor];
[model addItem:textItem toSectionWithIdentifier:SectionIdentifierText];

TableViewAccessoryItem* textAccessoryItem =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,7 @@ extern const CGFloat kUseDefaultFontSize;
// Spacing between text label and cell contentView.
extern const CGFloat kTableViewLabelVerticalTopSpacing;

// Hex Value for light gray label text color.
extern const CGFloat kTableViewTextLabelColorLightGrey;

#endif // IOS_CHROME_BROWSER_UI_TABLE_VIEW_CELLS_TABLE_VIEW_CELLS_CONSTANTS_H_
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@
const CGFloat kTableViewHighlightedCellColorAlpha = 0.5;
const CGFloat kUseDefaultFontSize = 0.0;
const CGFloat kTableViewLabelVerticalTopSpacing = 13.0;

const CGFloat kTableViewTextLabelColorLightGrey = 0x6D6D72;
14 changes: 4 additions & 10 deletions ios/chrome/browser/ui/table_view/cells/table_view_text_item.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,16 @@

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

// Defines the colors used for the cell text.
typedef NS_ENUM(UInt32, TextItemColor) {
TextItemColorLightGrey = 0x6D6D72,
TextItemColorBlack = 0x000000,
};

// TableViewTextItem contains the model data for a TableViewTextCell.
@interface TableViewTextItem : TableViewItem

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

// Hex color for the cell's textLabel. Default is TextItemColorLightGrey.
// ChromeTableViewStyler's |cellTitleColor| takes precedence over the default
// color, but not over |textColor|.
@property(nonatomic, assign) TextItemColor textColor;
// UIColor for the cell's textLabel. Default is
// kTableViewTextLabelColorLightGrey. ChromeTableViewStyler's |cellTitleColor|
// takes precedence over the default color, but not over |textColor|.
@property(nonatomic, assign) UIColor* textColor;

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ - (void)configureCell:(UITableViewCell*)tableCell
cell.textLabel.backgroundColor = styler.tableViewBackgroundColor;
// This item's text color takes precedence over the global styler.
// TODO(crbug.com/854249): redo the logic for this convoluted if clause.
if (self.textColor == TextItemColorBlack ||
self.textColor == TextItemColorLightGrey) {
cell.textLabel.textColor = UIColorFromRGB(self.textColor);
if (self.textColor) {
cell.textLabel.textColor = self.textColor;
} else if (styler.cellTitleColor) {
cell.textLabel.textColor = styler.cellTitleColor;
} else {
cell.textLabel.textColor = UIColorFromRGB(TextItemColorLightGrey);
cell.textLabel.textColor =
UIColorFromRGB(kTableViewTextLabelColorLightGrey);
}
cell.textLabel.textAlignment =
self.textAlignment ? self.textAlignment : NSTextAlignmentLeft;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
#import "ios/chrome/browser/ui/table_view/cells/table_view_text_item.h"

#include "base/mac/foundation_util.h"
#import "ios/chrome/browser/ui/table_view/cells/table_view_cells_constants.h"
#import "ios/chrome/browser/ui/table_view/chrome_table_view_styler.h"
#import "ios/chrome/browser/ui/uikit_ui_util.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/gtest_mac.h"
#include "testing/platform_test.h"
Expand Down Expand Up @@ -42,9 +44,37 @@
TableViewTextCell* cell = [[[item cellClass] alloc] init];
ASSERT_TRUE([cell isMemberOfClass:[TableViewTextCell class]]);

ChromeTableViewStyler* styler = [[ChromeTableViewStyler alloc] init];
UIColor* testTextColor = [UIColor redColor];
styler.cellTitleColor = testTextColor;
UIColor* testBackgroundColor = [UIColor blueColor];
styler.tableViewBackgroundColor = testBackgroundColor;
[item configureCell:cell withStyler:styler];
EXPECT_NSEQ(testBackgroundColor, cell.textLabel.backgroundColor);
EXPECT_NSEQ(testTextColor, cell.textLabel.textColor);
}

TEST_F(TableViewTextItemTest, ConfigureLabelColorWithProperty) {
TableViewTextItem* item = [[TableViewTextItem alloc] initWithType:0];
UIColor* textColor = [UIColor blueColor];
item.textColor = textColor;
TableViewTextCell* cell = [[[item cellClass] alloc] init];
ASSERT_TRUE([cell isMemberOfClass:[TableViewTextCell class]]);

ChromeTableViewStyler* styler = [[ChromeTableViewStyler alloc] init];
UIColor* testColor = [UIColor redColor];
styler.tableViewBackgroundColor = testColor;
[item configureCell:cell withStyler:styler];
EXPECT_NSEQ(testColor, cell.textLabel.backgroundColor);
EXPECT_NSEQ(textColor, cell.textLabel.textColor);
EXPECT_NSNE(testColor, cell.textLabel.textColor);
}

TEST_F(TableViewTextItemTest, ConfigureLabelColorWithDefaultColor) {
TableViewTextItem* item = [[TableViewTextItem alloc] initWithType:0];
TableViewTextCell* cell = [[[item cellClass] alloc] init];
ASSERT_TRUE([cell isMemberOfClass:[TableViewTextCell class]]);
ChromeTableViewStyler* styler = [[ChromeTableViewStyler alloc] init];
[item configureCell:cell withStyler:styler];
EXPECT_NSEQ(UIColorFromRGB(kTableViewTextLabelColorLightGrey),
cell.textLabel.textColor);
}

0 comments on commit 7c48c74

Please sign in to comment.