Commit 10c8b121 authored by Jeroen Dhollander's avatar Jeroen Dhollander Committed by Commit Bot

Dismiss on-screen keyboard when opening the Assistant UI in voice mode.

If the on-screen keyboard is visible and we open the Assistant UI with
"OK Google", the keyboard should be dismissed but it remains visible.

Bug: b/147835595
Change-Id: I00050ce36c28f84c18cc8e6d8af0d157f22c1cd2
Tests: new ash_unittest AssistantPageViewTabletModeTest.ShouldDismissKeyboardWhenOpeningUiInVoiceMode
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2006212
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733037}
parent 1dd7e2a7
...@@ -64,7 +64,7 @@ class AssistantTextfield : public views::Textfield { ...@@ -64,7 +64,7 @@ class AssistantTextfield : public views::Textfield {
}; };
void HideKeyboard() { void HideKeyboard() {
keyboard::KeyboardUIController::Get()->HideKeyboardImplicitlyBySystem(); keyboard::KeyboardUIController::Get()->HideKeyboardImplicitlyByUser();
} }
} // namespace } // namespace
...@@ -248,6 +248,7 @@ void AssistantDialogPlate::OnUiVisibilityChanged( ...@@ -248,6 +248,7 @@ void AssistantDialogPlate::OnUiVisibilityChanged(
base::Optional<AssistantExitPoint> exit_point) { base::Optional<AssistantExitPoint> exit_point) {
if (new_visibility == AssistantVisibility::kVisible) { if (new_visibility == AssistantVisibility::kVisible) {
UpdateModalityVisibility(); UpdateModalityVisibility();
UpdateKeyboardVisibility();
} else { } else {
// When the Assistant UI is no longer visible we need to clear the dialog // When the Assistant UI is no longer visible we need to clear the dialog
// plate so that text does not persist across Assistant launches. // plate so that text does not persist across Assistant launches.
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "ui/events/event.h" #include "ui/events/event.h"
#include "ui/views/controls/textfield/textfield.h" #include "ui/views/controls/textfield/textfield.h"
#include "ui/views/focus/focus_manager.h" #include "ui/views/focus/focus_manager.h"
#include "ui/views/widget/widget.h"
namespace ash { namespace ash {
...@@ -49,6 +50,15 @@ using chromeos::assistant::mojom::AssistantInteractionType; ...@@ -49,6 +50,15 @@ using chromeos::assistant::mojom::AssistantInteractionType;
<< "' should not have the focus (but it does)."; \ << "' should not have the focus (but it does)."; \
}) })
views::View* AddTextfield(views::Widget* widget) {
auto* result = widget->GetContentsView()->AddChildView(
std::make_unique<views::Textfield>());
// Give the text field a non-zero size, otherwise things like tapping on it
// will fail.
result->SetSize(gfx::Size(20, 10));
return result;
}
// Stubbed |FocusChangeListener| that simply remembers all the views that // Stubbed |FocusChangeListener| that simply remembers all the views that
// received focus. // received focus.
class FocusChangeListenerStub : public views::FocusChangeListener { class FocusChangeListenerStub : public views::FocusChangeListener {
...@@ -485,11 +495,26 @@ TEST_F(AssistantPageViewTabletModeTest, ...@@ -485,11 +495,26 @@ TEST_F(AssistantPageViewTabletModeTest,
EXPECT_FALSE(IsKeyboardShowing()); EXPECT_FALSE(IsKeyboardShowing());
} }
TEST_F(AssistantPageViewTabletModeTest,
ShouldDismissKeyboardWhenOpeningUiInVoiceMode) {
// Start by focussing a text field so the system has a reason to show the
// keyboard.
views::Widget* widget = SwitchToNewWidget();
auto* textfield = AddTextfield(widget);
TapOnAndWait(textfield);
ASSERT_TRUE(IsKeyboardShowing());
ShowAssistantUiInVoiceMode();
EXPECT_FALSE(IsKeyboardShowing());
}
TEST_F(AssistantPageViewTabletModeTest, TEST_F(AssistantPageViewTabletModeTest,
ShouldDismissAssistantUiIfLostFocusWhenOtherAppWindowOpens) { ShouldDismissAssistantUiIfLostFocusWhenOtherAppWindowOpens) {
ShowAssistantUi(); ShowAssistantUi();
// Creates a new window to steal the focus should dismiss the Assistant UI. // Create a new window to steal the focus which should dismiss the Assistant
// UI.
SwitchToNewAppWindow(); SwitchToNewAppWindow();
EXPECT_FALSE(IsVisible()); EXPECT_FALSE(IsVisible());
......
...@@ -101,6 +101,7 @@ void AssistantAshTestBase::SetUp() { ...@@ -101,6 +101,7 @@ void AssistantAshTestBase::SetUp() {
void AssistantAshTestBase::TearDown() { void AssistantAshTestBase::TearDown() {
windows_.clear(); windows_.clear();
widgets_.clear();
SetTouchKeyboardEnabled(false); SetTouchKeyboardEnabled(false);
AshTestBase::TearDown(); AshTestBase::TearDown();
scoped_feature_list_.Reset(); scoped_feature_list_.Reset();
...@@ -216,6 +217,16 @@ aura::Window* AssistantAshTestBase::SwitchToNewAppWindow() { ...@@ -216,6 +217,16 @@ aura::Window* AssistantAshTestBase::SwitchToNewAppWindow() {
return window; return window;
} }
views::Widget* AssistantAshTestBase::SwitchToNewWidget() {
widgets_.push_back(CreateTestWidget());
views::Widget* result = widgets_.back().get();
// Give the widget a non-zero size, otherwise things like tapping and clicking
// on it do not work.
result->SetBounds(gfx::Rect(500, 100));
return result;
}
aura::Window* AssistantAshTestBase::window() { aura::Window* AssistantAshTestBase::window() {
return test_api_->window(); return test_api_->window();
} }
......
...@@ -14,9 +14,14 @@ ...@@ -14,9 +14,14 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
namespace aura {
class Window;
} // namespace aura
namespace views { namespace views {
class Textfield; class Textfield;
class View; class View;
class Widget;
} // namespace views } // namespace views
namespace ash { namespace ash {
...@@ -107,12 +112,16 @@ class AssistantAshTestBase : public AshTestBase { ...@@ -107,12 +112,16 @@ class AssistantAshTestBase : public AshTestBase {
base::Optional<chromeos::assistant::mojom::AssistantInteractionMetadata> base::Optional<chromeos::assistant::mojom::AssistantInteractionMetadata>
current_interaction(); current_interaction();
// Create a new App window, and activate it. This will take the focus away // Creates a new App window, and activate it.
// from the Assistant UI (and force it to close).
// Returns a pointer to the newly created window. // Returns a pointer to the newly created window.
// The window will be destroyed when the test if finished. // The window will be destroyed when the test is finished.
aura::Window* SwitchToNewAppWindow(); aura::Window* SwitchToNewAppWindow();
// Creates a new Widget, and activate it.
// Returns a pointer to the newly created widget.
// The widget will be destroyed when the test is finished.
views::Widget* SwitchToNewWidget();
// Return the window containing the Assistant UI. // Return the window containing the Assistant UI.
// Note that this window is shared for all components of the |AppList|. // Note that this window is shared for all components of the |AppList|.
aura::Window* window(); aura::Window* window();
...@@ -151,6 +160,7 @@ class AssistantAshTestBase : public AshTestBase { ...@@ -151,6 +160,7 @@ class AssistantAshTestBase : public AshTestBase {
AssistantController* controller_ = nullptr; AssistantController* controller_ = nullptr;
std::vector<std::unique_ptr<aura::Window>> windows_; std::vector<std::unique_ptr<aura::Window>> windows_;
std::vector<std::unique_ptr<views::Widget>> widgets_;
DISALLOW_COPY_AND_ASSIGN(AssistantAshTestBase); DISALLOW_COPY_AND_ASSIGN(AssistantAshTestBase);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment