child user: Show error state on ParentAccessView

Error state is shown after unsuccessful access code validation
and cleared when user changes input (deletes or updates input
field content).

Backspace behavior was updated accordingly. Pressing backspace
when input field has text deletes the text but does not move
focus to the previous field. Pressing backspace on empty input
field moves focus to the previous field.

Bug: 911326
Test: ParentAccessViewTest
Change-Id: I2860c0d02dee6f7aba95fcdaaa3271b98a25c07c
Reviewed-on: https://chromium-review.googlesource.com/c/1477878
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635751}

ash/ash_strings.grd

@@ -1583,6 +1583,9 @@ This file contains the strings for ash.
       <message name="IDS_ASH_LOGIN_PARENT_ACCESS_TITLE" desc="Title of the parent access input dialog that allows to unlock child's device with a code.">
         Unlock device with parent code
       </message>
+      <message name="IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_ERROR" desc="Title of the parent access input dialog after incorrect code was entered.">
+        Incorrect parent code
+      </message>
       <message name="IDS_ASH_LOGIN_PARENT_ACCESS_DESCRIPTION" desc="Description shown on the parent access input dialog. Explains what code should be used to unlock the device.">
         To unlock the device, enter your Family Link parent access code
       </message>

ash/ash_strings_grd/IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_ERROR.png.sha1

+2094e9c0edd7f57dbf9e86ccc84373a917bd5d94
\ No newline at end of file

ash/login/ui/parent_access_view.cc

@@ -74,6 +74,7 @@ constexpr int kArrowButtonSizeDp = 40;
 constexpr int kArrowSizeDp = 20;
 
 constexpr SkColor kTextColor = SK_ColorWHITE;
+constexpr SkColor kErrorColor = SkColorSetARGB(0xFF, 0xF2, 0x8B, 0x82);
 constexpr SkColor kArrowButtonColor = SkColorSetARGB(0x57, 0xFF, 0xFF, 0xFF);
 
 }  // namespace
@@ -83,16 +84,16 @@ constexpr SkColor kArrowButtonColor = SkColorSetARGB(0x57, 0xFF, 0xFF, 0xFF);
 class ParentAccessView::AccessCodeInput : public views::View,
                                           public views::TextfieldController {
  public:
-  using OnCodeComplete = base::RepeatingCallback<void(bool complete)>;
+  using OnInputChange = base::RepeatingCallback<void(bool complete)>;
 
   // Builds the view for an access code that consists out of |length| digits.
-  // |on_code_complete| will be called when code completion state changes. True
-  // will be passed if the current code is complate (all digits have input
-  // value) and false otherwise.
-  AccessCodeInput(int length, const OnCodeComplete& on_code_complete)
-      : on_code_complete_(on_code_complete) {
+  // |on_input_change| will be called upon access code digit insertion, deletion
+  // or change. True will be passed if the current code is complete (all digits
+  // have input values) and false otherwise.
+  AccessCodeInput(int length, OnInputChange on_input_change)
+      : on_input_change_(std::move(on_input_change)) {
     DCHECK_LT(0, length);
-    DCHECK(on_code_complete_);
+    DCHECK(on_input_change_);
 
     SetLayoutManager(std::make_unique<views::BoxLayout>(
         views::BoxLayout::kHorizontal, gfx::Insets(),
@@ -132,16 +133,18 @@ class ParentAccessView::AccessCodeInput : public views::View,
     ActiveField()->SetText(base::IntToString16(value));
     FocusNextField();
 
-    if (GetCode().has_value())
-      on_code_complete_.Run(true);
+    on_input_change_.Run(GetCode().has_value());
   }
 
-  // Clears value from the |active_field_| and moves focus to the previous field
-  // if it exists.
+  // Clears input from the |active_field_|. If |active_field| is empty moves
+  // focus to the previous field (if exists) and clears input there.
   void Backspace() {
+    if (ActiveInput().empty()) {
+      FocusPreviousField();
+    }
+
     ActiveField()->SetText(base::string16());
-    FocusPreviousField();
-    on_code_complete_.Run(false);
+    on_input_change_.Run(false);
   }
 
   // Returns access code as string if all fields contain input.
@@ -159,6 +162,13 @@ class ParentAccessView::AccessCodeInput : public views::View,
     return result;
   }
 
+  // Sets the color of the input text.
+  void SetInputColor(SkColor color) {
+    for (auto* field : input_fields_) {
+      field->SetTextColor(color);
+    }
+  }
+
   // views::View:
   void RequestFocus() override { ActiveField()->RequestFocus(); }
 
@@ -215,9 +225,10 @@ class ParentAccessView::AccessCodeInput : public views::View,
   // Returns text in the active input field.
   const base::string16& ActiveInput() const { return ActiveField()->text(); }
 
-  // To be called when access code completion state changes. Passes true when
-  // code is complete (all digits have input value) and false otherwise.
-  OnCodeComplete on_code_complete_;
+  // To be called when access input code changes (digit is inserted, deleted or
+  // updated). Passes true when code is complete (all digits have input value)
+  // and false otherwise.
+  OnInputChange on_input_change_;
 
   // An active/focused input field index. Incoming digit will be inserted here.
   int active_input_index_ = 0;
@@ -245,6 +256,10 @@ LoginPinView* ParentAccessView::TestApi::pin_keyboard_view() const {
   return view_->pin_keyboard_view_;
 }
 
+ParentAccessView::State ParentAccessView::TestApi::state() const {
+  return view_->state_;
+}
+
 ParentAccessView::Callbacks::Callbacks() = default;
 
 ParentAccessView::Callbacks::Callbacks(const Callbacks& other) = default;
@@ -344,7 +359,7 @@ ParentAccessView::ParentAccessView(const AccountId& account_id,
   // Access code input view.
   access_code_view_ =
       new AccessCodeInput(kParentAccessCodePinLength,
-                          base::BindRepeating(&ParentAccessView::OnCodeComplete,
+                          base::BindRepeating(&ParentAccessView::OnInputChange,
                                               base::Unretained(this)));
   AddChildView(access_code_view_);
 
@@ -435,6 +450,29 @@ void ParentAccessView::SubmitCode() {
                      weak_ptr_factory_.GetWeakPtr()));
 }
 
+void ParentAccessView::UpdateState(State state) {
+  if (state_ == state)
+    return;
+
+  state_ = state;
+  switch (state_) {
+    case State::kNormal: {
+      access_code_view_->SetInputColor(kTextColor);
+      title_label_->SetEnabledColor(kTextColor);
+      title_label_->SetText(
+          l10n_util::GetStringUTF16(IDS_ASH_LOGIN_PARENT_ACCESS_TITLE));
+      return;
+    }
+    case State::kError: {
+      access_code_view_->SetInputColor(kErrorColor);
+      title_label_->SetEnabledColor(kErrorColor);
+      title_label_->SetText(
+          l10n_util::GetStringUTF16(IDS_ASH_LOGIN_PARENT_ACCESS_TITLE_ERROR));
+      return;
+    }
+  }
+}
+
 void ParentAccessView::OnValidationResult(base::Optional<bool> result) {
   if (result.has_value() && *result) {
     VLOG(1) << "Parent access code successfully validated";
@@ -443,10 +481,13 @@ void ParentAccessView::OnValidationResult(base::Optional<bool> result) {
   }
 
   VLOG(1) << "Invalid parent access code entered";
-  // TODO(agawronska): Show error.
+  UpdateState(State::kError);
 }
 
-void ParentAccessView::OnCodeComplete(bool complete) {
+void ParentAccessView::OnInputChange(bool complete) {
+  if (state_ == State::kError)
+    UpdateState(State::kNormal);
+
   submit_button_->SetEnabled(complete);
 }
 

ash/login/ui/parent_access_view.h

@@ -31,6 +31,12 @@ class LoginPinView;
 class ASH_EXPORT ParentAccessView : public NonAccessibleView,
                                     public views::ButtonListener {
  public:
+  // ParentAccessView state.
+  enum class State {
+    kNormal,  // View with default texts and colors.
+    kError    // View with texts and color signalizing input error.
+  };
+
   class ASH_EXPORT TestApi {
    public:
     explicit TestApi(ParentAccessView* view);
@@ -40,6 +46,8 @@ class ASH_EXPORT ParentAccessView : public NonAccessibleView,
     ArrowButtonView* submit_button() const;
     LoginPinView* pin_keyboard_view() const;
 
+    State state() const;
+
    private:
     ParentAccessView* const view_;
   };
@@ -78,9 +86,12 @@ class ASH_EXPORT ParentAccessView : public NonAccessibleView,
   // Submits access code for validation.
   void SubmitCode();
 
-  // Called when completion state of |access_code_| changes. |complete| brings
-  // information whether current input code is complete.
-  void OnCodeComplete(bool complete);
+  // Updates state of the view.
+  void UpdateState(State state);
+
+  // Called when access code input changes. |complete| brings information
+  // whether current input code is complete.
+  void OnInputChange(bool complete);
 
   // To be called when parent access code validation was completed. Result of
   // the validation is available in |result| if validation was performed.
@@ -92,6 +103,8 @@ class ASH_EXPORT ParentAccessView : public NonAccessibleView,
   // Account id of the user that parent access code is processed for.
   const AccountId account_id_;
 
+  State state_ = State::kNormal;
+
   views::Label* title_label_ = nullptr;
   views::Label* description_label_ = nullptr;
   AccessCodeInput* access_code_view_ = nullptr;

ash/login/ui/parent_access_view_unittest.cc

@@ -148,7 +148,12 @@ TEST_F(ParentAccessViewTest, Backspace) {
     generator->PressKey(ui::KeyboardCode::VKEY_1, ui::EF_NONE);
   EXPECT_TRUE(test_api.submit_button()->enabled());
 
-  // Clear last field - will move focus to before last field.
+  // Active field has content - backspace clears the content, but does not move
+  // focus.
+  generator->PressKey(ui::KeyboardCode::VKEY_BACK, ui::EF_NONE);
+  EXPECT_FALSE(test_api.submit_button()->enabled());
+
+  // Active Field is empty - backspace moves focus to before last field.
   generator->PressKey(ui::KeyboardCode::VKEY_BACK, ui::EF_NONE);
   EXPECT_FALSE(test_api.submit_button()->enabled());
 
@@ -190,4 +195,40 @@ TEST_F(ParentAccessViewTest, PinKeyboard) {
   EXPECT_EQ(1, successful_validation_);
 }
 
+// Tests that error state is shown and cleared when neccesary.
+TEST_F(ParentAccessViewTest, ErrorState) {
+  ParentAccessView::TestApi test_api(view_);
+  EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
+
+  ui::test::EventGenerator* generator = GetEventGenerator();
+  for (int i = 0; i < 6; ++i) {
+    generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_0 + i),
+                        ui::EF_NONE);
+  }
+
+  // Error should be shown after unsuccessful validation.
+  login_client_->set_validate_parent_access_code_result(false);
+  EXPECT_CALL(*login_client_,
+              ValidateParentAccessCode_(account_id_, "012345", testing::_))
+      .Times(1);
+
+  SimulateButtonPress(test_api.submit_button());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(ParentAccessView::State::kError, test_api.state());
+  EXPECT_EQ(0, successful_validation_);
+
+  // Updating input code (here last digit) should clear error state.
+  generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
+  EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
+
+  login_client_->set_validate_parent_access_code_result(true);
+  EXPECT_CALL(*login_client_,
+              ValidateParentAccessCode_(account_id_, "012346", testing::_))
+      .Times(1);
+
+  SimulateButtonPress(test_api.submit_button());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, successful_validation_);
+}
+
 }  // namespace ash