Modify TrayItemMore to use enabled status

ash/common/system/tray/tray_item_more.cc

Index: ash/common/system/tray/tray_item_more.cc
diff --git a/ash/common/system/tray/tray_item_more.cc b/ash/common/system/tray/tray_item_more.cc
index a6e38fff82dd8a200200c727ed69d33f9323bac9..592f8eab6d1619f60ac3d99feabeaf557371bb2c 100644
--- a/ash/common/system/tray/tray_item_more.cc
+++ b/ash/common/system/tray/tray_item_more.cc
@@ -4,7 +4,6 @@
 
 #include "ash/common/system/tray/tray_item_more.h"
 
-#include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/system/tray/system_tray_item.h"
 #include "ash/common/system/tray/tray_constants.h"
 #include "ash/common/system/tray/tray_popup_item_style.h"
@@ -12,49 +11,32 @@
 #include "ash/common/system/tray/tri_view.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "base/memory/ptr_util.h"
-#include "grit/ash_resources.h"
 #include "ui/accessibility/ax_node_data.h"
-#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
-#include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/fill_layout.h"
 
 namespace ash {
 
-TrayItemMore::TrayItemMore(SystemTrayItem* owner, bool show_more)
+TrayItemMore::TrayItemMore(SystemTrayItem* owner)
     : ActionableView(owner, TrayPopupInkDropStyle::FILL_BOUNDS),
-      show_more_(show_more),
-      icon_(nullptr),
-      label_(nullptr),
-      more_(nullptr) {
-  TriView* tri_view = TrayPopupUtils::CreateDefaultRowView();
-  AddChildView(tri_view);
+      tri_view_(TrayPopupUtils::CreateDefaultRowView()),
+      icon_(TrayPopupUtils::CreateMainImageView()),
+      label_(TrayPopupUtils::CreateDefaultLabel()),
+      more_(TrayPopupUtils::CreateMoreImageView()) {
+  AddChildView(tri_view_);
   SetLayoutManager(new views::FillLayout);
 
-  icon_ = TrayPopupUtils::CreateMainImageView();
-  tri_view->AddView(TriView::Container::START, icon_);
-
-  label_ = TrayPopupUtils::CreateDefaultLabel();
-  tri_view->AddView(TriView::Container::CENTER, label_);
-
-  if (show_more) {
-    more_ = TrayPopupUtils::CreateMoreImageView();
-    if (!MaterialDesignController::IsSystemTrayMenuMaterial()) {
-      // The icon doesn't change in non-md.
-      more_->SetImage(ui::ResourceBundle::GetSharedInstance()
-                          .GetImageNamed(IDR_AURA_UBER_TRAY_MORE)
-                          .ToImageSkia());
-    }
-    tri_view->AddView(TriView::Container::END, more_);
-  } else {
-    tri_view->SetContainerVisible(TriView::Container::END, false);
-  }
-
-  if (MaterialDesignController::IsSystemTrayMenuMaterial())
-    SetInkDropMode(InkDropHostView::InkDropMode::ON);
+  more_->SetImage(
+      gfx::CreateVectorIcon(kSystemMenuArrowRightIcon, kMenuIconColor));
+
+  tri_view_->AddView(TriView::Container::START, icon_);
+  tri_view_->AddView(TriView::Container::CENTER, label_);
+  tri_view_->AddView(TriView::Container::END, more_);
+
+  SetInkDropMode(InkDropHostView::InkDropMode::ON);
 }
 
 TrayItemMore::~TrayItemMore() {}
@@ -75,26 +57,23 @@ void TrayItemMore::SetAccessibleName(const base::string16& name) {
 }
 
 std::unique_ptr<TrayPopupItemStyle> TrayItemMore::CreateStyle() const {
+  std::unique_ptr<TrayPopupItemStyle> style = HandleCreateStyle();
+  if (!enabled())
+    style->set_color_style(TrayPopupItemStyle::ColorStyle::DISABLED);
+  return style;
+}
+
+std::unique_ptr<TrayPopupItemStyle> TrayItemMore::HandleCreateStyle() const {
   return base::MakeUnique<TrayPopupItemStyle>(
       TrayPopupItemStyle::FontStyle::DEFAULT_VIEW_LABEL);
 }
 
 void TrayItemMore::UpdateStyle() {
-  if (!MaterialDesignController::IsSystemTrayMenuMaterial())
-    return;
   std::unique_ptr<TrayPopupItemStyle> style = CreateStyle();
   style->SetupLabel(label_);

[tdanderson, 2017/02/17 22:29:47]

Don't we also want to be theming |icon_| based on the enabled/disabled state? 

[mohsen, 2017/02/21 21:04:56]

Theming of the |icon_| happens in each subclass since currently we can't
generically change the icon color here. Subclasses already use the style
returned by CreateStyle() which now takes enabled state into account to update
the icon (an exception is network row).

[tdanderson, 2017/02/21 21:30:41]

Acknowledged. Might be worth a quick chat with Sebastien to see if he's happy
with that or if he has any further theming suggestions (no need to block landing
this CL, though.)

-
-  if (more_) {
-    more_->SetImage(
-        gfx::CreateVectorIcon(kSystemMenuArrowRightIcon, kMenuIconColor));
-  }
 }
 
 bool TrayItemMore::PerformAction(const ui::Event& event) {
-  if (!show_more_)
-    return false;
-
   owner()->TransitionDetailedView();
   return true;
 }
@@ -105,6 +84,11 @@ void TrayItemMore::GetAccessibleNodeData(ui::AXNodeData* node_data) {
     node_data->SetName(accessible_name_);
 }
 
+void TrayItemMore::OnEnabledChanged() {
+  ActionableView::OnEnabledChanged();
+  tri_view_->SetContainerVisible(TriView::Container::END, enabled());

[tdanderson, 2017/02/17 22:29:47]

If the enabled state were to ever change from disabled->enabled (though I don't
think that will currently ever happen), it seems that you'd want to be calling
UpdateStyle() otherwise the color theming will be incorrect. Is there any harm
in just calling UpdateStyle() at the end of this function?

[mohsen, 2017/02/21 21:04:56]

Right. Done.

+}
+
 void TrayItemMore::OnNativeThemeChanged(const ui::NativeTheme* theme) {
   ActionableView::OnNativeThemeChanged(theme);
   UpdateStyle();