[Chrome OS MD] Implement accessibility detailed view for the MD Ash system menu

ash/common/system/tray_accessibility.cc

Index: ash/common/system/tray_accessibility.cc
diff --git a/ash/common/system/tray_accessibility.cc b/ash/common/system/tray_accessibility.cc
index 273a93937a0aff907e732b51e109e1b83d867279..559a10db11ab4998680373d4507df7bba823b572 100644
--- a/ash/common/system/tray_accessibility.cc
+++ b/ash/common/system/tray_accessibility.cc
@@ -23,8 +23,10 @@
 #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_public.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
@@ -151,7 +153,12 @@ AccessibilityDetailedView::AccessibilityDetailedView(SystemTrayItem* owner,
   Reset();
 
   AppendAccessibilityList();
-  AppendHelpEntries();
+
+  // The help option row is shown in non MD only. Note that the row will be
+  // integrated with the title row in MD design.
+  if (!ash::MaterialDesignController::IsSystemTrayMenuMaterial())
+    AppendHelpEntries();

[tdanderson, 2016/09/15 16:13:26]

This initializes two members that are now non-MD-only. Can you make a comment
above these members in the .h?

Also, you don't want to compare against these members in HandleButtonPressed()
if you're in MD, so an early return should be added there too.

[yiyix, 2016/09/19 20:24:55]

Good call.

+
   CreateTitleRow(IDS_ASH_STATUS_TRAY_ACCESSIBILITY_TITLE);
 
   Layout();
@@ -161,46 +168,101 @@ void AccessibilityDetailedView::AppendAccessibilityList() {
   CreateScrollableList();
   ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
 
+  // Generate entries in Accessibility detailed view menu for MD and non-MD,
+  // respectively.
   AccessibilityDelegate* delegate = WmShell::Get()->accessibility_delegate();
   spoken_feedback_enabled_ = delegate->IsSpokenFeedbackEnabled();
-  spoken_feedback_view_ =
-      AddScrollListItem(bundle.GetLocalizedString(
-                            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_SPOKEN_FEEDBACK),
-                        spoken_feedback_enabled_, spoken_feedback_enabled_);
-
-  // Large Cursor item is shown only in Login screen.
-  if (login_ == LoginStatus::NOT_LOGGED_IN) {
-    large_cursor_enabled_ = delegate->IsLargeCursorEnabled();
-    large_cursor_view_ =
-        AddScrollListItem(bundle.GetLocalizedString(
-                              IDS_ASH_STATUS_TRAY_ACCESSIBILITY_LARGE_CURSOR),
-                          large_cursor_enabled_, large_cursor_enabled_);
-  }
-
   high_contrast_enabled_ = delegate->IsHighContrastEnabled();
-  high_contrast_view_ = AddScrollListItem(
-      bundle.GetLocalizedString(
-          IDS_ASH_STATUS_TRAY_ACCESSIBILITY_HIGH_CONTRAST_MODE),
-      high_contrast_enabled_, high_contrast_enabled_);
   screen_magnifier_enabled_ = delegate->IsMagnifierEnabled();
-  screen_magnifier_view_ =
-      AddScrollListItem(bundle.GetLocalizedString(
-                            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_SCREEN_MAGNIFIER),
-                        screen_magnifier_enabled_, screen_magnifier_enabled_);
-
-  // Don't show autoclick option at login screen.
-  if (login_ != LoginStatus::NOT_LOGGED_IN) {
-    autoclick_enabled_ = delegate->IsAutoclickEnabled();
-    autoclick_view_ = AddScrollListItem(
-        bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_ACCESSIBILITY_AUTOCLICK),
-        autoclick_enabled_, autoclick_enabled_);
-  }
-
   virtual_keyboard_enabled_ = delegate->IsVirtualKeyboardEnabled();
-  virtual_keyboard_view_ =
-      AddScrollListItem(bundle.GetLocalizedString(
-                            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_VIRTUAL_KEYBOARD),
-                        virtual_keyboard_enabled_, virtual_keyboard_enabled_);
+
+  if (ash::MaterialDesignController::IsSystemTrayMenuMaterial()) {

[tdanderson, 2016/09/15 16:13:26]

This feels like a lot of repeated code. Can you instead just check for MD or
non-MD in AddScrollListItem() rather than introducing an AddScrollListItemMD() ?

[yiyix, 2016/09/19 20:24:55]

Done.

+    gfx::ImageSkia image = CreateVectorIcon(
+        kSystemMenuAccessibilityChromevoxIcon, kMenuIconSize, kMenuIconColor);
+    spoken_feedback_view_ = AddScrollListItemMD(
+        bundle.GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_SPOKEN_FEEDBACK),
+        spoken_feedback_enabled_, spoken_feedback_enabled_, image);
+
+    image = CreateVectorIcon(kSystemMenuAccessibilityContrastIcon,
+                             kMenuIconSize, kMenuIconColor);

[tdanderson, 2016/09/15 16:13:26]

Since the icon size is specified in the .icon files themselves, there is no need
to pass in kMenuIconSize.

[yiyix, 2016/09/19 20:24:55]

Done.

+    high_contrast_view_ = AddScrollListItemMD(
+        bundle.GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_HIGH_CONTRAST_MODE),
+        high_contrast_enabled_, high_contrast_enabled_, image);
+
+    image = CreateVectorIcon(kSystemMenuAccessibilityScreenMagnifierIcon,
+                             kMenuIconSize, kMenuIconColor);
+    screen_magnifier_view_ = AddScrollListItemMD(
+        bundle.GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_SCREEN_MAGNIFIER),
+        screen_magnifier_enabled_, screen_magnifier_enabled_, image);
+
+    // "Large Cursor" item is shown only in Login screen.
+    if (login_ == LoginStatus::NOT_LOGGED_IN) {
+      large_cursor_enabled_ = delegate->IsLargeCursorEnabled();
+      image = CreateVectorIcon(kSystemMenuAccessibilityLargeCursorIcon,
+                               kMenuIconSize, kMenuIconColor);
+      large_cursor_view_ = AddScrollListItemMD(
+          bundle.GetLocalizedString(
+              IDS_ASH_STATUS_TRAY_ACCESSIBILITY_LARGE_CURSOR),
+          large_cursor_enabled_, large_cursor_enabled_, image);
+    }
+
+    // Don't show "Auto Click" option at login screen.
+    if (login_ != LoginStatus::NOT_LOGGED_IN) {
+      image = CreateVectorIcon(kSystemMenuAccessibilityAutoClickIcon,
+                               kMenuIconSize, kMenuIconColor);
+      autoclick_enabled_ = delegate->IsAutoclickEnabled();
+      autoclick_view_ =
+          AddScrollListItemMD(bundle.GetLocalizedString(
+                                  IDS_ASH_STATUS_TRAY_ACCESSIBILITY_AUTOCLICK),
+                              autoclick_enabled_, autoclick_enabled_, image);
+    }
+
+    image = CreateVectorIcon(kSystemMenuKeyboardIcon, kMenuIconSize,
+                             kMenuIconColor);
+    virtual_keyboard_view_ = AddScrollListItemMD(
+        bundle.GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_VIRTUAL_KEYBOARD),
+        virtual_keyboard_enabled_, virtual_keyboard_enabled_, image);
+
+  } else {
+    spoken_feedback_view_ = AddScrollListItem(
+        bundle.GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_SPOKEN_FEEDBACK),
+        spoken_feedback_enabled_, spoken_feedback_enabled_);
+    high_contrast_view_ = AddScrollListItem(
+        bundle.GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_HIGH_CONTRAST_MODE),
+        high_contrast_enabled_, high_contrast_enabled_);
+    screen_magnifier_view_ = AddScrollListItem(
+        bundle.GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_SCREEN_MAGNIFIER),
+        screen_magnifier_enabled_, screen_magnifier_enabled_);
+    virtual_keyboard_view_ = AddScrollListItem(
+        bundle.GetLocalizedString(
+            IDS_ASH_STATUS_TRAY_ACCESSIBILITY_VIRTUAL_KEYBOARD),
+        virtual_keyboard_enabled_, virtual_keyboard_enabled_);
+
+    // Large Cursor item is shown only in Login screen.
+    if (login_ == LoginStatus::NOT_LOGGED_IN) {
+      large_cursor_enabled_ = delegate->IsLargeCursorEnabled();
+      large_cursor_view_ =
+          AddScrollListItem(bundle.GetLocalizedString(
+                                IDS_ASH_STATUS_TRAY_ACCESSIBILITY_LARGE_CURSOR),
+                            large_cursor_enabled_, large_cursor_enabled_);
+    }
+
+    // Don't show autoclick option at login screen.
+    if (login_ != LoginStatus::NOT_LOGGED_IN) {
+      autoclick_enabled_ = delegate->IsAutoclickEnabled();
+      autoclick_view_ =
+          AddScrollListItem(bundle.GetLocalizedString(
+                                IDS_ASH_STATUS_TRAY_ACCESSIBILITY_AUTOCLICK),
+                            autoclick_enabled_, autoclick_enabled_);
+    }
+  }
 }
 
 void AccessibilityDetailedView::AppendHelpEntries() {
@@ -244,6 +306,23 @@ HoverHighlightView* AccessibilityDetailedView::AddScrollListItem(
   return container;
 }
 
+HoverHighlightView* AccessibilityDetailedView::AddScrollListItemMD(
+    const base::string16& text,
+    bool highlight,
+    bool checked,
+    const gfx::ImageSkia& image) {
+  HoverHighlightView* container = new HoverHighlightView(this);
+  container->AddIconAndLabelCustomSize(
+      image, text, highlight, kMenuIconSize + kMenuIconMargin + kMenuIconMargin,

[tdanderson, 2016/09/15 16:13:26]

Prefer using kMenuIconSize + 2 * kMenuIconMargin in this situation.

[yiyix, 2016/09/19 20:24:55]

Done.

+      kIconPadding, kIconPadding);
+  gfx::ImageSkia check = CreateVectorIcon(gfx::VectorIconId::CHECK_CIRCLE,
+                                          kMenuIconSize, gfx::kGoogleGreen700);
+  container->AddRightIcon(check, kMenuIconSize);

[tdanderson, 2016/09/15 16:13:26]

I don't think AddRightIcon() actually needs to have the icon size passed in as a
parameter, it can just derive the size from the ImageSkia.

[yiyix, 2016/09/19 20:24:55]

Done.

+  container->SetRightIconVisible(checked);
+  scroll_content()->AddChildView(container);
+  return container;
+}
+
 void AccessibilityDetailedView::HandleViewClicked(views::View* view) {
   AccessibilityDelegate* delegate = WmShell::Get()->accessibility_delegate();
   if (view == spoken_feedback_view_) {