[Mahi] Implement retry link
This CL shows a link to retry the previous failed Mahi service request on the Mahi error status view. When user clicks the link: 1. Re-ask the question if the error comes from question asking; OR 2. Re-generate the summary & outlines if the error comes from summary and outlines generation. Attach a demo image to the issue. The UMA metrics will be collected in the subsequent CL. Bug: b/319731862 Change-Id: Ic2ca797f1fe415ac901754f4d2702f9048ffc639 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5440224 Commit-Queue: Andrew Xu <andrewxu@chromium.org> Reviewed-by: Andre Le <leandre@chromium.org> Cr-Commit-Position: refs/heads/main@{#1286753}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
9ea84d3f8c
commit
9973203a3c
ash
ash_strings.grd
ash_strings_grd
system
mahi
mahi_constants.hmahi_error_status_view.ccmahi_error_status_view_pixeltest.ccmahi_panel_view.ccmahi_panel_view_unittest.ccmahi_panel_widget.ccmahi_question_answer_view.ccmahi_question_answer_view.hmahi_ui_controller.ccmahi_ui_controller.hmahi_ui_update.ccmahi_ui_update.hmahi_utils.ccmahi_utils.hrefresh_banner_view.ccsummary_outlines_section.cc
@ -5219,6 +5219,9 @@ No devices connected.
|
||||
<message name="IDS_ASH_MAHI_ERROR_STATUS_LABEL_RESOURCE_EXHAUSTED" translateable="false" desc="Text indicates that the service resource has been exhausted">
|
||||
Can't use right now. Try again later.
|
||||
</message>
|
||||
<message name="IDS_ASH_MAHI_RETRY_LINK_LABEL_TEXT" desc="Text for the link that user can retry their previous Mahi request">
|
||||
Try again
|
||||
</message>
|
||||
|
||||
<!-- Multi-profiles intro dialog -->
|
||||
<message name="IDS_ASH_MULTIPROFILES_INTRO_HEADLINE" desc="Describes which feature multi-profiles intro dialog presents.">
|
||||
|
@ -0,0 +1 @@
|
||||
67471e7a985946df41a6f954455c09b0f53df683
|
@ -43,6 +43,7 @@ enum ViewId {
|
||||
kRefreshButton,
|
||||
kErrorStatusView,
|
||||
kErrorStatusLabel,
|
||||
kErrorStatusRetryLink,
|
||||
kQuestionAnswerErrorImage,
|
||||
kQuestionAnswerErrorLabel,
|
||||
};
|
||||
|
@ -7,9 +7,13 @@
|
||||
#include <memory>
|
||||
|
||||
#include "ash/public/cpp/resources/grit/ash_public_unscaled_resources.h"
|
||||
#include "ash/strings/grit/ash_strings.h"
|
||||
#include "ash/style/typography.h"
|
||||
#include "ash/system/mahi/mahi_constants.h"
|
||||
#include "ash/system/mahi/mahi_ui_update.h"
|
||||
#include "ash/system/mahi/mahi_utils.h"
|
||||
#include "base/functional/bind.h"
|
||||
#include "base/functional/callback.h"
|
||||
#include "base/memory/raw_ptr.h"
|
||||
#include "base/notreached.h"
|
||||
#include "chromeos/components/mahi/public/cpp/mahi_manager.h"
|
||||
@ -23,6 +27,7 @@
|
||||
#include "ui/views/border.h"
|
||||
#include "ui/views/controls/image_view.h"
|
||||
#include "ui/views/controls/label.h"
|
||||
#include "ui/views/controls/link.h"
|
||||
#include "ui/views/layout/layout_types.h"
|
||||
#include "ui/views/view.h"
|
||||
|
||||
@ -56,7 +61,8 @@ class ErrorContentsView : public views::FlexLayoutView,
|
||||
public MahiUiController::Delegate {
|
||||
public:
|
||||
explicit ErrorContentsView(MahiUiController* ui_controller)
|
||||
: MahiUiController::Delegate(ui_controller) {
|
||||
: MahiUiController::Delegate(ui_controller),
|
||||
ui_controller_(ui_controller) {
|
||||
// TODO(http://b/319731862): Set the image when the image resource is ready.
|
||||
views::Builder<views::FlexLayoutView>(this)
|
||||
.SetBorder(views::CreateEmptyBorder(kContentsPaddings))
|
||||
@ -81,7 +87,14 @@ class ErrorContentsView : public views::FlexLayoutView,
|
||||
.SetHorizontalAlignment(gfx::HorizontalAlignment::ALIGN_CENTER)
|
||||
.SetID(mahi_constants::ViewId::kErrorStatusLabel)
|
||||
.SetMultiLine(true)
|
||||
.SetMaximumWidth(kLabelMaximumWidth))
|
||||
.SetMaximumWidth(kLabelMaximumWidth),
|
||||
views::Builder<views::Link>()
|
||||
.CopyAddressTo(&retry_link_)
|
||||
.SetForceUnderline(false)
|
||||
.SetID(mahi_constants::ViewId::kErrorStatusRetryLink)
|
||||
.SetText(l10n_util::GetStringUTF16(
|
||||
IDS_ASH_MAHI_RETRY_LINK_LABEL_TEXT))
|
||||
.SetVisible(false))
|
||||
.BuildChildren();
|
||||
}
|
||||
|
||||
@ -100,22 +113,40 @@ class ErrorContentsView : public views::FlexLayoutView,
|
||||
|
||||
void OnUpdated(const MahiUiUpdate& update) override {
|
||||
switch (update.type()) {
|
||||
case MahiUiUpdateType::kErrorReceived:
|
||||
case MahiUiUpdateType::kErrorReceived: {
|
||||
const MahiUiError& error = update.GetError();
|
||||
error_status_text_->SetText(l10n_util::GetStringUTF16(
|
||||
mahi_utils::GetErrorStatusViewTextId(update.GetError())));
|
||||
mahi_utils::GetErrorStatusViewTextId(error.status)));
|
||||
|
||||
retry_link_->SetVisible(
|
||||
mahi_utils::CalculateRetryLinkVisible(error.status));
|
||||
retry_link_->SetCallback(retry_link_->GetVisible()
|
||||
? base::BindRepeating(
|
||||
[](MahiUiController* controller,
|
||||
VisibilityState origin_state) {
|
||||
controller->Retry(origin_state);
|
||||
},
|
||||
ui_controller_, error.origin_state)
|
||||
: base::RepeatingClosure());
|
||||
return;
|
||||
}
|
||||
case MahiUiUpdateType::kAnswerLoaded:
|
||||
case MahiUiUpdateType::kContentsRefreshInitiated:
|
||||
case MahiUiUpdateType::kOutlinesLoaded:
|
||||
case MahiUiUpdateType::kQuestionPosted:
|
||||
case MahiUiUpdateType::kQuestionReAsked:
|
||||
case MahiUiUpdateType::kRefreshAvailabilityUpdated:
|
||||
case MahiUiUpdateType::kSummaryLoaded:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesSectionNavigated:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesReloaded:
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
const raw_ptr<MahiUiController> ui_controller_;
|
||||
|
||||
raw_ptr<views::Label> error_status_text_ = nullptr;
|
||||
raw_ptr<views::Link> retry_link_ = nullptr;
|
||||
};
|
||||
|
||||
} // namespace
|
||||
|
@ -70,7 +70,7 @@ TEST_F(MahiErrorStatusViewPixelTest, Basics) {
|
||||
mahi_constants::ViewId::kErrorStatusView);
|
||||
ASSERT_TRUE(error_status_view);
|
||||
EXPECT_TRUE(GetPixelDiffer()->CompareUiComponentsOnPrimaryScreen(
|
||||
"basics", /*revision_number=*/2, error_status_view));
|
||||
"basics", /*revision_number=*/3, error_status_view));
|
||||
}
|
||||
|
||||
} // namespace ash
|
||||
|
@ -691,9 +691,11 @@ void MahiPanelView::OnUpdated(const MahiUiUpdate& update) {
|
||||
return;
|
||||
case MahiUiUpdateType::kOutlinesLoaded:
|
||||
case MahiUiUpdateType::kQuestionPosted:
|
||||
case MahiUiUpdateType::kQuestionReAsked:
|
||||
case MahiUiUpdateType::kRefreshAvailabilityUpdated:
|
||||
case MahiUiUpdateType::kSummaryLoaded:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesSectionNavigated:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesReloaded:
|
||||
return;
|
||||
}
|
||||
}
|
||||
@ -730,7 +732,8 @@ void MahiPanelView::OnSendButtonPressed() {
|
||||
send_button_->SetEnabled(false);
|
||||
|
||||
ui_controller_->SendQuestion(std::u16string(trimmed_text),
|
||||
/*current_panel_content=*/true);
|
||||
/*current_panel_content=*/true,
|
||||
MahiUiController::QuestionSource::kPanel);
|
||||
question_textfield_->SetText(std::u16string());
|
||||
base::UmaHistogramEnumeration(
|
||||
mahi_constants::kMahiButtonClickHistogramName,
|
||||
|
@ -47,6 +47,7 @@ namespace {
|
||||
// Aliases ---------------------------------------------------------------------
|
||||
|
||||
using chromeos::MahiResponseStatus;
|
||||
using ::testing::_;
|
||||
using ::testing::Mock;
|
||||
using ::testing::NiceMock;
|
||||
using ::testing::Return;
|
||||
@ -979,14 +980,6 @@ TEST_F(MahiPanelViewTest, AnswerLoadingAnimation) {
|
||||
// iterating all possible errors.
|
||||
TEST_F(MahiPanelViewTest, FailToGetAnswer) {
|
||||
for (MahiResponseStatus error : GetMahiErrors()) {
|
||||
if (error == MahiResponseStatus::kInappropriate ||
|
||||
error == MahiResponseStatus::kLowQuota) {
|
||||
// `kInappropriate` introduced by a question is presented in the Q&A view,
|
||||
// verified in its own test. `kLowQuota` triggers a warning verified in
|
||||
// its own test.
|
||||
continue;
|
||||
}
|
||||
|
||||
// Config the mock mahi manager to return answer with an `error` asyncly.
|
||||
base::test::TestFuture<void> answer_waiter;
|
||||
EXPECT_CALL(mock_mahi_manager(), AnswerQuestion)
|
||||
@ -998,7 +991,8 @@ TEST_F(MahiPanelViewTest, FailToGetAnswer) {
|
||||
std::move(callback));
|
||||
});
|
||||
|
||||
SubmitTestQuestion();
|
||||
const std::u16string question(u"A question that brings errors");
|
||||
SubmitTestQuestion(question);
|
||||
|
||||
Mock::VerifyAndClearExpectations(&mock_mahi_manager());
|
||||
|
||||
@ -1026,8 +1020,21 @@ TEST_F(MahiPanelViewTest, FailToGetAnswer) {
|
||||
CHECK(error_status_label);
|
||||
EXPECT_TRUE(error_status_label->GetText().empty());
|
||||
|
||||
// Wait until an answer is loaded with an error. Verify views' visibility.
|
||||
// Wait until an answer is loaded with an error.
|
||||
ASSERT_TRUE(answer_waiter.Wait());
|
||||
|
||||
if (error == MahiResponseStatus::kInappropriate ||
|
||||
error == MahiResponseStatus::kLowQuota) {
|
||||
// `kInappropriate` introduced by a question is presented in the Q&A view,
|
||||
// verified in its own test. `kLowQuota` triggers a warning not presented
|
||||
// in the `error_status_view`.
|
||||
EXPECT_FALSE(error_status_view->GetVisible());
|
||||
EXPECT_TRUE(question_answer_view->GetVisible());
|
||||
EXPECT_FALSE(summary_outlines_section->GetVisible());
|
||||
CreatePanelWidget();
|
||||
continue;
|
||||
}
|
||||
|
||||
EXPECT_FALSE(question_answer_view->GetVisible());
|
||||
EXPECT_FALSE(question_answer_view->GetViewByID(
|
||||
mahi_constants::ViewId::kAnswerLoadingAnimatedImage));
|
||||
@ -1039,6 +1046,27 @@ TEST_F(MahiPanelViewTest, FailToGetAnswer) {
|
||||
error_status_label->GetText(),
|
||||
l10n_util::GetStringUTF16(mahi_utils::GetErrorStatusViewTextId(error)));
|
||||
|
||||
const auto* const retry_link =
|
||||
panel_view()->GetViewByID(mahi_constants::kErrorStatusRetryLink);
|
||||
ASSERT_TRUE(retry_link);
|
||||
EXPECT_EQ(retry_link->GetVisible(),
|
||||
mahi_utils::CalculateRetryLinkVisible(error));
|
||||
|
||||
if (retry_link->GetVisible()) {
|
||||
// Click the `retry_link`. The mock mahi manager should be asked about the
|
||||
// same question.
|
||||
views::test::RunScheduledLayout(widget());
|
||||
GetEventGenerator()->MoveMouseTo(
|
||||
retry_link->GetBoundsInScreen().CenterPoint());
|
||||
EXPECT_CALL(mock_mahi_manager(),
|
||||
AnswerQuestion(question, /*current_panel_content=*/true,
|
||||
/*callback=*/_));
|
||||
EXPECT_CALL(mock_mahi_manager(), GetOutlines).Times(0);
|
||||
EXPECT_CALL(mock_mahi_manager(), GetSummary).Times(0);
|
||||
GetEventGenerator()->ClickLeftButton();
|
||||
Mock::VerifyAndClear(&mock_mahi_manager());
|
||||
}
|
||||
|
||||
CreatePanelWidget();
|
||||
}
|
||||
}
|
||||
@ -1047,11 +1075,6 @@ TEST_F(MahiPanelViewTest, FailToGetAnswer) {
|
||||
// iterating all possible errors.
|
||||
TEST_F(MahiPanelViewTest, FailToGetOutlines) {
|
||||
for (MahiResponseStatus error : GetMahiErrors()) {
|
||||
if (error == MahiResponseStatus::kLowQuota) {
|
||||
// `kLowQuota` triggers a warning verified in its own test.
|
||||
continue;
|
||||
}
|
||||
|
||||
// Config the mock mahi manager to return outlines with an `error` asyncly.
|
||||
base::test::TestFuture<void> outlines_waiter;
|
||||
EXPECT_CALL(mock_mahi_manager(), GetOutlines)
|
||||
@ -1086,8 +1109,17 @@ TEST_F(MahiPanelViewTest, FailToGetOutlines) {
|
||||
CHECK(error_status_label);
|
||||
EXPECT_TRUE(error_status_label->GetText().empty());
|
||||
|
||||
// Wait until outlines are loaded with an error. Verify views' visibility.
|
||||
// Wait until outlines are loaded with an error.
|
||||
ASSERT_TRUE(outlines_waiter.Wait());
|
||||
|
||||
if (error == MahiResponseStatus::kLowQuota) {
|
||||
// `kLowQuota` triggers a warning not presented in `error_status_view`.
|
||||
EXPECT_FALSE(error_status_view->GetVisible());
|
||||
EXPECT_FALSE(question_answer_view->GetVisible());
|
||||
EXPECT_TRUE(summary_outlines_section->GetVisible());
|
||||
continue;
|
||||
}
|
||||
|
||||
EXPECT_FALSE(question_answer_view->GetVisible());
|
||||
EXPECT_FALSE(summary_outlines_section->GetVisible());
|
||||
EXPECT_TRUE(error_status_view->GetVisible());
|
||||
@ -1096,6 +1128,25 @@ TEST_F(MahiPanelViewTest, FailToGetOutlines) {
|
||||
EXPECT_EQ(
|
||||
error_status_label->GetText(),
|
||||
l10n_util::GetStringUTF16(mahi_utils::GetErrorStatusViewTextId(error)));
|
||||
|
||||
const auto* const retry_link =
|
||||
panel_view()->GetViewByID(mahi_constants::kErrorStatusRetryLink);
|
||||
ASSERT_TRUE(retry_link);
|
||||
EXPECT_EQ(retry_link->GetVisible(),
|
||||
mahi_utils::CalculateRetryLinkVisible(error));
|
||||
|
||||
if (retry_link->GetVisible()) {
|
||||
// Click the `retry_link`. The mock mahi manager should be requested for a
|
||||
// summary and outlines again.
|
||||
views::test::RunScheduledLayout(widget());
|
||||
GetEventGenerator()->MoveMouseTo(
|
||||
retry_link->GetBoundsInScreen().CenterPoint());
|
||||
EXPECT_CALL(mock_mahi_manager(), GetSummary);
|
||||
EXPECT_CALL(mock_mahi_manager(), GetOutlines);
|
||||
EXPECT_CALL(mock_mahi_manager(), AnswerQuestion).Times(0);
|
||||
GetEventGenerator()->ClickLeftButton();
|
||||
Mock::VerifyAndClear(&mock_mahi_manager());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -1103,11 +1154,6 @@ TEST_F(MahiPanelViewTest, FailToGetOutlines) {
|
||||
// all possible errors.
|
||||
TEST_F(MahiPanelViewTest, FailToGetSummary) {
|
||||
for (MahiResponseStatus error : GetMahiErrors()) {
|
||||
if (error == MahiResponseStatus::kLowQuota) {
|
||||
// `kLowQuota` triggers a warning verified in its own test.
|
||||
continue;
|
||||
}
|
||||
|
||||
// Config the mock mahi manager to return a summary with an `error` asyncly.
|
||||
base::test::TestFuture<void> summary_waiter;
|
||||
EXPECT_CALL(mock_mahi_manager(), GetSummary)
|
||||
@ -1142,8 +1188,17 @@ TEST_F(MahiPanelViewTest, FailToGetSummary) {
|
||||
CHECK(error_status_label);
|
||||
EXPECT_TRUE(error_status_label->GetText().empty());
|
||||
|
||||
// Wait until the summary is loaded with an error. Verify views' visibility.
|
||||
// Wait until the summary is loaded with an error.
|
||||
ASSERT_TRUE(summary_waiter.Wait());
|
||||
|
||||
if (error == MahiResponseStatus::kLowQuota) {
|
||||
// `kLowQuota` triggers a warning not presented in `error_status_view`.
|
||||
EXPECT_FALSE(error_status_view->GetVisible());
|
||||
EXPECT_FALSE(question_answer_view->GetVisible());
|
||||
EXPECT_TRUE(summary_outlines_section->GetVisible());
|
||||
continue;
|
||||
}
|
||||
|
||||
EXPECT_FALSE(question_answer_view->GetVisible());
|
||||
EXPECT_FALSE(summary_outlines_section->GetVisible());
|
||||
EXPECT_TRUE(error_status_view->GetVisible());
|
||||
@ -1152,6 +1207,25 @@ TEST_F(MahiPanelViewTest, FailToGetSummary) {
|
||||
EXPECT_EQ(
|
||||
error_status_label->GetText(),
|
||||
l10n_util::GetStringUTF16(mahi_utils::GetErrorStatusViewTextId(error)));
|
||||
|
||||
const auto* const retry_link =
|
||||
panel_view()->GetViewByID(mahi_constants::kErrorStatusRetryLink);
|
||||
ASSERT_TRUE(retry_link);
|
||||
EXPECT_EQ(retry_link->GetVisible(),
|
||||
mahi_utils::CalculateRetryLinkVisible(error));
|
||||
|
||||
if (retry_link->GetVisible()) {
|
||||
// Click the `retry_link`. The mock mahi manager should be requested for a
|
||||
// summary and outlines again.
|
||||
views::test::RunScheduledLayout(widget());
|
||||
GetEventGenerator()->MoveMouseTo(
|
||||
retry_link->GetBoundsInScreen().CenterPoint());
|
||||
EXPECT_CALL(mock_mahi_manager(), GetSummary);
|
||||
EXPECT_CALL(mock_mahi_manager(), GetOutlines);
|
||||
EXPECT_CALL(mock_mahi_manager(), AnswerQuestion).Times(0);
|
||||
GetEventGenerator()->ClickLeftButton();
|
||||
Mock::VerifyAndClear(&mock_mahi_manager());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -108,7 +108,8 @@ void MahiPanelWidget::NotifyRefreshAvailabilityChanged(bool available) {
|
||||
|
||||
void MahiPanelWidget::SendQuestion(const std::u16string& question,
|
||||
bool current_panel_content) {
|
||||
ui_controller_.SendQuestion(question, current_panel_content);
|
||||
ui_controller_.SendQuestion(question, current_panel_content,
|
||||
MahiUiController::QuestionSource::kMenuView);
|
||||
}
|
||||
|
||||
void MahiPanelWidget::OnViewVisibilityChanged(views::View* observed_view,
|
||||
|
@ -205,7 +205,9 @@ DEFINE_VIEW_BUILDER(ASH_EXPORT, ash::ErrorBubble)
|
||||
namespace ash {
|
||||
|
||||
MahiQuestionAnswerView::MahiQuestionAnswerView(MahiUiController* ui_controller)
|
||||
: MahiUiController::Delegate(ui_controller) {
|
||||
: MahiUiController::Delegate(ui_controller), ui_controller_(ui_controller) {
|
||||
CHECK(ui_controller);
|
||||
|
||||
SetOrientation(views::LayoutOrientation::kVertical);
|
||||
SetInteriorMargin(kQuestionAnswerInteriorMargin);
|
||||
SetIgnoreDefaultMainAxisMargins(true);
|
||||
@ -243,11 +245,13 @@ void MahiQuestionAnswerView::OnUpdated(const MahiUiUpdate& update) {
|
||||
case MahiUiUpdateType::kContentsRefreshInitiated:
|
||||
RemoveAllChildViews();
|
||||
return;
|
||||
case MahiUiUpdateType::kErrorReceived:
|
||||
case MahiUiUpdateType::kErrorReceived: {
|
||||
RemoveLoadingAnimatedImage();
|
||||
|
||||
// Creates `error_bubble_` if having an inappropriate question error.
|
||||
if (update.GetError() == chromeos::MahiResponseStatus::kInappropriate) {
|
||||
const MahiUiError& error = update.GetError();
|
||||
if (error.origin_state == VisibilityState::kQuestionAndAnswer &&
|
||||
error.status == chromeos::MahiResponseStatus::kInappropriate) {
|
||||
if (error_bubble_) {
|
||||
LOG(ERROR) << "Tried to add a new error bubble when there is an "
|
||||
"existing one.";
|
||||
@ -265,6 +269,7 @@ void MahiQuestionAnswerView::OnUpdated(const MahiUiUpdate& update) {
|
||||
.Build());
|
||||
}
|
||||
return;
|
||||
}
|
||||
case MahiUiUpdateType::kQuestionPosted: {
|
||||
AddChildView(CreateQuestionAnswerRow(update.GetQuestion(),
|
||||
/*is_question=*/true));
|
||||
@ -295,10 +300,19 @@ void MahiQuestionAnswerView::OnUpdated(const MahiUiUpdate& update) {
|
||||
|
||||
return;
|
||||
}
|
||||
case MahiUiUpdateType::kQuestionReAsked: {
|
||||
const MahiQuestionParams& question_params =
|
||||
update.GetReAskQuestionParams();
|
||||
ui_controller_->SendQuestion(question_params.question,
|
||||
question_params.current_panel_content,
|
||||
MahiUiController::QuestionSource::kRetry);
|
||||
return;
|
||||
}
|
||||
case MahiUiUpdateType::kOutlinesLoaded:
|
||||
case MahiUiUpdateType::kRefreshAvailabilityUpdated:
|
||||
case MahiUiUpdateType::kSummaryLoaded:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesSectionNavigated:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesReloaded:
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
@ -41,6 +41,8 @@ class ASH_EXPORT MahiQuestionAnswerView : public views::FlexLayoutView,
|
||||
|
||||
void RemoveLoadingAnimatedImage();
|
||||
|
||||
const raw_ptr<MahiUiController> ui_controller_;
|
||||
|
||||
// Tracks the bubble that presents the error introduced by the most recent
|
||||
// question. The bubble is created when the error occurs and is destroyed when
|
||||
// the user asks a new question.
|
||||
|
@ -4,7 +4,9 @@
|
||||
|
||||
#include "ash/system/mahi/mahi_ui_controller.h"
|
||||
|
||||
#include "ash/system/mahi/mahi_ui_update.h"
|
||||
#include "base/logging.h"
|
||||
#include "base/notreached.h"
|
||||
#include "chromeos/components/mahi/public/cpp/mahi_manager.h"
|
||||
#include "ui/views/view.h"
|
||||
|
||||
@ -54,12 +56,39 @@ void MahiUiController::NotifyRefreshAvailabilityChanged(bool available) {
|
||||
}
|
||||
|
||||
void MahiUiController::RefreshContents() {
|
||||
most_recent_question_params_.reset();
|
||||
NavigateToSummaryOutlinesSection();
|
||||
NotifyUiUpdate(MahiUiUpdate(MahiUiUpdateType::kContentsRefreshInitiated));
|
||||
}
|
||||
|
||||
void MahiUiController::Retry(VisibilityState origin_state) {
|
||||
switch (origin_state) {
|
||||
case VisibilityState::kQuestionAndAnswer:
|
||||
if (most_recent_question_params_) {
|
||||
SetVisibilityStateAndNotifyUiUpdate(
|
||||
origin_state, MahiUiUpdate(MahiUiUpdateType::kQuestionReAsked,
|
||||
*most_recent_question_params_));
|
||||
} else {
|
||||
LOG(ERROR) << "Tried to re-ask a non-existing question";
|
||||
}
|
||||
return;
|
||||
case VisibilityState::kSummaryAndOutlines:
|
||||
SetVisibilityStateAndNotifyUiUpdate(
|
||||
origin_state,
|
||||
MahiUiUpdate(MahiUiUpdateType::kSummaryAndOutlinesReloaded));
|
||||
return;
|
||||
case VisibilityState::kError:
|
||||
NOTREACHED_NORETURN();
|
||||
}
|
||||
}
|
||||
|
||||
void MahiUiController::SendQuestion(const std::u16string& question,
|
||||
bool current_panel_content) {
|
||||
bool current_panel_content,
|
||||
QuestionSource source) {
|
||||
if (source != QuestionSource::kRetry) {
|
||||
most_recent_question_params_.emplace(question, current_panel_content);
|
||||
}
|
||||
|
||||
// Display the Q&A section.
|
||||
SetVisibilityStateAndNotifyUiUpdate(
|
||||
VisibilityState::kQuestionAndAnswer,
|
||||
@ -78,10 +107,8 @@ void MahiUiController::UpdateSummaryAndOutlines() {
|
||||
&MahiUiController::OnOutlinesLoaded, weak_ptr_factory_.GetWeakPtr()));
|
||||
}
|
||||
|
||||
void MahiUiController::HandleErrorStatus(chromeos::MahiResponseStatus status) {
|
||||
CHECK(HasError(status));
|
||||
|
||||
if (status == chromeos::MahiResponseStatus::kLowQuota) {
|
||||
void MahiUiController::HandleError(const MahiUiError& error) {
|
||||
if (error.status == chromeos::MahiResponseStatus::kLowQuota) {
|
||||
// TODO(http://b/319731862): Add the low quota warning toast.
|
||||
return;
|
||||
}
|
||||
@ -89,9 +116,9 @@ void MahiUiController::HandleErrorStatus(chromeos::MahiResponseStatus status) {
|
||||
// The presentation of the inappropriate error during
|
||||
// `State::kQuestionAndAnswer` should be embedded into the Q&A view instead
|
||||
// of a separate view.
|
||||
const MahiUiUpdate update(MahiUiUpdateType::kErrorReceived, status);
|
||||
if (status == chromeos::MahiResponseStatus::kInappropriate &&
|
||||
visibility_state_ == VisibilityState::kQuestionAndAnswer) {
|
||||
const MahiUiUpdate update(MahiUiUpdateType::kErrorReceived, error);
|
||||
if (error.status == chromeos::MahiResponseStatus::kInappropriate &&
|
||||
error.origin_state == VisibilityState::kQuestionAndAnswer) {
|
||||
NotifyUiUpdate(update);
|
||||
return;
|
||||
}
|
||||
@ -125,7 +152,8 @@ void MahiUiController::SetVisibilityStateAndNotifyUiUpdate(
|
||||
void MahiUiController::OnAnswerLoaded(std::optional<std::u16string> answer,
|
||||
chromeos::MahiResponseStatus status) {
|
||||
if (HasError(status)) {
|
||||
HandleErrorStatus(status);
|
||||
HandleError(MahiUiError(
|
||||
status, /*origin_state=*/VisibilityState::kQuestionAndAnswer));
|
||||
return;
|
||||
}
|
||||
|
||||
@ -144,7 +172,8 @@ void MahiUiController::OnOutlinesLoaded(
|
||||
std::vector<chromeos::MahiOutline> outlines,
|
||||
chromeos::MahiResponseStatus status) {
|
||||
if (HasError(status)) {
|
||||
HandleErrorStatus(status);
|
||||
HandleError(MahiUiError(
|
||||
status, /*origin_state=*/VisibilityState::kSummaryAndOutlines));
|
||||
return;
|
||||
}
|
||||
|
||||
@ -154,7 +183,8 @@ void MahiUiController::OnOutlinesLoaded(
|
||||
void MahiUiController::OnSummaryLoaded(std::u16string summary_text,
|
||||
chromeos::MahiResponseStatus status) {
|
||||
if (HasError(status)) {
|
||||
HandleErrorStatus(status);
|
||||
HandleError(MahiUiError(
|
||||
status, /*origin_state=*/VisibilityState::kSummaryAndOutlines));
|
||||
return;
|
||||
}
|
||||
|
||||
|
@ -47,6 +47,18 @@ class ASH_EXPORT MahiUiController {
|
||||
base::ScopedObservation<MahiUiController, Delegate> observation_{this};
|
||||
};
|
||||
|
||||
// Lists question sources.
|
||||
enum class QuestionSource {
|
||||
// From the Mahi menu view.
|
||||
kMenuView,
|
||||
|
||||
// From the Mahi panel view.
|
||||
kPanel,
|
||||
|
||||
// From the retry button.
|
||||
kRetry,
|
||||
};
|
||||
|
||||
MahiUiController();
|
||||
MahiUiController(const MahiUiController&) = delete;
|
||||
MahiUiController& operator=(const MahiUiController&) = delete;
|
||||
@ -65,9 +77,20 @@ class ASH_EXPORT MahiUiController {
|
||||
// navigates to the summary view.
|
||||
void RefreshContents();
|
||||
|
||||
// Retries the operation associated with `origin_state`.
|
||||
// If `origin_state` is `VisibilityState::kQuestionAndAnswer`, re-asks the
|
||||
// question.
|
||||
// If `origin_state` is `VisibilityState::kSummaryAndOutlines`, regenerates
|
||||
// the summary & outlines.
|
||||
// NOTE: `origin_state` should not be `VisibilityState::kError`.
|
||||
void Retry(VisibilityState origin_state);
|
||||
|
||||
// Sends `question` to the backend. `current_panel_content` determines if the
|
||||
// `question` is regarding the current content displayed on the panel.
|
||||
void SendQuestion(const std::u16string& question, bool current_panel_content);
|
||||
// `source` indicates where `question` is posted.
|
||||
void SendQuestion(const std::u16string& question,
|
||||
bool current_panel_content,
|
||||
QuestionSource source);
|
||||
|
||||
// Sends requests to the backend to update summary and outlines.
|
||||
// `delegates_` will be notified of the updated summary and outlines when
|
||||
@ -75,9 +98,7 @@ class ASH_EXPORT MahiUiController {
|
||||
void UpdateSummaryAndOutlines();
|
||||
|
||||
private:
|
||||
// Handles the error indicated by `status`. `status` cannot be
|
||||
// `chromeos::MahiResponseStatus::kSuccess`.
|
||||
void HandleErrorStatus(chromeos::MahiResponseStatus status);
|
||||
void HandleError(const MahiUiError& error);
|
||||
|
||||
// Notifies `delegates_` of `update`.
|
||||
void NotifyUiUpdate(const MahiUiUpdate& update);
|
||||
@ -102,6 +123,11 @@ class ASH_EXPORT MahiUiController {
|
||||
|
||||
base::ObserverList<Delegate> delegates_;
|
||||
|
||||
// Indicates the params of the most recent question.
|
||||
// Set when the controller receives a request to send a question.
|
||||
// Reset when the content is refreshed.
|
||||
std::optional<MahiQuestionParams> most_recent_question_params_;
|
||||
|
||||
base::WeakPtrFactory<MahiUiController> weak_ptr_factory_{this};
|
||||
};
|
||||
|
||||
|
@ -4,11 +4,31 @@
|
||||
|
||||
#include "ash/system/mahi/mahi_ui_update.h"
|
||||
|
||||
#include <variant>
|
||||
|
||||
#include "base/check_op.h"
|
||||
#include "chromeos/components/mahi/public/cpp/mahi_manager.h"
|
||||
|
||||
namespace ash {
|
||||
|
||||
// MahiQuestionParams ----------------------------------------------------------
|
||||
|
||||
MahiQuestionParams::MahiQuestionParams(const std::u16string& question,
|
||||
bool current_panel_content)
|
||||
: question(question), current_panel_content(current_panel_content) {}
|
||||
|
||||
MahiQuestionParams::~MahiQuestionParams() = default;
|
||||
|
||||
// MahiUiError -----------------------------------------------------------------
|
||||
|
||||
MahiUiError::MahiUiError(chromeos::MahiResponseStatus status,
|
||||
VisibilityState origin_state)
|
||||
: status(status), origin_state(origin_state) {
|
||||
CHECK_NE(status, chromeos::MahiResponseStatus::kSuccess);
|
||||
}
|
||||
|
||||
MahiUiError::~MahiUiError() = default;
|
||||
|
||||
// MahiUiUpdate ----------------------------------------------------------------
|
||||
|
||||
MahiUiUpdate::MahiUiUpdate(MahiUiUpdateType type)
|
||||
@ -16,13 +36,18 @@ MahiUiUpdate::MahiUiUpdate(MahiUiUpdateType type)
|
||||
CheckTypeMatchesPayload();
|
||||
}
|
||||
|
||||
MahiUiUpdate::MahiUiUpdate(MahiUiUpdateType type,
|
||||
chromeos::MahiResponseStatus payload)
|
||||
MahiUiUpdate::MahiUiUpdate(MahiUiUpdateType type, bool payload)
|
||||
: type_(type), payload_(payload) {
|
||||
CheckTypeMatchesPayload();
|
||||
}
|
||||
|
||||
MahiUiUpdate::MahiUiUpdate(MahiUiUpdateType type, bool payload)
|
||||
MahiUiUpdate::MahiUiUpdate(MahiUiUpdateType type, const MahiUiError& payload)
|
||||
: type_(type), payload_(payload) {
|
||||
CheckTypeMatchesPayload();
|
||||
}
|
||||
|
||||
MahiUiUpdate::MahiUiUpdate(MahiUiUpdateType type,
|
||||
const MahiQuestionParams& payload)
|
||||
: type_(type), payload_(payload) {
|
||||
CheckTypeMatchesPayload();
|
||||
}
|
||||
@ -45,9 +70,9 @@ const std::u16string& MahiUiUpdate::GetAnswer() const {
|
||||
return std::get<std::reference_wrapper<const std::u16string>>(*payload_);
|
||||
}
|
||||
|
||||
chromeos::MahiResponseStatus MahiUiUpdate::GetError() const {
|
||||
const MahiUiError& MahiUiUpdate::GetError() const {
|
||||
CHECK_EQ(type_, MahiUiUpdateType::kErrorReceived);
|
||||
return std::get<chromeos::MahiResponseStatus>(*payload_);
|
||||
return std::get<std::reference_wrapper<const MahiUiError>>(*payload_);
|
||||
}
|
||||
|
||||
const std::vector<chromeos::MahiOutline>& MahiUiUpdate::GetOutlines() const {
|
||||
@ -62,6 +87,11 @@ const std::u16string& MahiUiUpdate::GetQuestion() const {
|
||||
return std::get<std::reference_wrapper<const std::u16string>>(*payload_);
|
||||
}
|
||||
|
||||
const MahiQuestionParams& MahiUiUpdate::GetReAskQuestionParams() const {
|
||||
CHECK_EQ(type_, MahiUiUpdateType::kQuestionReAsked);
|
||||
return std::get<std::reference_wrapper<const MahiQuestionParams>>(*payload_);
|
||||
}
|
||||
|
||||
bool MahiUiUpdate::GetRefreshAvailability() const {
|
||||
CHECK_EQ(type_, MahiUiUpdateType::kRefreshAvailabilityUpdated);
|
||||
return std::get<bool>(*payload_);
|
||||
@ -85,9 +115,8 @@ void MahiUiUpdate::CheckTypeMatchesPayload() {
|
||||
break;
|
||||
case MahiUiUpdateType::kErrorReceived:
|
||||
CHECK(payload_.has_value());
|
||||
CHECK(std::holds_alternative<chromeos::MahiResponseStatus>(*payload_));
|
||||
CHECK_NE(std::get<chromeos::MahiResponseStatus>(*payload_),
|
||||
chromeos::MahiResponseStatus::kSuccess);
|
||||
CHECK(std::holds_alternative<std::reference_wrapper<const MahiUiError>>(
|
||||
*payload_));
|
||||
break;
|
||||
case MahiUiUpdateType::kOutlinesLoaded:
|
||||
CHECK(payload_.has_value());
|
||||
@ -101,6 +130,11 @@ void MahiUiUpdate::CheckTypeMatchesPayload() {
|
||||
std::holds_alternative<std::reference_wrapper<const std::u16string>>(
|
||||
*payload_));
|
||||
break;
|
||||
case MahiUiUpdateType::kQuestionReAsked:
|
||||
CHECK(payload_.has_value());
|
||||
CHECK(std::holds_alternative<
|
||||
std::reference_wrapper<const MahiQuestionParams>>(*payload_));
|
||||
break;
|
||||
case MahiUiUpdateType::kRefreshAvailabilityUpdated:
|
||||
CHECK(payload_.has_value());
|
||||
CHECK(std::holds_alternative<bool>(*payload_));
|
||||
@ -114,6 +148,9 @@ void MahiUiUpdate::CheckTypeMatchesPayload() {
|
||||
std::holds_alternative<std::reference_wrapper<const std::u16string>>(
|
||||
*payload_));
|
||||
break;
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesReloaded:
|
||||
CHECK(!payload_.has_value());
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -7,6 +7,7 @@
|
||||
|
||||
#include <functional>
|
||||
#include <optional>
|
||||
#include <string>
|
||||
#include <variant>
|
||||
#include <vector>
|
||||
|
||||
@ -45,6 +46,9 @@ enum class MahiUiUpdateType {
|
||||
// A question is posted by user.
|
||||
kQuestionPosted,
|
||||
|
||||
// A question is re-asked by user.
|
||||
kQuestionReAsked,
|
||||
|
||||
// The content refresh availability changes.
|
||||
kRefreshAvailabilityUpdated,
|
||||
|
||||
@ -53,18 +57,53 @@ enum class MahiUiUpdateType {
|
||||
|
||||
// A summary is loaded with a success.
|
||||
kSummaryLoaded,
|
||||
|
||||
// The summary and outlines are requested to reload.
|
||||
kSummaryAndOutlinesReloaded,
|
||||
};
|
||||
|
||||
// Contains the params required to send a question to the Mahi backend.
|
||||
struct MahiQuestionParams {
|
||||
MahiQuestionParams(const std::u16string& question,
|
||||
bool current_panel_content);
|
||||
MahiQuestionParams(const MahiQuestionParams&) = delete;
|
||||
MahiQuestionParams& operator=(const MahiQuestionParams&) = delete;
|
||||
~MahiQuestionParams();
|
||||
|
||||
const std::u16string question;
|
||||
|
||||
// Determines if the `question` is regarding the current content displayed on
|
||||
// the panel.
|
||||
const bool current_panel_content;
|
||||
};
|
||||
|
||||
// Describes a Mahi UI error, including its origin and status.
|
||||
struct MahiUiError {
|
||||
MahiUiError(chromeos::MahiResponseStatus status,
|
||||
VisibilityState origin_state);
|
||||
MahiUiError(const MahiUiError&) = delete;
|
||||
MahiUiError& operator=(const MahiUiError&) = delete;
|
||||
~MahiUiError();
|
||||
|
||||
// The error status. NOTE: `status` should not be
|
||||
// `chromeos::MahiResponseStatus::kSuccess`.
|
||||
const chromeos::MahiResponseStatus status;
|
||||
|
||||
// Indicates the `VisibilityState` where `status` comes from.
|
||||
const VisibilityState origin_state;
|
||||
};
|
||||
|
||||
// Indicates a change that triggers a visible update on the Mahi UI.
|
||||
class MahiUiUpdate {
|
||||
public:
|
||||
explicit MahiUiUpdate(MahiUiUpdateType type);
|
||||
MahiUiUpdate(MahiUiUpdateType type, chromeos::MahiResponseStatus payload);
|
||||
MahiUiUpdate(MahiUiUpdateType type, bool payload);
|
||||
|
||||
// NOTE: `MahiUiUpdate` caches the const reference to `payload`, not a copy.
|
||||
// The class user has the duty to ensure the original `payload` object
|
||||
// outlives the `MahiUiUpdate` instance.
|
||||
MahiUiUpdate(MahiUiUpdateType type, const MahiUiError& payload);
|
||||
MahiUiUpdate(MahiUiUpdateType type, const MahiQuestionParams& payload);
|
||||
MahiUiUpdate(MahiUiUpdateType type, const std::u16string& payload);
|
||||
MahiUiUpdate(MahiUiUpdateType type,
|
||||
const std::vector<chromeos::MahiOutline>& payload);
|
||||
@ -79,7 +118,7 @@ class MahiUiUpdate {
|
||||
|
||||
// Returns the error from `payload`.
|
||||
// NOTE: This function should be called only if `type` is `kErrorReceived`.
|
||||
chromeos::MahiResponseStatus GetError() const;
|
||||
const MahiUiError& GetError() const;
|
||||
|
||||
// Returns the outlines from `payload`.
|
||||
// NOTE: This function should be called only if `type` is `kOutlinesLoaded`.
|
||||
@ -89,6 +128,10 @@ class MahiUiUpdate {
|
||||
// NOTE: This function should be called only if `type` is `kQuestionPosted`.
|
||||
const std::u16string& GetQuestion() const;
|
||||
|
||||
// Returns the params required to re-ask a question.
|
||||
// NOTE: This function should be called only if `type` is `kQuestionReAsked`.
|
||||
const MahiQuestionParams& GetReAskQuestionParams() const;
|
||||
|
||||
// Returns the refresh availability from `payload`.
|
||||
// NOTE: This function should be called only if `type` is
|
||||
// `kRefreshAvailabilityUpdated`.
|
||||
@ -112,12 +155,15 @@ class MahiUiUpdate {
|
||||
// For `kErrorReceived`, `payload` is an error;
|
||||
// For `kOutlinesLoaded`, `payload` is an array of outlines;
|
||||
// For `kQuestionPosted`, `payload` is a question;
|
||||
// For `kQuestionReAsked`, `payload` is a question params struct;
|
||||
// For `kRefreshAvailabilityUpdated`, `payload` is a boolean;
|
||||
// For `kSummaryAndOutlinesSectionNavigated`, `payload` is `std::nullopt`;
|
||||
// For `kSummaryLoaded`, `payload` is a summary.
|
||||
// For `kSummaryLoaded`, `payload` is a summary;
|
||||
// For `kSummaryAndOutlinesReloaded`, `payload` is `std::nullopt`.
|
||||
using PayloadType = std::variant<
|
||||
std::reference_wrapper<const std::u16string>,
|
||||
chromeos::MahiResponseStatus,
|
||||
std::reference_wrapper<const MahiQuestionParams>,
|
||||
std::reference_wrapper<const MahiUiError>,
|
||||
std::reference_wrapper<const std::vector<chromeos::MahiOutline>>,
|
||||
bool>;
|
||||
const std::optional<PayloadType> payload_;
|
||||
|
@ -10,6 +10,22 @@
|
||||
|
||||
namespace ash::mahi_utils {
|
||||
|
||||
bool CalculateRetryLinkVisible(chromeos::MahiResponseStatus error) {
|
||||
switch (error) {
|
||||
case chromeos::MahiResponseStatus::kCantFindOutputData:
|
||||
case chromeos::MahiResponseStatus::kContentExtractionError:
|
||||
case chromeos::MahiResponseStatus::kInappropriate:
|
||||
case chromeos::MahiResponseStatus::kUnknownError:
|
||||
return true;
|
||||
case chromeos::MahiResponseStatus::kQuotaLimitHit:
|
||||
case chromeos::MahiResponseStatus::kResourceExhausted:
|
||||
return false;
|
||||
case chromeos::MahiResponseStatus::kLowQuota:
|
||||
case chromeos::MahiResponseStatus::kSuccess:
|
||||
NOTREACHED_NORETURN();
|
||||
}
|
||||
}
|
||||
|
||||
int GetErrorStatusViewTextId(chromeos::MahiResponseStatus error) {
|
||||
switch (error) {
|
||||
case chromeos::MahiResponseStatus::kCantFindOutputData:
|
||||
|
@ -13,6 +13,11 @@ enum class MahiResponseStatus;
|
||||
|
||||
namespace ash::mahi_utils {
|
||||
|
||||
// Returns the retry link's target visible for `error`.
|
||||
// NOTE: This function should be called only if the `error` should be presented
|
||||
// on `MahiErrorStatusView`.
|
||||
ASH_EXPORT bool CalculateRetryLinkVisible(chromeos::MahiResponseStatus error);
|
||||
|
||||
// Returns the text ID of the `error` description on `MahiErrorStatusView`.
|
||||
// NOTE: This function should be called only if the `error` should be presented
|
||||
// on `MahiErrorStatusView`.
|
||||
|
@ -226,8 +226,10 @@ void RefreshBannerView::OnUpdated(const MahiUiUpdate& update) {
|
||||
case MahiUiUpdateType::kAnswerLoaded:
|
||||
case MahiUiUpdateType::kOutlinesLoaded:
|
||||
case MahiUiUpdateType::kQuestionPosted:
|
||||
case MahiUiUpdateType::kQuestionReAsked:
|
||||
case MahiUiUpdateType::kSummaryLoaded:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesSectionNavigated:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesReloaded:
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
@ -9,6 +9,7 @@
|
||||
#include "ash/style/typography.h"
|
||||
#include "ash/system/mahi/mahi_animation_utils.h"
|
||||
#include "ash/system/mahi/mahi_constants.h"
|
||||
#include "ash/system/mahi/mahi_ui_update.h"
|
||||
#include "ash/system/mahi/resources/grit/mahi_resources.h"
|
||||
#include "base/check.h"
|
||||
#include "base/check_is_test.h"
|
||||
@ -149,6 +150,7 @@ bool SummaryOutlinesSection::GetViewVisibility(VisibilityState state) const {
|
||||
void SummaryOutlinesSection::OnUpdated(const MahiUiUpdate& update) {
|
||||
switch (update.type()) {
|
||||
case MahiUiUpdateType::kContentsRefreshInitiated:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesReloaded:
|
||||
LoadSummaryAndOutlines();
|
||||
return;
|
||||
case MahiUiUpdateType::kOutlinesLoaded:
|
||||
@ -160,6 +162,7 @@ void SummaryOutlinesSection::OnUpdated(const MahiUiUpdate& update) {
|
||||
case MahiUiUpdateType::kAnswerLoaded:
|
||||
case MahiUiUpdateType::kErrorReceived:
|
||||
case MahiUiUpdateType::kQuestionPosted:
|
||||
case MahiUiUpdateType::kQuestionReAsked:
|
||||
case MahiUiUpdateType::kRefreshAvailabilityUpdated:
|
||||
case MahiUiUpdateType::kSummaryAndOutlinesSectionNavigated:
|
||||
return;
|
||||
|
Reference in New Issue
Block a user