Skip to content

Commit

Permalink
QA Rich Card: handle view dismissal
Browse files Browse the repository at this point in the history
Add logic to handle dismissal of the Quick Answers rich card view.
Currently only used to dismiss the rich card after clicking on the
settings button. Also update Quick Answers visibility states.

Will update later to include rich-answer-specific exit point data as
we add more dismissal cases.

Tests: tested on DUT
Bug: b/265211938
Change-Id: Ib7827fe749f4bbcf39be33ea8da5f2bc62abb875
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4327770
Reviewed-by: Yuki Awano <yawano@google.com>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Angela Xiao <angelaxiao@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1116003}
  • Loading branch information
Angela Xiao authored and Chromium LUCI CQ committed Mar 11, 2023
1 parent a0a70a1 commit f8acf4c
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void QuickAnswersMenuObserver::OnMenuClosed() {

QuickAnswersController::Get()->DismissQuickAnswers(
is_other_command_executed_ ? QuickAnswersExitPoint::kContextMenuClick
: QuickAnswersExitPoint::KContextMenuDismiss);
: QuickAnswersExitPoint::kContextMenuDismiss);
}

void QuickAnswersMenuObserver::CommandWillBeExecuted(int command_id) {
Expand Down
73 changes: 50 additions & 23 deletions chrome/browser/ui/quick_answers/quick_answers_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,9 @@ void QuickAnswersControllerImpl::MaybeShowQuickAnswers(
if (!ShouldShowQuickAnswers())
return;

if (visibility_ == QuickAnswersVisibility::kClosed)
if (visibility_ != QuickAnswersVisibility::kPending) {
return;
}

// Cache anchor-bounds and query.
anchor_bounds_ = anchor_bounds;
Expand Down Expand Up @@ -124,7 +125,7 @@ void QuickAnswersControllerImpl::HandleQuickAnswerRequest(
IntentTypeToString(request.preprocessed_output.intent_info.intent_type),
base::UTF8ToUTF16(request.preprocessed_output.intent_info.intent_text));
} else {
visibility_ = QuickAnswersVisibility::kVisible;
visibility_ = QuickAnswersVisibility::kQuickAnswersVisible;
quick_answers_ui_controller_->CreateQuickAnswersView(
anchor_bounds_, title_, query_,
request.context.device_properties.is_internal);
Expand All @@ -138,25 +139,44 @@ void QuickAnswersControllerImpl::HandleQuickAnswerRequest(

void QuickAnswersControllerImpl::DismissQuickAnswers(
QuickAnswersExitPoint exit_point) {
visibility_ = QuickAnswersVisibility::kClosed;
MaybeDismissQuickAnswersConsent();
bool closed = quick_answers_ui_controller_->CloseQuickAnswersView();
// |quick_answer_| could be null before we receive the result from the server.
// Do not send the signal since the quick answer is dismissed before ready.
if (quick_answer_) {
// For quick-answer rendered along with browser context menu, if user didn't
// click on other context menu items, it is considered as active impression.
bool is_active = exit_point != QuickAnswersExitPoint::kContextMenuClick;
quick_answers_client_->OnQuickAnswersDismissed(quick_answer_->result_type,
is_active && closed);

// Record Quick Answers exit point.
// Make sure |closed| is true so that only the direct exit point is recorded
// when multiple dissmiss requests are received (For example, dissmiss
// request from context menu will also fire when the settings button is
// pressed).
if (closed)
base::UmaHistogramEnumeration(kQuickAnswersExitPoint, exit_point);
switch (visibility_) {
case QuickAnswersVisibility::kRichAnswersVisible: {
// For the rich-answers view, ignore dismissal by context-menu related
// actions as they should only affect the companion quick-answers view.
if (exit_point == QuickAnswersExitPoint::kContextMenuDismiss ||
exit_point == QuickAnswersExitPoint::kContextMenuClick) {
return;
}
quick_answers_ui_controller_->CloseRichAnswersView();
visibility_ = QuickAnswersVisibility::kClosed;
return;
}
default: {
visibility_ = QuickAnswersVisibility::kClosed;
MaybeDismissQuickAnswersConsent();
bool closed = quick_answers_ui_controller_->CloseQuickAnswersView();
// |quick_answer_| could be null before we receive the result from the
// server. Do not send the signal since the quick answer is dismissed
// before ready.
if (quick_answer_) {
// For quick-answer rendered along with browser context menu, if user
// didn't click on other context menu items, it is considered as active
// impression.
bool is_active = exit_point != QuickAnswersExitPoint::kContextMenuClick;
quick_answers_client_->OnQuickAnswersDismissed(
quick_answer_->result_type, is_active && closed);

// Record Quick Answers exit point.
// Make sure |closed| is true so that only the direct exit point is
// recorded when multiple dismiss requests are received (For example,
// dismiss request from context menu will also fire when the settings
// button is pressed).
if (closed) {
base::UmaHistogramEnumeration(kQuickAnswersExitPoint, exit_point);
}
}
return;
}
}
}

Expand All @@ -170,10 +190,16 @@ QuickAnswersVisibility QuickAnswersControllerImpl::GetVisibilityForTesting()
return visibility_;
}

void QuickAnswersControllerImpl::SetVisibility(
QuickAnswersVisibility visibility) {
visibility_ = visibility;
}

void QuickAnswersControllerImpl::OnQuickAnswerReceived(
std::unique_ptr<QuickAnswer> quick_answer) {
if (visibility_ != QuickAnswersVisibility::kVisible)
if (visibility_ != QuickAnswersVisibility::kQuickAnswersVisible) {
return;
}

if (quick_answer) {
if (quick_answer->title.empty()) {
Expand All @@ -200,8 +226,9 @@ void QuickAnswersControllerImpl::OnQuickAnswerReceived(
}

void QuickAnswersControllerImpl::OnNetworkError() {
if (visibility_ != QuickAnswersVisibility::kVisible)
if (visibility_ != QuickAnswersVisibility::kQuickAnswersVisible) {
return;
}

// Notify quick_answers_ui_controller_ to show retry UI.
quick_answers_ui_controller_->ShowRetry();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ class QuickAnswersControllerImpl : public QuickAnswersController,

QuickAnswersVisibility GetVisibilityForTesting() const override;

void SetVisibility(QuickAnswersVisibility visibility) override;

// QuickAnswersDelegate:
void OnQuickAnswerReceived(
std::unique_ptr<quick_answers::QuickAnswer> answer) override;
Expand Down
24 changes: 20 additions & 4 deletions chrome/browser/ui/quick_answers/quick_answers_ui_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,19 +99,22 @@ void QuickAnswersUiController::CreateQuickAnswersView(const gfx::Rect& bounds,
}

void QuickAnswersUiController::OnQuickAnswersViewPressed() {
// Route dismissal through |controller_| for logging impressions.
controller_->DismissQuickAnswers(QuickAnswersExitPoint::kQuickAnswersClick);

if (chromeos::features::IsQuickAnswersRichCardEnabled()) {
auto* const rich_answers_view =
new RichAnswersView(quick_answers_view_tracker_.view()->bounds(),
weak_factory_.GetWeakPtr());
rich_answers_view_tracker_.SetView(rich_answers_view);
rich_answers_view->GetWidget()->ShowInactive();
controller_->DismissQuickAnswers(QuickAnswersExitPoint::kQuickAnswersClick);

// Temporarily set rich-answers visibility here. Will move after
// setting up rich-answers request handling.
controller_->SetVisibility(QuickAnswersVisibility::kRichAnswersVisible);
return;
}

// Route dismissal through |controller_| for logging impressions.
controller_->DismissQuickAnswers(QuickAnswersExitPoint::kQuickAnswersClick);

// TODO(b/240619915): Refactor so that we can access the request metadata
// instead of just the query itself.
if (base::StartsWith(query_, kTranslationQueryPrefix)) {
Expand All @@ -138,6 +141,14 @@ bool QuickAnswersUiController::CloseQuickAnswersView() {
return false;
}

bool QuickAnswersUiController::CloseRichAnswersView() {
if (IsShowingRichAnswersView()) {
rich_answers_view()->GetWidget()->Close();
return true;
}
return false;
}

void QuickAnswersUiController::OnRetryLabelPressed() {
controller_->OnRetryQuickAnswersRequest();
}
Expand Down Expand Up @@ -248,3 +259,8 @@ bool QuickAnswersUiController::IsShowingQuickAnswersView() const {
return quick_answers_view_tracker_.view() &&
!quick_answers_view_tracker_.view()->GetWidget()->IsClosed();
}

bool QuickAnswersUiController::IsShowingRichAnswersView() const {
return rich_answers_view_tracker_.view() &&
!rich_answers_view_tracker_.view()->GetWidget()->IsClosed();
}
11 changes: 11 additions & 0 deletions chrome/browser/ui/quick_answers/quick_answers_ui_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/ui/quick_answers/ui/quick_answers_view.h"
#include "chrome/browser/ui/quick_answers/ui/rich_answers_view.h"
#include "chrome/browser/ui/quick_answers/ui/user_consent_view.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/views/view_tracker.h"
Expand Down Expand Up @@ -42,6 +43,9 @@ class QuickAnswersUiController {
// Returns true if there was a QuickAnswersView to close.
bool CloseQuickAnswersView();

// Returns true if there was a RichAnswersView to close.
bool CloseRichAnswersView();

void OnQuickAnswersViewPressed();

void OnRetryLabelPressed();
Expand Down Expand Up @@ -88,13 +92,20 @@ class QuickAnswersUiController {
// showing.
bool IsShowingQuickAnswersView() const;

// Used by the controller to check if the RichAnswers view is currently
// showing.
bool IsShowingRichAnswersView() const;

QuickAnswersView* quick_answers_view() {
return static_cast<QuickAnswersView*>(quick_answers_view_tracker_.view());
}
quick_answers::UserConsentView* user_consent_view() {
return static_cast<quick_answers::UserConsentView*>(
user_consent_view_tracker_.view());
}
RichAnswersView* rich_answers_view() {
return static_cast<RichAnswersView*>(rich_answers_view_tracker_.view());
}

private:
raw_ptr<QuickAnswersControllerImpl> controller_ = nullptr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ enum class QuickAnswersVisibility {
// context is ready.
kPending = 1,
// Quick Answers UI is visible.
kVisible = 2,
kQuickAnswersVisible = 2,
// User Consent UI is visible.
// kUserConsentVisible = 3,
// Rich Answers UI is visible.
kRichAnswersVisible = 4,
};

// A controller to manage quick answers UI.
Expand All @@ -50,9 +54,8 @@ class QuickAnswersController {
const std::string& title,
const quick_answers::Context& context) = 0;

// Dismiss the quick-answers view (and/or any associated views like
// user-consent view) currently shown. |exit_point| indicates the exit point
// of the quick-answers view.
// Dismiss the specific quick-answers, user-consent, or rich-answers view
// currently shown. |exit_point| indicates the exit point of the view.
virtual void DismissQuickAnswers(
quick_answers::QuickAnswersExitPoint exit_point) = 0;

Expand All @@ -67,6 +70,8 @@ class QuickAnswersController {
virtual quick_answers::QuickAnswersDelegate* GetQuickAnswersDelegate() = 0;

virtual QuickAnswersVisibility GetVisibilityForTesting() const = 0;

virtual void SetVisibility(QuickAnswersVisibility visibility) = 0;
};

#endif // CHROMEOS_COMPONENTS_QUICK_ANSWERS_PUBLIC_CPP_CONTROLLER_QUICK_ANSWERS_CONTROLLER_H_
2 changes: 1 addition & 1 deletion chromeos/components/quick_answers/quick_answers_model.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ enum class QuickAnswersExitPoint {
// The exit point is unspecified. Might be used by tests, obsolete code or as
// placeholders.
kUnspecified = 0,
KContextMenuDismiss = 1,
kContextMenuDismiss = 1,
kContextMenuClick = 2,
kQuickAnswersClick = 3,
kSettingsButtonClick = 4,
Expand Down

0 comments on commit f8acf4c

Please sign in to comment.