ash: Adjust layout of clock for left/right shelf

* Convert the text to two lines, hours and minutes
* Make the hours white and the minutes gray
* Horizontally center the hours
* Bring the hours and minutes closer together

Also added a simple test for the class and renamed a misspelled constant.

BUG=292902
TEST=ash_unittests TimeViewTest.*
R=jennyz@chromium.org

Review URL: https://codereview.chromium.org/23463034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@223677 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 131 insertions, 63 deletions

diff --git a/ash/ash.gyp b/ash/ash.gyp
index cab1886..5d37119 100644
--- a/ash/ash.gyp
+++ b/ash/ash.gyp
@@ -753,6 +753,7 @@
         'system/chromeos/power/tray_power_unittest.cc',
         'system/chromeos/screen_security/screen_tray_item_unittest.cc',
         'system/chromeos/tray_display_unittest.cc',
+        'system/date/date_view_unittest.cc',
         'system/tray/system_tray_unittest.cc',
         'system/user/tray_user_unittest.cc',
         'system/web_notification/web_notification_tray_unittest.cc',
diff --git a/ash/system/date/date_view.cc b/ash/system/date/date_view.cc
index cc1552d..53eb162 100644
--- a/ash/system/date/date_view.cc
+++ b/ash/system/date/date_view.cc
@@ -31,8 +31,16 @@ namespace {
 // when the timer goes off.
 const int kTimerSlopSeconds = 1;
 
-// Top number text color of vertical clock.
-const SkColor kVerticalClockHourColor = SkColorSetRGB(0xBA, 0xBA, 0xBA);
+// Text color of the vertical clock minutes.
+const SkColor kVerticalClockMinuteColor = SkColorSetRGB(0xBA, 0xBA, 0xBA);
+
+// Padding between the left edge of the shelf and the left edge of the vertical
+// clock.
+const int kVerticalClockLeftPadding = 9;
+
+// Offset used to bring the minutes line closer to the hours line in the
+// vertical clock.
+const int kVerticalClockMinutesTopOffset = -4;
 
 base::string16 FormatDate(const base::Time& time) {
   icu::UnicodeString date_string;
@@ -169,6 +177,8 @@ void DateView::OnMouseExited(const ui::MouseEvent& event) {
   SchedulePaint();
 }
 
+///////////////////////////////////////////////////////////////////////////////
+
 TimeView::TimeView(TrayDate::ClockLayout clock_layout)
     : hour_type_(ash::Shell::GetInstance()->system_tray_delegate()->
                  GetHourClockType()) {
@@ -198,19 +208,15 @@ void TimeView::UpdateTextInternal(const base::Time& now) {
 
   base::string16 current_time = base::TimeFormatTimeOfDayWithHourClockType(
       now, hour_type_, base::kDropAmPm);
-  label_->SetText(current_time);
-  label_->SetTooltipText(base::TimeFormatFriendlyDate(now));
+  horizontal_label_->SetText(current_time);
+  horizontal_label_->SetTooltipText(base::TimeFormatFriendlyDate(now));
 
   // Calculate vertical clock layout labels.
   size_t colon_pos = current_time.find(ASCIIToUTF16(":"));
   base::string16 hour = current_time.substr(0, colon_pos);
   base::string16 minute = current_time.substr(colon_pos + 1);
-  label_hour_left_->SetText(hour.substr(0, 1));
-  label_hour_right_->SetText(hour.length() == 2 ?
-      hour.substr(1,1) : ASCIIToUTF16(":"));
-  label_minute_left_->SetText(minute.substr(0, 1));
-  label_minute_right_->SetText(minute.substr(1, 1));
-
+  vertical_label_hours_->SetText(hour);
+  vertical_label_minutes_->SetText(minute);
   Layout();
 }
 
@@ -226,31 +232,26 @@ bool TimeView::OnMousePressed(const ui::MouseEvent& event) {
 void TimeView::UpdateClockLayout(TrayDate::ClockLayout clock_layout){
   SetBorder(clock_layout);
   if (clock_layout == TrayDate::HORIZONTAL_CLOCK) {
-    RemoveChildView(label_hour_left_.get());
-    RemoveChildView(label_hour_right_.get());
-    RemoveChildView(label_minute_left_.get());
-    RemoveChildView(label_minute_right_.get());
+    RemoveChildView(vertical_label_hours_.get());
+    RemoveChildView(vertical_label_minutes_.get());
     SetLayoutManager(
         new views::BoxLayout(views::BoxLayout::kHorizontal, 0, 0, 0));
-    AddChildView(label_.get());
+    AddChildView(horizontal_label_.get());
   } else {
-    RemoveChildView(label_.get());
+    RemoveChildView(horizontal_label_.get());
     views::GridLayout* layout = new views::GridLayout(this);
     SetLayoutManager(layout);
-    views::ColumnSet* columns = layout->AddColumnSet(0);
-    columns->AddPaddingColumn(0, 6);
-    columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER,
-                       0, views::GridLayout::USE_PREF, 0, 0);
+    const int kColumnId = 0;
+    views::ColumnSet* columns = layout->AddColumnSet(kColumnId);
+    columns->AddPaddingColumn(0, kVerticalClockLeftPadding);
     columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER,
                        0, views::GridLayout::USE_PREF, 0, 0);
-    layout->AddPaddingRow(0, kTrayLabelItemVerticalPaddingVeriticalAlignment);
-    layout->StartRow(0, 0);
-    layout->AddView(label_hour_left_.get());
-    layout->AddView(label_hour_right_.get());
-    layout->StartRow(0, 0);
-    layout->AddView(label_minute_left_.get());
-    layout->AddView(label_minute_right_.get());
-    layout->AddPaddingRow(0, kTrayLabelItemVerticalPaddingVeriticalAlignment);
+    layout->AddPaddingRow(0, kTrayLabelItemVerticalPaddingVerticalAlignment);
+    layout->StartRow(0, kColumnId);
+    layout->AddView(vertical_label_hours_.get());
+    layout->StartRow(0, kColumnId);
+    layout->AddView(vertical_label_minutes_.get());
+    layout->AddPaddingRow(0, kTrayLabelItemVerticalPaddingVerticalAlignment);
   }
   Layout();
 }
@@ -265,18 +266,17 @@ void TimeView::SetBorder(TrayDate::ClockLayout clock_layout) {
 }
 
 void TimeView::SetupLabels() {
-  label_.reset(CreateLabel());
-  SetupLabel(label_.get());
-  label_hour_left_.reset(CreateLabel());
-  SetupLabel(label_hour_left_.get());
-  label_hour_right_.reset(CreateLabel());
-  SetupLabel(label_hour_right_.get());
-  label_minute_left_.reset(CreateLabel());
-  SetupLabel(label_minute_left_.get());
-  label_minute_right_.reset(CreateLabel());
-  SetupLabel(label_minute_right_.get());
-  label_hour_left_->SetEnabledColor(kVerticalClockHourColor);
-  label_hour_right_->SetEnabledColor(kVerticalClockHourColor);
+  horizontal_label_.reset(CreateLabel());
+  SetupLabel(horizontal_label_.get());
+  vertical_label_hours_.reset(CreateLabel());
+  SetupLabel(vertical_label_hours_.get());
+  vertical_label_minutes_.reset(CreateLabel());
+  SetupLabel(vertical_label_minutes_.get());
+  vertical_label_minutes_->SetEnabledColor(kVerticalClockMinuteColor);
+  // Pull the minutes up closer to the hours by using a negative top border.
+  vertical_label_minutes_->set_border(
+      views::Border::CreateEmptyBorder(
+          kVerticalClockMinutesTopOffset, 0, 0, 0));
 }
 
 void TimeView::SetupLabel(views::Label* label) {
diff --git a/ash/system/date/date_view.h b/ash/system/date/date_view.h
index 98bc45f..2c83da8 100644
--- a/ash/system/date/date_view.h
+++ b/ash/system/date/date_view.h
@@ -5,6 +5,7 @@
 #ifndef ASH_SYSTEM_DATE_DATE_VIEW_H_
 #define ASH_SYSTEM_DATE_DATE_VIEW_H_
 
+#include "ash/ash_export.h"
 #include "ash/system/date/tray_date.h"
 #include "ash/system/tray/actionable_view.h"
 #include "base/i18n/time_formatting.h"
@@ -20,8 +21,8 @@ namespace internal {
 namespace tray {
 
 // Abstract base class containing common updating and layout code for the
-// DateView popup and the TimeView tray icon.
-class BaseDateTimeView : public ActionableView {
+// DateView popup and the TimeView tray icon. Exported for tests.
+class ASH_EXPORT BaseDateTimeView : public ActionableView {
  public:
   virtual ~BaseDateTimeView();
 
@@ -78,17 +79,12 @@ class DateView : public BaseDateTimeView {
 };
 
 // Tray view used to display the current time.
-class TimeView : public BaseDateTimeView {
+// Exported for tests.
+class ASH_EXPORT TimeView : public BaseDateTimeView {
  public:
-  TimeView(TrayDate::ClockLayout clock_layout);
+  explicit TimeView(TrayDate::ClockLayout clock_layout);
   virtual ~TimeView();
 
-  views::Label* label() const { return label_.get(); }
-  views::Label* label_hour_left() const { return label_hour_left_.get(); }
-  views::Label* label_hour_right() const { return label_hour_right_.get(); }
-  views::Label* label_minute_left() const { return label_minute_left_.get(); }
-  views::Label* label_minute_right() const { return label_minute_right_.get(); }
-
   // Updates the format of the displayed time.
   void UpdateTimeFormat();
 
@@ -96,6 +92,8 @@ class TimeView : public BaseDateTimeView {
   void UpdateClockLayout(TrayDate::ClockLayout clock_layout);
 
  private:
+  friend class TimeViewTest;
+
   // Overridden from BaseDateTimeView.
   virtual void UpdateTextInternal(const base::Time& now) OVERRIDE;
 
@@ -109,11 +107,12 @@ class TimeView : public BaseDateTimeView {
   void SetupLabels();
   void SetupLabel(views::Label* label);
 
-  scoped_ptr<views::Label> label_;
-  scoped_ptr<views::Label> label_hour_left_;
-  scoped_ptr<views::Label> label_hour_right_;
-  scoped_ptr<views::Label> label_minute_left_;
-  scoped_ptr<views::Label> label_minute_right_;
+  // Label text used for the normal horizontal shelf.
+  scoped_ptr<views::Label> horizontal_label_;
+
+  // The time label is split into two lines for the vertical shelf.
+  scoped_ptr<views::Label> vertical_label_hours_;
+  scoped_ptr<views::Label> vertical_label_minutes_;
 
   base::HourClockType hour_type_;
 
diff --git a/ash/system/date/date_view_unittest.cc b/ash/system/date/date_view_unittest.cc
new file mode 100644
index 0000000..931d220
--- /dev/null
+++ b/ash/system/date/date_view_unittest.cc
@@ -0,0 +1,66 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/system/date/date_view.h"
+
+#include "ash/test/ash_test_base.h"
+#include "ui/views/controls/label.h"
+
+namespace ash {
+namespace internal {
+namespace tray {
+
+class TimeViewTest : public ash::test::AshTestBase {
+ public:
+  TimeViewTest() {}
+  virtual ~TimeViewTest() {}
+
+  TimeView* time_view() { return time_view_.get(); }
+
+  // Access to private fields of |time_view_|.
+  views::Label* horizontal_label() {
+    return time_view_->horizontal_label_.get();
+  }
+  views::Label* vertical_label_hours() {
+    return time_view_->vertical_label_hours_.get();
+  }
+  views::Label* vertical_label_minutes() {
+    return time_view_->vertical_label_minutes_.get();
+  }
+
+  // Creates a time view with horizontal or vertical |clock_layout|.
+  void CreateTimeView(TrayDate::ClockLayout clock_layout) {
+    time_view_.reset(new TimeView(clock_layout));
+  }
+
+ private:
+  scoped_ptr<TimeView> time_view_;
+
+  DISALLOW_COPY_AND_ASSIGN(TimeViewTest);
+};
+
+// Test the basics of the time view, mostly to ensure we don't leak memory.
+TEST_F(TimeViewTest, Basics) {
+  // A newly created horizontal clock only has the horizontal label.
+  CreateTimeView(TrayDate::HORIZONTAL_CLOCK);
+  EXPECT_EQ(time_view(), horizontal_label()->parent());
+  EXPECT_FALSE(vertical_label_hours()->parent());
+  EXPECT_FALSE(vertical_label_minutes()->parent());
+
+  // Switching the clock to vertical updates the labels.
+  time_view()->UpdateClockLayout(TrayDate::VERTICAL_CLOCK);
+  EXPECT_FALSE(horizontal_label()->parent());
+  EXPECT_EQ(time_view(), vertical_label_hours()->parent());
+  EXPECT_EQ(time_view(), vertical_label_minutes()->parent());
+
+  // Switching back to horizontal updates the labels again.
+  time_view()->UpdateClockLayout(TrayDate::HORIZONTAL_CLOCK);
+  EXPECT_EQ(time_view(), horizontal_label()->parent());
+  EXPECT_FALSE(vertical_label_hours()->parent());
+  EXPECT_FALSE(vertical_label_minutes()->parent());
+}
+
+}  // namespace tray
+}  // namespace internal
+}  // namespace ash
diff --git a/ash/system/session_length_limit/tray_session_length_limit.cc b/ash/system/session_length_limit/tray_session_length_limit.cc
index 6ca887a..fe8ff7e 100644
--- a/ash/system/session_length_limit/tray_session_length_limit.cc
+++ b/ash/system/session_length_limit/tray_session_length_limit.cc
@@ -201,7 +201,7 @@ void RemainingSessionTimeTrayView::UpdateClockLayout(
                        0, views::GridLayout::USE_PREF, 0, 0);
     columns->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER,
                        0, views::GridLayout::USE_PREF, 0, 0);
-    layout->AddPaddingRow(0, kTrayLabelItemVerticalPaddingVeriticalAlignment);
+    layout->AddPaddingRow(0, kTrayLabelItemVerticalPaddingVerticalAlignment);
     layout->StartRow(0, 0);
     layout->AddView(vertical_layout_label_hours_left_);
     layout->AddView(vertical_layout_label_hours_right_);
@@ -211,7 +211,7 @@ void RemainingSessionTimeTrayView::UpdateClockLayout(
     layout->StartRow(0, 0);
     layout->AddView(vertical_layout_label_seconds_left_);
     layout->AddView(vertical_layout_label_seconds_right_);
-    layout->AddPaddingRow(0, kTrayLabelItemVerticalPaddingVeriticalAlignment);
+    layout->AddPaddingRow(0, kTrayLabelItemVerticalPaddingVerticalAlignment);
   }
   Update();
 }
diff --git a/ash/system/tray/tray_constants.cc b/ash/system/tray/tray_constants.cc
index 5ce1f0e..024390f 100644
--- a/ash/system/tray/tray_constants.cc
+++ b/ash/system/tray/tray_constants.cc
@@ -26,7 +26,9 @@ const int kTrayImageItemHorizontalPaddingVerticalAlignment = 1;
 const int kTrayImageItemVerticalPaddingVerticalAlignment = 1;
 
 const int kTrayLabelItemHorizontalPaddingBottomAlignment = 7;
-const int kTrayLabelItemVerticalPaddingVeriticalAlignment = 4;
+
+// Vertical padding between status tray items when the shelf is vertical.
+const int kTrayLabelItemVerticalPaddingVerticalAlignment = 4;
 
 const int kTrayMenuBottomRowPadding = 5;
 const int kTrayMenuBottomRowPaddingBetweenItems = -1;
diff --git a/ash/system/tray/tray_constants.h b/ash/system/tray/tray_constants.h
index 1645f73..10c638e 100644
--- a/ash/system/tray/tray_constants.h
+++ b/ash/system/tray/tray_constants.h
@@ -24,7 +24,7 @@ extern const int kTrayImageItemHorizontalPaddingVerticalAlignment;
 extern const int kTrayImageItemVerticalPaddingVerticalAlignment;
 
 extern const int kTrayLabelItemHorizontalPaddingBottomAlignment;
-extern const int kTrayLabelItemVerticalPaddingVeriticalAlignment;
+extern const int kTrayLabelItemVerticalPaddingVerticalAlignment;
 
 extern const int kTrayMenuBottomRowPadding;
 extern const int kTrayMenuBottomRowPaddingBetweenItems;
diff --git a/ash/system/tray/tray_utils.cc b/ash/system/tray/tray_utils.cc
index 5f29a2e..40cabea 100644
--- a/ash/system/tray/tray_utils.cc
+++ b/ash/system/tray/tray_utils.cc
@@ -54,9 +54,9 @@ void SetTrayLabelItemBorder(TrayItemView* tray_view,
     int horizontal_padding = (tray_view->GetPreferredSize().width() -
         tray_view->label()->GetPreferredSize().width()) / 2;
     tray_view->set_border(views::Border::CreateEmptyBorder(
-        kTrayLabelItemVerticalPaddingVeriticalAlignment,
+        kTrayLabelItemVerticalPaddingVerticalAlignment,
         horizontal_padding,
-        kTrayLabelItemVerticalPaddingVeriticalAlignment,
+        kTrayLabelItemVerticalPaddingVerticalAlignment,
         horizontal_padding));
   }
 }
diff --git a/ash/system/user/tray_user.cc b/ash/system/user/tray_user.cc
index c33f28d..2e030f5 100644
--- a/ash/system/user/tray_user.cc
+++ b/ash/system/user/tray_user.cc
@@ -1260,9 +1260,9 @@ void TrayUser::UpdateAfterShelfAlignmentChange(ShelfAlignment alignment) {
     }
     if (label_) {
       label_->set_border(views::Border::CreateEmptyBorder(
-          kTrayLabelItemVerticalPaddingVeriticalAlignment,
+          kTrayLabelItemVerticalPaddingVerticalAlignment,
           kTrayLabelItemHorizontalPaddingBottomAlignment,
-          kTrayLabelItemVerticalPaddingVeriticalAlignment,
+          kTrayLabelItemVerticalPaddingVerticalAlignment,
           kTrayLabelItemHorizontalPaddingBottomAlignment));
     }
     layout_view_->SetLayoutManager(