Skip to content

Commit

Permalink
Enable autosubmit when input field is complete for the first time.
Browse files Browse the repository at this point in the history 
This CL enables autosubmit of the pac when input field is complete.
It updates the tests to ensure that they don't have to press the
submit button the first time the input field is complete.

Bug: 999389
Change-Id: I287f228de484c466df71f0a9d713d9020e2aee4b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1784938
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Commit-Queue: Yilkal Abe <yilkal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#693936}
  • Loading branch information
yilkal authored and Commit Bot committed Sep 5, 2019
1 parent d7a7e0b commit 5ee603a
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 95 deletions.
7 changes: 6 additions & 1 deletion ash/login/ui/parent_access_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,6 @@ ParentAccessView::ParentAccessView(const AccountId& account_id,
l10n_util::GetStringUTF16(IDS_ASH_LOGIN_SUBMIT_BUTTON_ACCESSIBLE_NAME));
submit_button_->SetFocusBehavior(FocusBehavior::ALWAYS);
footer->AddChildView(submit_button_);

add_spacer(kSubmitButtonBottomMarginDp);

// Pin keyboard is only shown in tablet mode.
Expand Down Expand Up @@ -863,6 +862,12 @@ void ParentAccessView::OnInputChange(bool complete, bool last_field_active) {
submit_button_->SetEnabled(complete);

if (complete && last_field_active) {
if (auto_submit_enabled_) {
auto_submit_enabled_ = false;
SubmitCode();
return;
}

// Moving focus is delayed by using PostTask to allow for proper
// a11y announcements.
base::ThreadTaskRunnerHandle::Get()->PostTask(
Expand Down
3 changes: 3 additions & 0 deletions ash/login/ui/parent_access_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ class ASH_EXPORT ParentAccessView : public views::DialogDelegateView,

State state_ = State::kNormal;

// Auto submit code when the last input has been inserted.
bool auto_submit_enabled_ = true;

views::Label* title_label_ = nullptr;
views::Label* description_label_ = nullptr;
AccessCodeInput* access_code_view_ = nullptr;
Expand Down
199 changes: 105 additions & 94 deletions ash/login/ui/parent_access_view_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,27 @@ class ParentAccessViewTest : public LoginTestBase {
}

// Verifies expectation that UMA |action| was logged.
// Cannot be used when more than one action is reported.
void ExpectUMAActionReported(ParentAccessView::UMAAction action) {
void ExpectUMAActionReported(ParentAccessView::UMAAction action,
int bucket_count,
int total_count) {
histogram_tester_.ExpectBucketCount(
ParentAccessView::kUMAParentAccessCodeAction, action, 1);
ParentAccessView::kUMAParentAccessCodeAction, action, bucket_count);
histogram_tester_.ExpectTotalCount(
ParentAccessView::kUMAParentAccessCodeAction, 1);
ParentAccessView::kUMAParentAccessCodeAction, total_count);
}

void SimulateFailedValidation() {
login_client_->set_validate_parent_access_code_result(false);
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
validation_time_))
.Times(1);

ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_0 + i),
ui::EF_NONE);
base::RunLoop().RunUntilIdle();
}
}

const AccountId account_id_;
Expand Down Expand Up @@ -203,33 +218,55 @@ TEST_F(ParentAccessViewTest, BackButton) {

EXPECT_EQ(1, back_action_);
EXPECT_EQ(0, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kCanceledByUser);
ExpectUMAActionReported(ParentAccessView::UMAAction::kCanceledByUser, 1, 1);
}

// Tests that submit button submits code from code input.
TEST_F(ParentAccessViewTest, SubmitButton) {
// Tests that the code is autosubmitted when input is complete.
TEST_F(ParentAccessViewTest, Autosubmit) {
StartView();
ParentAccessView::TestApi test_api(view_);
EXPECT_FALSE(test_api.submit_button()->GetEnabled());

login_client_->set_validate_parent_access_code_result(true);
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
validation_time_))
.Times(1);

ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_0 + i),
ui::EF_NONE);
base::RunLoop().RunUntilIdle();
}
EXPECT_TRUE(test_api.submit_button()->HasFocus());

EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
1);
}

// Tests that submit button submits code from code input.
TEST_F(ParentAccessViewTest, SubmitButton) {
StartView();
ParentAccessView::TestApi test_api(view_);
EXPECT_FALSE(test_api.submit_button()->GetEnabled());
SimulateFailedValidation();

auto* generator = GetEventGenerator();
// 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());
EXPECT_TRUE(test_api.submit_button()->GetEnabled());

login_client_->set_validate_parent_access_code_result(true);
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012346",
validation_time_))
.Times(1);

SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
2);
}

// Tests that help button opens help app.
Expand All @@ -243,30 +280,27 @@ TEST_F(ParentAccessViewTest, HelpButton) {
EXPECT_CALL(*client, ShowParentAccessHelpApp()).Times(1);
SimulateButtonPress(test_api.help_button());

ExpectUMAActionReported(ParentAccessView::UMAAction::kGetHelp);
ExpectUMAActionReported(ParentAccessView::UMAAction::kGetHelp, 1, 1);
}

// Tests that access code can be entered with numpad.
TEST_F(ParentAccessViewTest, Numpad) {
StartView();
ParentAccessView::TestApi test_api(view_);

login_client_->set_validate_parent_access_code_result(true);
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
validation_time_))
.Times(1);
ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode(ui::VKEY_NUMPAD0 + i), ui::EF_NONE);
base::RunLoop().RunUntilIdle();
}
EXPECT_TRUE(test_api.submit_button()->GetEnabled());

login_client_->set_validate_parent_access_code_result(true);
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
validation_time_))
.Times(1);

SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
1);
}

// Tests that access code can be submitted with press of 'enter' key.
Expand All @@ -275,23 +309,23 @@ TEST_F(ParentAccessViewTest, SubmitWithEnter) {
ParentAccessView::TestApi test_api(view_);
EXPECT_FALSE(test_api.submit_button()->GetEnabled());

ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_0 + i),
ui::EF_NONE);
base::RunLoop().RunUntilIdle();
}
EXPECT_TRUE(test_api.submit_button()->GetEnabled());
SimulateFailedValidation();

// Updating input code (here last digit) should clear error state.
auto* generator = GetEventGenerator();
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_, "012345",
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012346",
validation_time_))
.Times(1);

generator->PressKey(ui::KeyboardCode::VKEY_RETURN, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
2);
}

// Tests that 'enter' key does not submit incomplete code.
Expand All @@ -317,21 +351,30 @@ TEST_F(ParentAccessViewTest, PressEnterOnIncompleteCode) {
base::RunLoop().RunUntilIdle();
EXPECT_EQ(0, successful_validation_);

login_client_->set_validate_parent_access_code_result(false);
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012349",
validation_time_))
.Times(1);

// Fill in last digit of the code.
generator->PressKey(ui::KeyboardCode(ui::KeyboardCode::VKEY_9), ui::EF_NONE);
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(test_api.submit_button()->GetEnabled());

// 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_, "012349",
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012346",
validation_time_))
.Times(1);

// Now the code should be submitted with enter key.
generator->PressKey(ui::KeyboardCode::VKEY_RETURN, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
2);
}

// Tests that backspace button works.
Expand All @@ -340,19 +383,10 @@ TEST_F(ParentAccessViewTest, Backspace) {
ParentAccessView::TestApi test_api(view_);
EXPECT_FALSE(test_api.submit_button()->GetEnabled());

ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode::VKEY_1, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
}
EXPECT_TRUE(test_api.submit_button()->HasFocus());
EXPECT_TRUE(test_api.submit_button()->GetEnabled());
SimulateFailedValidation();
EXPECT_EQ(ParentAccessView::State::kError, test_api.state());

// After access code is completed, focus moves to submit button.
// Move focus back to access code input.
for (int i = 0; i < 2; ++i)
generator->PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_SHIFT_DOWN);
EXPECT_TRUE(HasFocusInAnyChildView(test_api.access_code_view()));
ui::test::EventGenerator* generator = GetEventGenerator();

// Active field has content - backspace clears the content, but does not move
// focus.
Expand All @@ -374,14 +408,15 @@ TEST_F(ParentAccessViewTest, Backspace) {
EXPECT_TRUE(test_api.submit_button()->GetEnabled());

login_client_->set_validate_parent_access_code_result(true);
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "111123",
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012323",
validation_time_))
.Times(1);

SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
2);
}

// Tests input with virtual pin keyboard.
Expand All @@ -393,21 +428,19 @@ TEST_F(ParentAccessViewTest, PinKeyboard) {
LoginPinView::TestApi test_pin_keyboard(test_api.pin_keyboard_view());
EXPECT_FALSE(test_api.submit_button()->GetEnabled());

for (int i = 0; i < 6; ++i) {
SimulatePinKeyboardPress(test_pin_keyboard.GetButton(i));
base::RunLoop().RunUntilIdle();
}
EXPECT_TRUE(test_api.submit_button()->GetEnabled());

login_client_->set_validate_parent_access_code_result(true);
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
validation_time_))
.Times(1);

SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
for (int i = 0; i < 6; ++i) {
SimulatePinKeyboardPress(test_pin_keyboard.GetButton(i));
base::RunLoop().RunUntilIdle();
}
EXPECT_TRUE(test_api.submit_button()->GetEnabled());
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess, 1,
1);
}

// Tests that pin keyboard visibility changes upon tablet mode changes.
Expand All @@ -430,33 +463,15 @@ 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);
base::RunLoop().RunUntilIdle();
}
EXPECT_TRUE(test_api.submit_button()->HasFocus());

// Error should be shown after unsuccessful validation.
login_client_->set_validate_parent_access_code_result(false);
EXPECT_CALL(*login_client_, ValidateParentAccessCode_(account_id_, "012345",
validation_time_))
.Times(1);

SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
SimulateFailedValidation();
EXPECT_EQ(ParentAccessView::State::kError, test_api.state());
EXPECT_EQ(0, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationError);

// After access code is completed, focus moves to submit button.
// Move focus back to access code input.
for (int i = 0; i < 2; ++i)
generator->PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_SHIFT_DOWN);
EXPECT_TRUE(HasFocusInAnyChildView(test_api.access_code_view()));
EXPECT_EQ(0, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationError, 1, 1);

// Updating input code (here last digit) should clear error state.
auto* generator = GetEventGenerator();
generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());

Expand All @@ -469,11 +484,7 @@ TEST_F(ParentAccessViewTest, ErrorState) {
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);

histogram_tester_.ExpectBucketCount(
ParentAccessView::kUMAParentAccessCodeAction,
ParentAccessView::UMAAction::kValidationError, 1);
histogram_tester_.ExpectTotalCount(
ParentAccessView::kUMAParentAccessCodeAction, 2);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationError, 1, 2);
}

// Tests children views traversal with tab key.
Expand All @@ -482,12 +493,13 @@ TEST_F(ParentAccessViewTest, TabKeyTraversal) {
ParentAccessView::TestApi test_api(view_);
EXPECT_TRUE(HasFocusInAnyChildView(test_api.access_code_view()));

// Enter access code, so submit button is enabled and focused.
ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode::VKEY_0, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
}
SimulateFailedValidation();

// Updating input code (here last digit) should clear error state.
auto* generator = GetEventGenerator();
generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
EXPECT_TRUE(test_api.submit_button()->HasFocus());

generator->PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_NONE);
Expand All @@ -512,12 +524,11 @@ TEST_F(ParentAccessViewTest, BackwardTabKeyTraversal) {
ParentAccessView::TestApi test_api(view_);
EXPECT_TRUE(HasFocusInAnyChildView(test_api.access_code_view()));

// Enter access code, so submit button is enabled and focusable.
ui::test::EventGenerator* generator = GetEventGenerator();
for (int i = 0; i < 6; ++i) {
generator->PressKey(ui::KeyboardCode::VKEY_0, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
}
SimulateFailedValidation();
auto* generator = GetEventGenerator();
generator->PressKey(ui::KeyboardCode::VKEY_6, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(ParentAccessView::State::kNormal, test_api.state());
EXPECT_TRUE(test_api.submit_button()->HasFocus());

generator->PressKey(ui::KeyboardCode::VKEY_TAB, ui::EF_SHIFT_DOWN);
Expand Down

0 comments on commit 5ee603a

Please sign in to comment.