[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_icon_types.h"

[tdanderson, 2016/09/19 23:32:27]

I don't think you need this #include (#including paint_vector_icon should be
sufficient?)

[yiyix, 2016/09/21 17:10:48]

you are right!

+#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();
+
+  if (!ash::MaterialDesignController::IsSystemTrayMenuMaterial())

[tdanderson, 2016/09/19 23:32:27]

nit: ash:: not needed (here and elsewhere in this CL). Originally I suggested
adding ash:: before MaterialDesignController to avoid confusing it with the top
chrome MDC, but that is in the process of being killed off anyway.

[yiyix, 2016/09/21 17:10:48]

I know MaterialDesignController exists in UI space and ash space. I thought it
is a good hobbit of adding it, so it can be differentiated from the UI one. Does
it mean that the ui one is enabled by default?

[tdanderson, 2016/09/21 17:59:32]

As discussed in person, top-chrome MDC calls are in the process of being
removed, and in retrospect telling the difference between the two was probably
only an issue for the handful of people who worked on both projects.

+    AppendHelpEntries();
+
   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_);
+                        spoken_feedback_enabled_, spoken_feedback_enabled_,
+                        kSystemMenuAccessibilityChromevoxIcon);
 
   // 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_);
+                          large_cursor_enabled_, large_cursor_enabled_,
+                          kSystemMenuAccessibilityLargeCursorIcon);
   }
 
   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_);
+      high_contrast_enabled_, high_contrast_enabled_,
+      kSystemMenuAccessibilityContrastIcon);
   screen_magnifier_enabled_ = delegate->IsMagnifierEnabled();
   screen_magnifier_view_ =
       AddScrollListItem(bundle.GetLocalizedString(
                             IDS_ASH_STATUS_TRAY_ACCESSIBILITY_SCREEN_MAGNIFIER),
-                        screen_magnifier_enabled_, screen_magnifier_enabled_);
+                        screen_magnifier_enabled_, screen_magnifier_enabled_,
+                        kSystemMenuAccessibilityScreenMagnifierIcon);
 
   // 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_);
+        autoclick_enabled_, autoclick_enabled_,
+        kSystemMenuAccessibilityAutoClickIcon);
   }
 
   virtual_keyboard_enabled_ = delegate->IsVirtualKeyboardEnabled();
   virtual_keyboard_view_ =
       AddScrollListItem(bundle.GetLocalizedString(
                             IDS_ASH_STATUS_TRAY_ACCESSIBILITY_VIRTUAL_KEYBOARD),
-                        virtual_keyboard_enabled_, virtual_keyboard_enabled_);
+                        virtual_keyboard_enabled_, virtual_keyboard_enabled_,
+                        kSystemMenuKeyboardIcon);
 }
 
 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 16 matching lines @@
       bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_ACCESSIBILITY_SETTINGS));
   bottom_row->AddChildView(settings);
   settings_view_ = settings;
 
   AddChildView(bottom_row);
 }
 
 HoverHighlightView* AccessibilityDetailedView::AddScrollListItem(
     const base::string16& text,
     bool highlight,
-    bool checked) {
+    bool checked,
+    const gfx::VectorIcon& icon) {
   HoverHighlightView* container = new HoverHighlightView(this);
-  container->AddCheckableLabel(text, highlight, checked);
+  if (ash::MaterialDesignController::IsSystemTrayMenuMaterial()) {
+    gfx::ImageSkia image = CreateVectorIcon(icon, kMenuIconColor);
+    const int padding = (kMenuButtonSize - image.width()) / 2;
+    container->AddIconAndLabelCustomSize(image, text, highlight,
+                                         image.width() + kMenuIconMargin * 2,
+                                         padding, padding);
+    gfx::ImageSkia check_mark =
+        CreateVectorIcon(gfx::VectorIconId::CHECK_CIRCLE, gfx::kGoogleGreen700);
+    container->AddRightIcon(check_mark, check_mark.width());
+    container->SetRightIconVisible(checked);
+  } else {
+    container->AddCheckableLabel(text, highlight, checked);
+  }
   scroll_content()->AddChildView(container);
   return container;
 }
 
 void AccessibilityDetailedView::HandleViewClicked(views::View* view) {
   AccessibilityDelegate* delegate = WmShell::Get()->accessibility_delegate();
+  UserMetricsAction user_action;

[tdanderson, 2016/09/19 23:32:27]

Nice.

[yiyix, 2016/09/21 17:10:48]

Thanks :)

   if (view == spoken_feedback_view_) {
-    WmShell::Get()->RecordUserMetricsAction(
-        delegate->IsSpokenFeedbackEnabled()
-            ? ash::UMA_STATUS_AREA_DISABLE_SPOKEN_FEEDBACK
-            : ash::UMA_STATUS_AREA_ENABLE_SPOKEN_FEEDBACK);
+    user_action = 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);
+    user_action = delegate->IsHighContrastEnabled()
+                      ? ash::UMA_STATUS_AREA_DISABLE_HIGH_CONTRAST
+                      : ash::UMA_STATUS_AREA_ENABLE_HIGH_CONTRAST;
     delegate->ToggleHighContrast();
   } else if (view == screen_magnifier_view_) {
-    WmShell::Get()->RecordUserMetricsAction(
-        delegate->IsMagnifierEnabled() ? ash::UMA_STATUS_AREA_DISABLE_MAGNIFIER
-                                       : ash::UMA_STATUS_AREA_ENABLE_MAGNIFIER);
+    user_action = delegate->IsMagnifierEnabled()
+                      ? ash::UMA_STATUS_AREA_DISABLE_MAGNIFIER
+                      : ash::UMA_STATUS_AREA_ENABLE_MAGNIFIER;
     delegate->SetMagnifierEnabled(!delegate->IsMagnifierEnabled());
   } else if (large_cursor_view_ && view == large_cursor_view_) {
-    WmShell::Get()->RecordUserMetricsAction(
-        delegate->IsLargeCursorEnabled()
-            ? ash::UMA_STATUS_AREA_DISABLE_LARGE_CURSOR
-            : ash::UMA_STATUS_AREA_ENABLE_LARGE_CURSOR);
+    user_action = delegate->IsLargeCursorEnabled()
+                      ? ash::UMA_STATUS_AREA_DISABLE_LARGE_CURSOR
+                      : ash::UMA_STATUS_AREA_ENABLE_LARGE_CURSOR;
     delegate->SetLargeCursorEnabled(!delegate->IsLargeCursorEnabled());
   } else if (autoclick_view_ && view == autoclick_view_) {
-    WmShell::Get()->RecordUserMetricsAction(
-        delegate->IsAutoclickEnabled()
-            ? ash::UMA_STATUS_AREA_DISABLE_AUTO_CLICK
-            : ash::UMA_STATUS_AREA_ENABLE_AUTO_CLICK);
+    user_action = delegate->IsAutoclickEnabled()
+                      ? ash::UMA_STATUS_AREA_DISABLE_AUTO_CLICK
+                      : ash::UMA_STATUS_AREA_ENABLE_AUTO_CLICK;
     delegate->SetAutoclickEnabled(!delegate->IsAutoclickEnabled());
   } else if (virtual_keyboard_view_ && view == virtual_keyboard_view_) {
-    WmShell::Get()->RecordUserMetricsAction(
-        delegate->IsVirtualKeyboardEnabled()
-            ? ash::UMA_STATUS_AREA_DISABLE_VIRTUAL_KEYBOARD
-            : ash::UMA_STATUS_AREA_ENABLE_VIRTUAL_KEYBOARD);
+    user_action = delegate->IsVirtualKeyboardEnabled()
+                      ? ash::UMA_STATUS_AREA_DISABLE_VIRTUAL_KEYBOARD
+                      : ash::UMA_STATUS_AREA_ENABLE_VIRTUAL_KEYBOARD;
     delegate->SetVirtualKeyboardEnabled(!delegate->IsVirtualKeyboardEnabled());
+  } else {
+    return;

[tdanderson, 2016/09/19 23:32:27]

An early return at the top of the function would be preferable.

   }
+  WmShell::Get()->RecordUserMetricsAction(user_action);
 }
 
 void AccessibilityDetailedView::HandleButtonPressed(views::Button* sender,
                                                     const ui::Event& event) {
-  SystemTrayDelegate* tray_delegate = WmShell::Get()->system_tray_delegate();
-  if (sender == help_view_)
-    tray_delegate->ShowAccessibilityHelp();
-  else if (sender == settings_view_)
-    tray_delegate->ShowAccessibilitySettings();
+  // Since this row is not initialized in MD, this function should be called for

[tdanderson, 2016/09/19 23:32:27]

I suggest removing this comment, since in the very near future you/someone will
be using this for the MD "info" button.

[yiyix, 2016/09/21 17:10:48]

Done.

+  // non-MD only.
+  if (!MaterialDesignController::UseMaterialDesignSystemIcons()) {

[tdanderson, 2016/09/19 23:32:27]

To avoid indenting the whole function, it would be better to use an early return
instead.

[yiyix, 2016/09/21 17:10:48]

Done.

+    SystemTrayDelegate* tray_delegate = WmShell::Get()->system_tray_delegate();
+    if (sender == help_view_)
+      tray_delegate->ShowAccessibilityHelp();
+    else if (sender == settings_view_)
+      tray_delegate->ShowAccessibilitySettings();
+  }
 }
 
 }  // namespace tray
 
 ////////////////////////////////////////////////////////////////////////////////
 // ash::TrayAccessibility
 
 TrayAccessibility::TrayAccessibility(SystemTray* system_tray)
     : TrayImageItem(system_tray,
                     IDR_AURA_UBER_TRAY_ACCESSIBILITY,
@@ skipping 111 matching lines @@
     if (detailed_popup_)
       detailed_popup_->GetWidget()->Close();
     if (detailed_menu_)
       detailed_menu_->GetWidget()->Close();
   }
 
   previous_accessibility_state_ = accessibility_state;
 }
 
 }  // namespace ash