Add bluetooth device type icons on MD system tray.

ash/common/system/chromeos/bluetooth/tray_bluetooth.cc

 // Copyright (c) 2012 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/common/system/chromeos/bluetooth/tray_bluetooth.h"
 
 #include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/session/session_state_delegate.h"
 #include "ash/common/system/tray/fixed_sized_scroll_view.h"
 #include "ash/common/system/tray/hover_highlight_view.h"
 #include "ash/common/system/tray/system_tray.h"
 #include "ash/common/system/tray/system_tray_delegate.h"
 #include "ash/common/system/tray/system_tray_notifier.h"
 #include "ash/common/system/tray/throbber_view.h"
 #include "ash/common/system/tray/tray_constants.h"
 #include "ash/common/system/tray/tray_details_view.h"
 #include "ash/common/system/tray/tray_item_more.h"
 #include "ash/common/system/tray/tray_popup_header_button.h"
 #include "ash/common/system/tray/tray_popup_item_style.h"
 #include "ash/common/wm_shell.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "grit/ash_resources.h"
 #include "grit/ash_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/vector_icons/vector_icons.h"

[tdanderson, 2016/09/29 18:54:39]

is this #include needed?

[fukino, 2016/09/30 02:43:49]

Removed. Thanks!

 #include "ui/views/controls/button/toggle_button.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
 
 namespace ash {
 namespace tray {
 namespace {
 
 // Updates bluetooth device |device| in the |list|. If it is new, append to the
@@ skipping 185 matching lines @@
                                            new_connecting_devices);
     RemoveObsoleteBluetoothDevicesFromList(&connected_devices_,
                                            new_connected_devices);
     RemoveObsoleteBluetoothDevicesFromList(&paired_not_connected_devices_,
                                            new_paired_not_connected_devices);
     RemoveObsoleteBluetoothDevicesFromList(&discovered_not_paired_devices_,
                                            new_discovered_not_paired_devices);
   }
 
   void UpdateHeaderEntry() {
-    if (toggle_bluetooth_) {
-      toggle_bluetooth_->SetToggled(
-          !WmShell::Get()->system_tray_delegate()->GetBluetoothEnabled());
-    }
+    bool isBluetoothEnabled =
+        WmShell::Get()->system_tray_delegate()->GetBluetoothEnabled();
+    if (toggle_)
+      toggle_->SetIsOn(isBluetoothEnabled, false);
+    if (toggle_bluetooth_)

[tdanderson, 2016/09/29 18:54:39]

nit: else if (since you'll never have both visible)?

[fukino, 2016/09/30 02:43:49]

Done.

+      toggle_bluetooth_->SetToggled(!isBluetoothEnabled);
   }
 
   void UpdateDeviceScrollList() {
     device_map_.clear();
     scroll_content()->RemoveAllChildViews(true);
     enable_bluetooth_ = nullptr;
 
     SystemTrayDelegate* delegate = WmShell::Get()->system_tray_delegate();
     bool bluetooth_enabled = delegate->GetBluetoothEnabled();
     bool blueooth_available = delegate->GetBluetoothAvailable();
@@ skipping 25 matching lines @@
     }
 
     scroll_content()->SizeToPreferredSize();
   }
 
   void AppendSameTypeDevicesToScrollList(const BluetoothDeviceList& list,
                                          bool highlight,
                                          bool checked,
                                          bool enabled) {
     for (size_t i = 0; i < list.size(); ++i) {
-      HoverHighlightView* container =
-          AddScrollListItem(list[i].display_name, highlight, checked, enabled);
-      device_map_[container] = list[i].address;
+      if (MaterialDesignController::IsSystemTrayMenuMaterial()) {
+        HoverHighlightView* container =
+            AddScrollListItemWithIcon(list[i].display_name, highlight, checked,
+                                      enabled, list[i].icon_image);
+        device_map_[container] = list[i].address;

[tdanderson, 2016/09/29 18:54:39]

nit: factor out lines 290 and 294 to after the if-else (perhaps use HHV*
container = md ? ... : ...;

[fukino, 2016/09/30 02:43:49]

Done. I used "md ? ... : ...";

+      } else {
+        HoverHighlightView* container = AddScrollListItem(
+            list[i].display_name, highlight, checked, enabled);
+        device_map_[container] = list[i].address;
+      }
     }
   }
 
   HoverHighlightView* AddScrollListItem(const base::string16& text,

[tdanderson, 2016/09/29 18:54:39]

As a side note, wherever it makes sense to do so, please add a "TODO: Remove
this code when material design is enabled by default. See crbug.com/614453" so
we don't leave any dead code behind.

[fukino, 2016/09/30 02:43:49]

Done.

                                         bool highlight,
                                         bool checked,
                                         bool enabled) {
     HoverHighlightView* container = new HoverHighlightView(this);
     views::Label* label =
         container->AddCheckableLabel(text, highlight, checked);
     label->SetEnabled(enabled);
     scroll_content()->AddChildView(container);
     return container;
   }
 
+  HoverHighlightView* AddScrollListItemWithIcon(const base::string16& text,
+                                                bool highlight,
+                                                bool checked,
+                                                bool enabled,
+                                                const gfx::ImageSkia& image) {
+    HoverHighlightView* container = new HoverHighlightView(this);

[fukino, 2016/09/29 16:27:36]

This implementation to place icons/labels/etc... is temporary.
I'll update the implementation in a separate CL to match the design spec.

[tdanderson, 2016/09/29 18:54:39]

Acknowledged. Whatever you do here is probably what we're going to use for the
rows across all other detailed views, and likely also for default views. Please
be sure to send those code reviews to me so we can look for opportunities to
share code.

The spec calls for a material design ripple to be added to these rows. I
recently introduced the SystemMenuButton class which defines an MD button with
hover and ripple. That may be useful to look at, but you may have to do
something slightly different if the items in the scroll list do not derive from
CustomButton (in particular, whatever class you use will have to derive from
InkDropHostView). If you have specific questions about the MD ripple, feel free
to reach out to bruthig@.

[fukino, 2016/09/30 02:43:49]

Thanks. I'll update this design next, and I'll make sure to send the reviews to
you.

+    const int padding = (kMenuButtonSize - image.width()) / 2;
+    container->AddIconAndLabelCustomSize(
+        image, text, highlight,
+        image.width() + kMenuSeparatorVerticalPadding * 2, padding, padding);
+    gfx::ImageSkia check_mark =
+        CreateVectorIcon(gfx::VectorIconId::CHECK_CIRCLE, gfx::kGoogleGreen700);
+    container->AddRightIcon(check_mark, check_mark.width());
+    container->SetRightIconVisible(checked);
+    container->text_label()->SetEnabled(enabled);
+    scroll_content()->AddChildView(container);
+    return container;
+  }
+
   // Add settings entries.
   void AppendSettingsEntries() {
     if (!WmShell::Get()->system_tray_delegate()->ShouldShowSettings())
       return;
 
     // Add bluetooth device requires a browser window, hide it for non logged in
     // user.
     if (login_ == LoginStatus::NOT_LOGGED_IN || login_ == LoginStatus::LOCKED ||
         WmShell::Get()->GetSessionStateDelegate()->IsInSecondaryLoginScreen()) {
       return;
@@ skipping 72 matching lines @@
     if (FoundDevice(device_id, connecting_devices_, nullptr))
       return;
 
     UpdateClickedDevice(device_id, view);
     delegate->ConnectToBluetoothDevice(device_id);
   }
 
   void HandleButtonPressed(views::Button* sender,
                            const ui::Event& event) override {
     if (MaterialDesignController::IsSystemTrayMenuMaterial()) {
-      // TODO(fukino): Make the toggle button functional.
       if (sender == toggle_)
-        toggle_->SetIsOn(toggle_->is_on(), true);
+        WmShell::Get()->system_tray_delegate()->ToggleBluetooth();
+      else
+        NOTREACHED();
       return;
     }
 
     SystemTrayDelegate* delegate = WmShell::Get()->system_tray_delegate();
     if (sender == toggle_bluetooth_)
       delegate->ToggleBluetooth();
     else
       NOTREACHED();
   }
 
@@ skipping 106 matching lines @@
     detailed_->Update();
 }
 
 void TrayBluetooth::OnBluetoothDiscoveringChanged() {
   if (!detailed_)
     return;
   detailed_->Update();
 }
 
 }  // namespace ash