Commit e90ce070 authored by David Black's avatar David Black Committed by Commit Bot

Switch to appropriate UI mode when Assistant interaction in progress.

Previously, if a text query were initiated via deeplink and the UI
was visible but not in main/embedded UI mode, the interaction would
not be visible to the user.

Bug: b:113278359
Change-Id: I1f65e0955df688cc08f1b511d09d6bf99a9a55a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1566725
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652691}
parent 5e1e3a35
......@@ -81,7 +81,8 @@ void AssistantPageView::InitLayout() {
AddChildView(assistant_web_view_);
// Update the view state based on the current UI mode.
OnUiModeChanged(assistant_view_delegate_->GetUiModel()->ui_mode());
OnUiModeChanged(assistant_view_delegate_->GetUiModel()->ui_mode(),
/*due_to_interaction=*/false);
}
}
......@@ -178,7 +179,8 @@ views::View* AssistantPageView::GetLastFocusableView() {
this, GetWidget(), /*reverse=*/true, /*dont_loop=*/false);
}
void AssistantPageView::OnUiModeChanged(ash::AssistantUiMode ui_mode) {
void AssistantPageView::OnUiModeChanged(ash::AssistantUiMode ui_mode,
bool due_to_interaction) {
for (auto* child : children())
child->SetVisible(false);
......
......@@ -48,7 +48,8 @@ class APP_LIST_EXPORT AssistantPageView : public AppListPage,
views::View* GetLastFocusableView() override;
// AssistantUiModelObserver:
void OnUiModeChanged(ash::AssistantUiMode ui_mode) override;
void OnUiModeChanged(ash::AssistantUiMode ui_mode,
bool due_to_interaction) override;
void OnUiVisibilityChanged(
ash::AssistantVisibility new_visibility,
ash::AssistantVisibility old_visibility,
......
......@@ -185,7 +185,8 @@ void AssistantInteractionController::OnDeepLinkReceived(
/*query_source=*/AssistantQuerySource::kDeepLink);
}
void AssistantInteractionController::OnUiModeChanged(AssistantUiMode ui_mode) {
void AssistantInteractionController::OnUiModeChanged(AssistantUiMode ui_mode,
bool due_to_interaction) {
if (ui_mode == AssistantUiMode::kMiniUi)
return;
......@@ -193,9 +194,15 @@ void AssistantInteractionController::OnUiModeChanged(AssistantUiMode ui_mode) {
case InputModality::kStylus:
// When the Assistant is not in mini state there should not be an active
// metalayer session. If we were in mini state when the UI mode was
// changed, we need to clean up the metalayer session and reset default
// input modality.
// changed, we need to clean up the metalayer session.
Shell::Get()->highlighter_controller()->AbortSession();
// If the UI mode change was not due to interaction, we should reset the
// default input modality. We don't do this if the change occurred due to
// an Assistant interaction because we might inadvertently stop the active
// interaction by changing input modality. When this is the case, the
// modality will be correctly set in |OnInteractionStarted| if needed.
if (!due_to_interaction)
model_.SetInputModality(InputModality::kKeyboard);
break;
case InputModality::kVoice:
......@@ -371,9 +378,8 @@ void AssistantInteractionController::OnInteractionStarted(
// AssistantInteractionController when beginning an interaction. To address
// this, we temporarily pend an empty text query to commit until we can do
// development to expose something more meaningful.
if (model_.pending_query().type() == AssistantQueryType::kNull) {
if (model_.pending_query().type() == AssistantQueryType::kNull)
model_.SetPendingQuery(std::make_unique<AssistantTextQuery>());
}
model_.CommitPendingQuery();
model_.SetMicState(MicState::kClosed);
......
......@@ -70,7 +70,8 @@ class AssistantInteractionController
void OnCommittedQueryChanged(const AssistantQuery& assistant_query) override;
// AssistantUiModelObserver:
void OnUiModeChanged(AssistantUiMode ui_mode) override;
void OnUiModeChanged(AssistantUiMode ui_mode,
bool due_to_interaction) override;
void OnUiVisibilityChanged(
AssistantVisibility new_visibility,
AssistantVisibility old_visibility,
......
......@@ -122,6 +122,15 @@ void AssistantUiController::OnInteractionStateChanged(
// not already showing. We don't have enough information here to know what
// the interaction source is.
ShowUi(AssistantEntryPoint::kUnspecified);
// We also need to ensure that we're in the appropriate UI mode if we aren't
// already so that the interaction is visible to the user. Note that we
// indicate that this UI mode change is occurring due to an interaction so
// that we won't inadvertently stop the interaction due to the UI mode change.
UpdateUiMode(app_list_features::IsEmbeddedAssistantUIEnabled()
? AssistantUiMode::kLauncherEmbeddedUi
: AssistantUiMode::kMainUi,
/*due_to_interaction=*/true);
}
void AssistantUiController::OnMicStateChanged(MicState mic_state) {
......@@ -428,7 +437,8 @@ void AssistantUiController::ToggleUi(
}
void AssistantUiController::UpdateUiMode(
base::Optional<AssistantUiMode> ui_mode) {
base::Optional<AssistantUiMode> ui_mode,
bool due_to_interaction) {
// If a UI mode is provided, we will use it in lieu of updating UI mode on the
// basis of interaction/widget visibility state.
if (ui_mode.has_value()) {
......@@ -436,12 +446,12 @@ void AssistantUiController::UpdateUiMode(
// TODO(wutao): Behavior is not defined.
if (model_.ui_mode() == AssistantUiMode::kLauncherEmbeddedUi)
DCHECK_NE(AssistantUiMode::kMiniUi, mode);
model_.SetUiMode(mode);
model_.SetUiMode(mode, due_to_interaction);
return;
}
if (app_list_features::IsEmbeddedAssistantUIEnabled()) {
model_.SetUiMode(AssistantUiMode::kLauncherEmbeddedUi);
model_.SetUiMode(AssistantUiMode::kLauncherEmbeddedUi, due_to_interaction);
return;
}
......@@ -453,7 +463,8 @@ void AssistantUiController::UpdateUiMode(
// Otherwise we fall back to main UI mode.
model_.SetUiMode(input_modality == InputModality::kStylus
? AssistantUiMode::kMiniUi
: AssistantUiMode::kMainUi);
: AssistantUiMode::kMainUi,
due_to_interaction);
}
void AssistantUiController::OnKeyboardWorkspaceOccludedBoundsChanged(
......
......@@ -131,8 +131,10 @@ class ASH_EXPORT AssistantUiController
private:
// Updates UI mode to |ui_mode| if specified. Otherwise UI mode is updated on
// the basis of interaction/widget visibility state.
void UpdateUiMode(base::Optional<AssistantUiMode> ui_mode = base::nullopt);
// the basis of interaction/widget visibility state. If |due_to_interaction|
// is true, the UI mode changed because of an Assistant interaction.
void UpdateUiMode(base::Optional<AssistantUiMode> ui_mode = base::nullopt,
bool due_to_interaction = false);
// Calculate and update the usable work area.
void UpdateUsableWorkArea(aura::Window* root_window);
......
......@@ -24,12 +24,13 @@ void AssistantUiModel::RemoveObserver(AssistantUiModelObserver* observer) {
observers_.RemoveObserver(observer);
}
void AssistantUiModel::SetUiMode(AssistantUiMode ui_mode) {
void AssistantUiModel::SetUiMode(AssistantUiMode ui_mode,
bool due_to_interaction) {
if (ui_mode == ui_mode_)
return;
ui_mode_ = ui_mode;
NotifyUiModeChanged();
NotifyUiModeChanged(due_to_interaction);
}
void AssistantUiModel::SetVisible(AssistantEntryPoint entry_point) {
......@@ -78,9 +79,9 @@ void AssistantUiModel::SetVisibility(
NotifyUiVisibilityChanged(old_visibility, entry_point, exit_point);
}
void AssistantUiModel::NotifyUiModeChanged() {
void AssistantUiModel::NotifyUiModeChanged(bool due_to_interaction) {
for (AssistantUiModelObserver& observer : observers_)
observer.OnUiModeChanged(ui_mode_);
observer.OnUiModeChanged(ui_mode_, due_to_interaction);
}
void AssistantUiModel::NotifyUiVisibilityChanged(
......
......@@ -92,8 +92,9 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantUiModel {
void AddObserver(AssistantUiModelObserver* observer);
void RemoveObserver(AssistantUiModelObserver* observer);
// Sets the UI mode.
void SetUiMode(AssistantUiMode ui_mode);
// Sets the UI mode. If |due_to_interaction| is true, the UI mode was changed
// as a result of an Assistant interaction.
void SetUiMode(AssistantUiMode ui_mode, bool due_to_interaction = false);
// Returns the UI mode.
AssistantUiMode ui_mode() const { return ui_mode_; }
......@@ -119,7 +120,7 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantUiModel {
base::Optional<AssistantEntryPoint> entry_point,
base::Optional<AssistantExitPoint> exit_point);
void NotifyUiModeChanged();
void NotifyUiModeChanged(bool due_to_interaction);
void NotifyUiVisibilityChanged(
AssistantVisibility old_visibility,
base::Optional<AssistantEntryPoint> entry_point,
......
......@@ -23,8 +23,10 @@ enum class AssistantVisibility;
class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantUiModelObserver
: public base::CheckedObserver {
public:
// Invoked when the UI mode is changed.
virtual void OnUiModeChanged(AssistantUiMode ui_mode) {}
// Invoked when the UI mode is changed. If |due_to_interaction| is true, the
// UI mode was changed as a result of an Assistant interaction.
virtual void OnUiModeChanged(AssistantUiMode ui_mode,
bool due_to_interaction) {}
// Invoked when the UI visibility is changed from |old_visibility| to
// |new_visibility|. The |source| of the visibility change event is provided
......
......@@ -342,7 +342,8 @@ void AssistantContainerView::Init() {
AddChildView(assistant_web_view_);
// Update the view state based on the current UI mode.
OnUiModeChanged(delegate_->GetUiModel()->ui_mode());
OnUiModeChanged(delegate_->GetUiModel()->ui_mode(),
/*due_to_interaction=*/false);
}
void AssistantContainerView::RequestFocus() {
......@@ -379,7 +380,8 @@ void AssistantContainerView::UpdateAnchor() {
SetArrow(views::BubbleBorder::Arrow::BOTTOM_CENTER);
}
void AssistantContainerView::OnUiModeChanged(AssistantUiMode ui_mode) {
void AssistantContainerView::OnUiModeChanged(AssistantUiMode ui_mode,
bool due_to_interaction) {
for (auto* child : children())
child->SetVisible(false);
......
......@@ -46,7 +46,8 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AssistantContainerView
void RequestFocus() override;
// AssistantUiModelObserver:
void OnUiModeChanged(AssistantUiMode ui_mode) override;
void OnUiModeChanged(AssistantUiMode ui_mode,
bool due_to_interaction) override;
void OnUsableWorkAreaChanged(const gfx::Rect& usable_work_area) override;
// Returns the first focusable view or nullptr to defer to views::FocusSearch.
......
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