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

ash/common/system/tray_accessibility.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/tray_accessibility.h"
 
 #include "ash/common/accessibility_delegate.h"
 #include "ash/common/accessibility_types.h"
 #include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/session/session_state_delegate.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/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_label_button.h"
 #include "ash/common/wm_shell.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "base/strings/utf_string_conversions.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_public.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/widget/widget.h"
 
 namespace ash {
 namespace {
 
 enum AccessibilityState {
   A11Y_NONE = 0,
@@ skipping 106 matching lines @@
       spoken_feedback_enabled_(false),
       high_contrast_enabled_(false),
       screen_magnifier_enabled_(false),
       large_cursor_enabled_(false),
       autoclick_enabled_(false),
       virtual_keyboard_enabled_(false),
       login_(login) {
   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();
 }
 
 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_);
+  high_contrast_enabled_ = delegate->IsHighContrastEnabled();
+  screen_magnifier_enabled_ = delegate->IsMagnifierEnabled();
+  virtual_keyboard_enabled_ = delegate->IsVirtualKeyboardEnabled();
 
-  // 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_);
+  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_);
+    }
   }
-
-  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_);
 }
 
 void AccessibilityDetailedView::AppendHelpEntries() {
   // Currently the help page requires a browser window.
   // TODO(yoshiki): show this even on login/lock screen. crbug.com/158286
   if (login_ == LoginStatus::NOT_LOGGED_IN || login_ == LoginStatus::LOCKED ||
       WmShell::Get()->GetSessionStateDelegate()->IsInSecondaryLoginScreen())
     return;
 
   views::View* bottom_row = new View();
@@ skipping 23 matching lines @@
 HoverHighlightView* AccessibilityDetailedView::AddScrollListItem(
     const base::string16& text,
     bool highlight,
     bool checked) {
   HoverHighlightView* container = new HoverHighlightView(this);
   container->AddCheckableLabel(text, highlight, checked);
   scroll_content()->AddChildView(container);
   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_) {
     WmShell::Get()->RecordUserMetricsAction(

[varkha, 2016/09/16 17:46:13]

While here, can you cache WmShell::Get() or even better make it so that there is
only one call to RecordUserMetricsAction() for an ID that depends on which view
is clicked.

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

Done.

         delegate->IsSpokenFeedbackEnabled()
             ? ash::UMA_STATUS_AREA_DISABLE_SPOKEN_FEEDBACK
             : ash::UMA_STATUS_AREA_ENABLE_SPOKEN_FEEDBACK);
     delegate->ToggleSpokenFeedback(A11Y_NOTIFICATION_NONE);
   } else if (view == high_contrast_view_) {
     WmShell::Get()->RecordUserMetricsAction(
         delegate->IsHighContrastEnabled()
             ? ash::UMA_STATUS_AREA_DISABLE_HIGH_CONTRAST
             : ash::UMA_STATUS_AREA_ENABLE_HIGH_CONTRAST);
     delegate->ToggleHighContrast();
@@ skipping 154 matching lines @@
     if (detailed_popup_)
       detailed_popup_->GetWidget()->Close();
     if (detailed_menu_)
       detailed_menu_->GetWidget()->Close();
   }
 
   previous_accessibility_state_ = accessibility_state;
 }
 
 }  // namespace ash