Commit 2117c90b authored by David Black's avatar David Black Committed by Commit Bot

Maybe update app list state when showing Assistant.

Previously, AssistantPageView would not update app list state when being
shown. With the Better Onboarding feature, AssistantPageView is too tall
to render in peeking state w/o being clipped off screen.

Now, AssistantPageView will check if it needs to update app list state
when being shown. This should have no effect on existing behavior as
only the Better Onboarding feature should cause this to happen. The
Better Onboarding feature is disabled by default.

Bug: b:157689497
Change-Id: I662eea9c181674d2af88790ac86daa6cd24a4f65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225513Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#774762}
parent 52384450
......@@ -184,7 +184,12 @@ void AssistantPageView::OnGestureEvent(ui::GestureEvent* event) {
void AssistantPageView::OnShown() {
// The preferred size might be different from the previous time, so updating
// to the correct size here.
SetSize(CalculatePreferredSize());
const gfx::Size size = CalculatePreferredSize();
SetSize(size);
// Our |size| may require a change in app list state in order to ensure that
// the AssistantPageView renders fully on screen w/o being clipped.
MaybeUpdateAppListState(size.height());
}
void AssistantPageView::OnAnimationStarted(AppListState from_state,
......@@ -301,13 +306,14 @@ int AssistantPageView::GetChildViewHeightForWidth(int width) const {
void AssistantPageView::MaybeUpdateAppListState(int child_height) {
auto* app_list_view = contents_view_->app_list_view();
auto* widget = app_list_view->GetWidget();
// |app_list_view| may not be initialized.
if (!widget || !widget->IsVisible())
return;
// Update app list view state for |assistant_page_view_|.
// Embedded Assistant Ui only has two sizes. The only states change is from
// kPeeking to kHalf state.
// Embedded Assistant Ui only has two sizes. The only state change is from
// |kPeeking| to |kHalf| state.
if (app_list_view->app_list_state() != AppListViewState::kPeeking)
return;
......
......@@ -2,10 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "ash/app_list/views/app_list_view.h"
#include "ash/assistant/model/assistant_ui_model.h"
#include "ash/assistant/test/assistant_ash_test_base.h"
#include "ash/assistant/ui/assistant_ui_constants.h"
#include "ash/assistant/ui/main_stage/suggestion_chip_view.h"
#include "ash/public/cpp/app_list/app_list_types.h"
#include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
......@@ -234,6 +236,26 @@ class AssistantInteractionCounter
} // namespace
TEST_F(AssistantPageViewTest, ShouldStartInPeekingState) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(
chromeos::assistant::features::kAssistantBetterOnboarding);
ShowAssistantUi();
EXPECT_EQ(AppListViewState::kPeeking, app_list_view()->app_list_state());
}
TEST_F(AssistantPageViewTest, ShouldStartInHalfState) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(
chromeos::assistant::features::kAssistantBetterOnboarding);
ShowAssistantUi();
EXPECT_EQ(AppListViewState::kHalf, app_list_view()->app_list_state());
}
TEST_F(AssistantPageViewTest, ShouldStartAtMinimumHeight) {
ShowAssistantUi();
......
......@@ -108,8 +108,8 @@ aura::Window* AssistantTestApiImpl::window() {
return main_view()->GetWidget()->GetNativeWindow();
}
views::View* AssistantTestApiImpl::app_list_view() {
return static_cast<views::View*>(contents_view()->app_list_view());
AppListView* AssistantTestApiImpl::app_list_view() {
return contents_view()->app_list_view();
}
aura::Window* AssistantTestApiImpl::root_window() {
......
......@@ -49,7 +49,7 @@ class AssistantTestApiImpl : public AssistantTestApi {
views::View* onboarding_view() override;
views::View* opt_in_view() override;
aura::Window* window() override;
views::View* app_list_view() override;
AppListView* app_list_view() override;
aura::Window* root_window() override;
private:
......
......@@ -195,7 +195,7 @@ views::View* AssistantAshTestBase::page_view() {
return test_api_->page_view();
}
views::View* AssistantAshTestBase::app_list_view() {
AppListView* AssistantAshTestBase::app_list_view() {
return test_api_->app_list_view();
}
......
......@@ -27,6 +27,7 @@ class Widget;
namespace ash {
class AppListView;
class AssistantTestApi;
class SuggestionChipView;
class TestAssistantClient;
......@@ -87,7 +88,7 @@ class AssistantAshTestBase : public AshTestBase {
// Return the app list view hosting the Assistant page view.
// Can only be used after |ShowAssistantUi| has been called.
views::View* app_list_view();
AppListView* app_list_view();
// Return the root view hosting the Assistant page view.
// Can only be used after |ShowAssistantUi| has been called.
......
......@@ -22,6 +22,7 @@ class View;
namespace ash {
class AppListView;
class AssistantState;
// Public test API for the Assistant UI.
......@@ -120,7 +121,7 @@ class ASH_EXPORT AssistantTestApi {
// Returns the app list view hosting the Assistant UI.
// Can only be used after the Assistant UI has been shown at least once.
virtual views::View* app_list_view() = 0;
virtual AppListView* app_list_view() = 0;
// Returns the root window containing the Assistant UI (and the Ash shell).
// This can be used even when the Assistant UI has never been shown.
......
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