Commit 71fd9c32 authored by Jeroen Dhollander's avatar Jeroen Dhollander Committed by Commit Bot

Fix lost focus when resizing embedded Assistant size.

When the embedded Assistant is initially shown, it uses a small window.
When a big response arrives, its size is increased (to what is called
|ash::AppListViewState::kHalf|).  When this happened, the focus was lost
from the input text box (and moved to the - at this point not visible -
launcher search box).

This is fixed by updating the |AppListView| to not move the focus to the
launcher search box if the embedded Assistant UI is showing.

I also introduced a new |AssistantTextfield| which is used for inputting
text based queries. This makes things like logs and stacktraces easier
to parse as they will now mention |AssistantTextfield| instead of just
|Textfield| which is used everywhere.

test: Added unittests and manually verified.
bug: b/143715841
Change-Id: I4da2b363ec6c93b5bb166b415e45c60c08206810
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896047
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711916}
parent ff02ecdc
...@@ -863,7 +863,7 @@ void AppListView::HandleClickOrTap(ui::LocatedEvent* event) { ...@@ -863,7 +863,7 @@ void AppListView::HandleClickOrTap(ui::LocatedEvent* event) {
} }
// Close embedded Assistant UI if it is shown. // Close embedded Assistant UI if it is shown.
if (app_list_main_view()->contents_view()->IsShowingEmbeddedAssistantUI()) { if (IsShowingEmbeddedAssistantUI()) {
Back(); Back();
search_box_view_->ClearSearchAndDeactivateSearchBox(); search_box_view_->ClearSearchAndDeactivateSearchBox();
return; return;
...@@ -1532,7 +1532,7 @@ void AppListView::SetState(ash::AppListViewState new_state) { ...@@ -1532,7 +1532,7 @@ void AppListView::SetState(ash::AppListViewState new_state) {
if (is_in_drag_ && app_list_state_ != ash::AppListViewState::kClosed) if (is_in_drag_ && app_list_state_ != ash::AppListViewState::kClosed)
app_list_main_view_->contents_view()->UpdateYPositionAndOpacity(); app_list_main_view_->contents_view()->UpdateYPositionAndOpacity();
if (GetWidget()->IsActive()) { if (GetWidget()->IsActive() && !IsShowingEmbeddedAssistantUI()) {
// Reset the focus to initially focused view. This should be // Reset the focus to initially focused view. This should be
// done before updating visibility of views, because setting // done before updating visibility of views, because setting
// focused view invisible automatically moves focus to next // focused view invisible automatically moves focus to next
...@@ -2037,7 +2037,7 @@ void AppListView::RedirectKeyEventToSearchBox(ui::KeyEvent* event) { ...@@ -2037,7 +2037,7 @@ void AppListView::RedirectKeyEventToSearchBox(ui::KeyEvent* event) {
return; return;
// Allow text input inside the Assistant page. // Allow text input inside the Assistant page.
if (app_list_main_view()->contents_view()->IsShowingEmbeddedAssistantUI()) if (IsShowingEmbeddedAssistantUI())
return; return;
views::Textfield* search_box = search_box_view_->search_box(); views::Textfield* search_box = search_box_view_->search_box();
...@@ -2209,6 +2209,10 @@ bool AppListView::ShouldIgnoreScrollEvents() { ...@@ -2209,6 +2209,10 @@ bool AppListView::ShouldIgnoreScrollEvents() {
GetRootAppsGridView()->pagination_model()->has_transition(); GetRootAppsGridView()->pagination_model()->has_transition();
} }
bool AppListView::IsShowingEmbeddedAssistantUI() const {
return app_list_main_view()->contents_view()->IsShowingEmbeddedAssistantUI();
}
int AppListView::GetPreferredWidgetYForState( int AppListView::GetPreferredWidgetYForState(
ash::AppListViewState state) const { ash::AppListViewState state) const {
// Note that app list container fills the screen, so we can treat the // Note that app list container fills the screen, so we can treat the
......
...@@ -464,6 +464,9 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView, ...@@ -464,6 +464,9 @@ class APP_LIST_EXPORT AppListView : public views::WidgetDelegateView,
// Returns true if scroll events should be ignored. // Returns true if scroll events should be ignored.
bool ShouldIgnoreScrollEvents(); bool ShouldIgnoreScrollEvents();
// Returns true if the Embedded Assistant UI is currently being shown.
bool IsShowingEmbeddedAssistantUI() const;
// Returns preferred y of fullscreen widget bounds in parent window for the // Returns preferred y of fullscreen widget bounds in parent window for the
// specified state. // specified state.
int GetPreferredWidgetYForState(ash::AppListViewState state) const; int GetPreferredWidgetYForState(ash::AppListViewState state) const;
......
...@@ -52,6 +52,17 @@ constexpr base::TimeDelta kAnimationTransformInDuration = ...@@ -52,6 +52,17 @@ constexpr base::TimeDelta kAnimationTransformInDuration =
base::TimeDelta::FromMilliseconds(333); base::TimeDelta::FromMilliseconds(333);
constexpr int kAnimationTranslationDip = 30; constexpr int kAnimationTranslationDip = 30;
// Textfield used for inputting text based Assistant queries.
class AssistantTextfield : public views::Textfield {
public:
AssistantTextfield() : views::Textfield() {
SetID(AssistantViewID::kTextQueryField);
}
// views::Textfield overrides:
const char* GetClassName() const override { return "AssistantTextfield"; }
};
} // namespace } // namespace
// AssistantDialogPlate -------------------------------------------------------- // AssistantDialogPlate --------------------------------------------------------
...@@ -312,8 +323,7 @@ void AssistantDialogPlate::InitKeyboardLayoutContainer() { ...@@ -312,8 +323,7 @@ void AssistantDialogPlate::InitKeyboardLayoutContainer() {
ash::assistant::ui::GetDefaultFontList().DeriveWithSizeDelta(2); ash::assistant::ui::GetDefaultFontList().DeriveWithSizeDelta(2);
// Textfield. // Textfield.
textfield_ = new views::Textfield(); textfield_ = new AssistantTextfield();
textfield_->SetID(AssistantViewID::kTextQueryField);
textfield_->SetBackgroundColor(SK_ColorTRANSPARENT); textfield_->SetBackgroundColor(SK_ColorTRANSPARENT);
textfield_->SetBorder(views::NullBorder()); textfield_->SetBorder(views::NullBorder());
textfield_->set_controller(this); textfield_->set_controller(this);
......
...@@ -133,7 +133,7 @@ TEST_F(AssistantPageViewTest, ShouldFocusTextDialogWhenOpeningWithHotkey) { ...@@ -133,7 +133,7 @@ TEST_F(AssistantPageViewTest, ShouldFocusTextDialogWhenOpeningWithHotkey) {
EXPECT_HAS_FOCUS(input_text_field()); EXPECT_HAS_FOCUS(input_text_field());
} }
TEST_F(AssistantPageViewTest, ShouldFocusTextDialogAfterSendingQuery) { TEST_F(AssistantPageViewTest, ShouldNotLoseTextfieldFocusWhenSendingTextQuery) {
ShowAssistantUi(); ShowAssistantUi();
SendQueryThroughTextField("The query"); SendQueryThroughTextField("The query");
...@@ -141,6 +141,25 @@ TEST_F(AssistantPageViewTest, ShouldFocusTextDialogAfterSendingQuery) { ...@@ -141,6 +141,25 @@ TEST_F(AssistantPageViewTest, ShouldFocusTextDialogAfterSendingQuery) {
EXPECT_HAS_FOCUS(input_text_field()); EXPECT_HAS_FOCUS(input_text_field());
} }
TEST_F(AssistantPageViewTest,
ShouldNotLoseTextfieldFocusWhenDisplayingResponse) {
ShowAssistantUi();
MockAssistantInteractionWithResponse("The response");
EXPECT_HAS_FOCUS(input_text_field());
}
TEST_F(AssistantPageViewTest, ShouldNotLoseTextfieldFocusWhenResizing) {
ShowAssistantUi();
MockAssistantInteractionWithResponse(
"This\ntext\nis\nbig\nenough\nto\ncause\nthe\nassistant\nscreen\nto\n"
"resize.");
EXPECT_HAS_FOCUS(input_text_field());
}
TEST_F(AssistantPageViewTest, ShouldFocusMicWhenOpeningWithHotword) { TEST_F(AssistantPageViewTest, ShouldFocusMicWhenOpeningWithHotword) {
ShowAssistantUi(AssistantEntryPoint::kHotword); ShowAssistantUi(AssistantEntryPoint::kHotword);
......
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