ash: Visual and accessibility fixes for in-session auth dialog

1. Adjust title font size and margins according to style guide
2. Left align title, right align cancel button.
3. Apply round corners.
4. Enable title to be read out in accessibility use cases.
5. Enable fingerprint label to be read in accessibility use cases.

Bug: b:156258540, b:144861739
Change-Id: I2d5970da96e31f9df162ccbe74db8d95fdd49ec6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2393249Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Yicheng Li <yichengli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805541}

ash/in_session_auth/auth_dialog_contents_view.cc

@@ -34,25 +34,22 @@ namespace ash {
 namespace {
 
 enum class ButtonId {
-  kMoreOptions,
   kCancel,
 };
 
 // TODO(b/164195709): Move these strings to a grd file.
 const char kTitle[] = "Verify it's you";
-// If fingerprint option is available, password input field will be hidden
-// until the user taps the MoreOptions button.
-const char kMoreOptionsButtonText[] = "More options";
 const char kCancelButtonText[] = "Cancel";
 
 const int kContainerPreferredWidth = 512;
-const int kTopVerticalSpacing = 24;
-const int kVerticalSpacingBetweenTitleAndPrompt = 16;
-const int kVerticalSpacingBetweenPasswordAndPINKeyboard = 16;
-const int kBottomVerticalSpacing = 20;
-const int kButtonSpacing = 8;
+const int kSpacingAfterTitle = 16;
 
-const int kTitleFontSize = 14;
+const int kBorderTopDp = 24;
+const int kBorderLeftDp = 24;
+const int kBorderBottomDp = 20;
+const int kBorderRightDp = 24;
+
+const int kTitleFontSizeDeltaDp = 4;
 
 constexpr int kFingerprintIconSizeDp = 28;
 constexpr int kFingerprintIconTopSpacingDp = 20;
@@ -70,6 +67,8 @@ constexpr base::TimeDelta kFingerprintFailedAnimationDuration =
 constexpr SkColor kDisabledFingerprintIconColor =
     SkColorSetA(SK_ColorDKGRAY, 97);
 
+constexpr int kSpacingBeforeButtons = 32;
+
 }  // namespace
 
 // Consists of fingerprint icon view and a label.
@@ -113,6 +112,7 @@ class AuthDialogContentsView::FingerprintView : public views::View {
     label_->SetAutoColorReadabilityEnabled(false);
     label_->SetEnabledColor(SK_ColorDKGRAY);
     label_->SetMultiLine(true);
+    label_->SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
 
     DisplayCurrentState();
   }
@@ -196,8 +196,16 @@ class AuthDialogContentsView::FingerprintView : public views::View {
   void DisplayCurrentState() {
     SetVisible(state_ != FingerprintState::UNAVAILABLE);
     SetIcon(state_);
-    if (state_ != FingerprintState::UNAVAILABLE)
-      label_->SetText(l10n_util::GetStringUTF16(GetTextIdFromState()));
+    if (state_ != FingerprintState::UNAVAILABLE) {
+      base::string16 fingerprint_text =
+          l10n_util::GetStringUTF16(GetTextIdFromState());
+      label_->SetText(fingerprint_text);
+      label_->SetAccessibleName(
+          state_ == FingerprintState::DISABLED_FROM_ATTEMPTS
+              ? l10n_util::GetStringUTF16(
+                    IDS_ASH_IN_SESSION_AUTH_FINGERPRINT_ACCESSIBLE_DISABLED_FROM_ATTEMPTS)
+              : fingerprint_text);
+    }
   }
 
   void SetIcon(FingerprintState state) {
@@ -258,6 +266,8 @@ AuthDialogContentsView::AuthDialogContentsView(uint32_t auth_methods)
   SetLayoutManager(std::make_unique<views::FillLayout>());
   container_ = AddChildView(std::make_unique<NonAccessibleView>());
   container_->SetBackground(views::CreateSolidBackground(SK_ColorWHITE));
+  container_->SetBorder(views::CreateEmptyBorder(
+      kBorderTopDp, kBorderLeftDp, kBorderBottomDp, kBorderRightDp));
 
   main_layout_ =
       container_->SetLayoutManager(std::make_unique<views::BoxLayout>(
@@ -267,13 +277,10 @@ AuthDialogContentsView::AuthDialogContentsView(uint32_t auth_methods)
   main_layout_->set_cross_axis_alignment(
       views::BoxLayout::CrossAxisAlignment::kCenter);
 
-  AddVerticalSpacing(kTopVerticalSpacing);
   AddTitleView();
-  AddVerticalSpacing(kVerticalSpacingBetweenTitleAndPrompt);
-  // TODO(b/156258540): Add proper spacing once all elements are determined.
+  AddVerticalSpacing(kSpacingAfterTitle);
   AddPasswordView();
   AddPinView();
-  AddVerticalSpacing(kVerticalSpacingBetweenPasswordAndPINKeyboard);
 
   if (auth_methods_ & kAuthFingerprint) {
     fingerprint_view_ =
@@ -281,13 +288,15 @@ AuthDialogContentsView::AuthDialogContentsView(uint32_t auth_methods)
     fingerprint_view_->SetCanUsePin(auth_methods_ & kAuthPin);
   }
 
+  AddVerticalSpacing(kSpacingBeforeButtons);
   AddActionButtonsView();
-  AddVerticalSpacing(kBottomVerticalSpacing);
 
   // Deferred because it needs the pin_view_ pointer.
   InitPasswordView();
 }
 
+AuthDialogContentsView::~AuthDialogContentsView() = default;
+
 void AuthDialogContentsView::AddedToWidget() {
   if (auth_methods_ & kAuthFingerprint) {
     // Inject a callback from the contents view so that we can show retry
@@ -298,21 +307,25 @@ void AuthDialogContentsView::AddedToWidget() {
   }
 }
 
-AuthDialogContentsView::~AuthDialogContentsView() = default;
-
 void AuthDialogContentsView::AddTitleView() {
   title_ = container_->AddChildView(std::make_unique<views::Label>());
   title_->SetEnabledColor(SK_ColorBLACK);
   title_->SetSubpixelRenderingEnabled(false);
   title_->SetAutoColorReadabilityEnabled(false);
+  title_->SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
 
   const gfx::FontList& base_font_list = views::Label::GetDefaultFontList();
 
-  title_->SetFontList(base_font_list.Derive(
-      kTitleFontSize, gfx::Font::FontStyle::NORMAL, gfx::Font::Weight::NORMAL));
+  title_->SetFontList(base_font_list.Derive(kTitleFontSizeDeltaDp,
+                                            gfx::Font::FontStyle::NORMAL,
+                                            gfx::Font::Weight::NORMAL));
   title_->SetText(base::UTF8ToUTF16(kTitle));
   title_->SetMaximumWidth(kContainerPreferredWidth);
   title_->SetElideBehavior(gfx::ElideBehavior::ELIDE_TAIL);
+
+  title_->SetPreferredSize(
+      gfx::Size(kContainerPreferredWidth, title_->height()));
+  title_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
 }
 
 void AuthDialogContentsView::AddPasswordView() {
@@ -367,14 +380,15 @@ void AuthDialogContentsView::AddActionButtonsView() {
   auto* buttons_layout = action_view_container_->SetLayoutManager(
       std::make_unique<views::BoxLayout>(
           views::BoxLayout::Orientation::kHorizontal));
-  buttons_layout->set_between_child_spacing(kButtonSpacing);
+  buttons_layout->set_main_axis_alignment(
+      views::BoxLayout::MainAxisAlignment::kEnd);
 
-  more_options_button_ = AddButton(kMoreOptionsButtonText,
-                                   static_cast<int>(ButtonId::kMoreOptions),
-                                   action_view_container_);
   cancel_button_ =
       AddButton(kCancelButtonText, static_cast<int>(ButtonId::kCancel),
                 action_view_container_);
+
+  action_view_container_->SetPreferredSize(
+      gfx::Size(kContainerPreferredWidth, cancel_button_->height()));
 }
 
 void AuthDialogContentsView::ButtonPressed(views::Button* sender,
@@ -383,9 +397,6 @@ void AuthDialogContentsView::ButtonPressed(views::Button* sender,
     // Cancel() deletes |this|.
     InSessionAuthDialogController::Get()->Cancel();
   }
-
-  // TODO(b/156258540): Enable more options button when we have both fingerprint
-  // view and password input view.
 }
 
 views::LabelButton* AuthDialogContentsView::AddButton(const std::string& text,
@@ -426,4 +437,10 @@ void AuthDialogContentsView::OnFingerprintAuthComplete(
   fingerprint_view_->NotifyFingerprintAuthResult(success);
 }
 
+void AuthDialogContentsView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
+  views::View::GetAccessibleNodeData(node_data);
+  node_data->role = ax::mojom::Role::kDialog;
+  node_data->SetName(base::UTF8ToUTF16(kTitle));
+}
+
 }  // namespace ash

ash/in_session_auth/auth_dialog_contents_view.h

@@ -40,6 +40,9 @@ class AuthDialogContentsView : public views::View,
   AuthDialogContentsView& operator=(const AuthDialogContentsView&) = delete;
   ~AuthDialogContentsView() override;
 
+  // views::Views:
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
+
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
@@ -110,9 +113,6 @@ class AuthDialogContentsView : public views::View,
   // Flags of auth methods that should be visible.
   uint32_t auth_methods_ = 0u;
 
-  // Show other authentication mechanisms if more than one.
-  views::LabelButton* more_options_button_ = nullptr;
-
   // Cancel all operations and close th dialog.
   views::LabelButton* cancel_button_ = nullptr;
 

ash/in_session_auth/in_session_auth_dialog.cc

@@ -5,7 +5,9 @@
 #include "ash/in_session_auth/in_session_auth_dialog.h"
 
 #include "ash/in_session_auth/auth_dialog_contents_view.h"
+#include "ash/public/cpp/rounded_corner_decorator.h"
 #include "base/command_line.h"
+#include "ui/aura/window.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/views/widget/widget.h"
@@ -17,6 +19,7 @@ namespace {
 // The initial height does nothing except determining the vertical position of
 // the dialog, since the dialog is centered with the initial height.
 constexpr gfx::Size kDefaultSize(340, 490);
+constexpr int kCornerRadius = 12;
 
 class AuthDialogWidgetDelegate : public views::WidgetDelegate {
  public:
@@ -65,6 +68,10 @@ InSessionAuthDialog::InSessionAuthDialog(uint32_t auth_methods) {
   bound.set_height(contents_view_->GetPreferredSize().height());
   widget_->SetBounds(bound);
 
+  aura::Window* window = widget_->GetNativeWindow();
+  rounded_corner_decorator_ = std::make_unique<RoundedCornerDecorator>(
+      window, window, window->layer(), kCornerRadius);
+
   widget_->Show();
 }
 

ash/in_session_auth/in_session_auth_dialog.h

@@ -16,6 +16,7 @@ class Widget;
 namespace ash {
 
 class AuthDialogContentsView;
+class RoundedCornerDecorator;
 
 // InSessionAuthDialog gets instantiated on every request to show
 // an authentication dialog, and gets destroyed when the request is
@@ -41,6 +42,7 @@ class InSessionAuthDialog {
   // Pointer to the contents view. Used to query and update the set of available
   // auth methods.
   AuthDialogContentsView* contents_view_ = nullptr;
+  std::unique_ptr<RoundedCornerDecorator> rounded_corner_decorator_;
 };
 
 }  // namespace ash