Skip to content

Commit

Permalink
Handle onboarding suggestion icons and presses.
Browse files Browse the repository at this point in the history 
Press events are delegated to the AssistantViewDelegate and follow the
standard Assistant code path for suggestion press events.

A new enum value is added to distinguish onboarding suggestions from
suggestions of other types in logs.

Bug: b:157689497
Change-Id: I1e9151b88a161c717ade85002e0e80fa644653ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239118
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777647}
  • Loading branch information
David Black authored and Commit Bot committed Jun 12, 2020
1 parent 698b0ac commit 2653bc7
Show file tree
Hide file tree
Showing 9 changed files with 223 additions and 48 deletions.
19 changes: 15 additions & 4 deletions ash/assistant/assistant_interaction_controller_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,20 @@ void AssistantInteractionControllerImpl::OnSuggestionChipPressed(
return;
}

// Determine query source from suggestion type.
AssistantQuerySource query_source;
switch (suggestion->type) {
case AssistantSuggestionType::kBetterOnboarding:
query_source = AssistantQuerySource::kBetterOnboarding;
break;
case AssistantSuggestionType::kConversationStarter:
query_source = AssistantQuerySource::kConversationStarter;
break;
case AssistantSuggestionType::kUnspecified:
query_source = AssistantQuerySource::kSuggestionChip;
break;
}

// Otherwise, we will submit a simple text query using the suggestion text.
// Note that a text query originating from a suggestion chip will carry
// forward the allowance/forbiddance of TTS from the previous response. This
Expand All @@ -533,10 +547,7 @@ void AssistantInteractionControllerImpl::OnSuggestionChipPressed(
StartTextInteraction(
suggestion->text,
/*allow_tts=*/model_.response() && model_.response()->has_tts(),
/*query_source=*/suggestion->type ==
AssistantSuggestionType::kConversationStarter
? AssistantQuerySource::kConversationStarter
: AssistantQuerySource::kSuggestionChip);
query_source);
}

void AssistantInteractionControllerImpl::OnTabletModeStarted() {
Expand Down
1 change: 0 additions & 1 deletion ash/assistant/assistant_interaction_controller_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ namespace ash {

class AssistantControllerImpl;
enum class AssistantButtonId;
enum class AssistantQuerySource;

class AssistantInteractionControllerImpl
: public AssistantInteractionController,
Expand Down
69 changes: 68 additions & 1 deletion ash/assistant/assistant_interaction_controller_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@

#include "ash/assistant/assistant_interaction_controller_impl.h"

#include <map>

#include "ash/assistant/test/assistant_ash_test_base.h"
#include "ash/public/cpp/assistant/controller/assistant_interaction_controller.h"
#include "ash/test/fake_android_intent_helper.h"
#include "base/bind.h"
#include "chromeos/services/assistant/public/mojom/assistant.mojom-forward.h"
#include "chromeos/services/assistant/public/cpp/default_assistant_interaction_subscriber.h"
#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
#include "testing/gmock/include/gmock/gmock.h"

namespace ash {
Expand All @@ -17,13 +20,38 @@ namespace {

using chromeos::assistant::mojom::AndroidAppInfo;
using chromeos::assistant::mojom::AndroidAppInfoPtr;
using chromeos::assistant::mojom::Assistant;
using chromeos::assistant::mojom::AssistantInteractionMetadata;
using chromeos::assistant::mojom::AssistantInteractionMetadataPtr;
using chromeos::assistant::mojom::AssistantInteractionSubscriber;
using chromeos::assistant::mojom::AssistantInteractionType;
using chromeos::assistant::mojom::AssistantQuerySource;
using chromeos::assistant::mojom::AssistantSuggestion;
using chromeos::assistant::mojom::AssistantSuggestionType;

using ::testing::Invoke;
using ::testing::Mock;
using ::testing::Return;
using ::testing::StrictMock;

constexpr bool kErrorResult = false;
constexpr bool kSuccessResult = true;

// Mocks -----------------------------------------------------------------------

class AssistantInteractionSubscriberMock
: public chromeos::assistant::DefaultAssistantInteractionSubscriber {
public:
explicit AssistantInteractionSubscriberMock(Assistant* service) {
service->AddAssistantInteractionSubscriber(BindNewPipeAndPassRemote());
}

MOCK_METHOD(void,
OnInteractionStarted,
(AssistantInteractionMetadataPtr),
(override));
};

// A simple mock to allow mocking of the callback passed to |OnOpenAppResponse|.
// Example usage:
//
Expand All @@ -41,6 +69,8 @@ class OpenAppCallbackMock {
}
};

// AssistantInteractionControllerImplTest --------------------------------------

class AssistantInteractionControllerImplTest : public AssistantAshTestBase {
public:
AssistantInteractionControllerImplTest() = default;
Expand Down Expand Up @@ -151,4 +181,41 @@ TEST_F(AssistantInteractionControllerImplTest,
EXPECT_EQ(intent_with_scheme, fake_helper.last_launched_android_intent());
}

TEST_F(AssistantInteractionControllerImplTest,
ShouldCorrectlyMapSuggestionTypeToQuerySource) {
// Mock Assistant interaction subscriber.
StrictMock<AssistantInteractionSubscriberMock> mock(assistant_service());

// Configure the expected mappings between suggestion type and query source.
const std::map<AssistantSuggestionType, AssistantQuerySource>
types_to_sources = {{AssistantSuggestionType::kConversationStarter,
AssistantQuerySource::kConversationStarter},
{AssistantSuggestionType::kBetterOnboarding,
AssistantQuerySource::kBetterOnboarding},
{AssistantSuggestionType::kUnspecified,
AssistantQuerySource::kSuggestionChip}};

// Iterate over all expected mappings.
for (const auto& type_to_source : types_to_sources) {
base::RunLoop run_loop;

// Confirm subscribers are delivered the expected query source...
EXPECT_CALL(mock, OnInteractionStarted)
.WillOnce(Invoke([&](AssistantInteractionMetadataPtr metadata) {
EXPECT_EQ(type_to_source.second, metadata->source);
run_loop.QuitClosure().Run();
}));

// ...when an Assistant suggestion of a given type is pressed.
interaction_controller()->OnSuggestionChipPressed(
AssistantSuggestion::New(/*id=*/base::UnguessableToken::Create(),
/*type=*/type_to_source.first,
/*text=*/"", /*icon_url=*/GURL(),
/*action_url=*/GURL())
.get());

run_loop.Run();
}
}

} // namespace ash
2 changes: 2 additions & 0 deletions ash/assistant/test/assistant_ash_test_base.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "ash/public/cpp/assistant/assistant_state.h"
#include "ash/public/cpp/assistant/controller/assistant_ui_controller.h"
#include "ash/public/cpp/test/assistant_test_api.h"
#include "ash/public/cpp/test/test_image_downloader.h"
#include "ash/shell.h"
#include "ash/test/ash_test_helper.h"
#include "base/run_loop.h"
Expand Down Expand Up @@ -102,6 +103,7 @@ AssistantAshTestBase::AssistantAshTestBase(
test_api_(AssistantTestApi::Create()),
test_setup_(std::make_unique<TestAssistantSetup>()),
test_web_view_factory_(std::make_unique<TestAssistantWebViewFactory>()),
test_image_downloader_(std::make_unique<TestImageDownloader>()),
assistant_client_(std::make_unique<TestAssistantClient>()) {}

AssistantAshTestBase::~AssistantAshTestBase() = default;
Expand Down
2 changes: 2 additions & 0 deletions ash/assistant/test/assistant_ash_test_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class TestAssistantClient;
class TestAssistantService;
class TestAssistantSetup;
class TestAssistantWebViewFactory;
class TestImageDownloader;

// Helper class to make testing the Assistant Ash UI easier.
class AssistantAshTestBase : public AshTestBase {
Expand Down Expand Up @@ -185,6 +186,7 @@ class AssistantAshTestBase : public AshTestBase {
std::unique_ptr<AssistantTestApi> test_api_;
std::unique_ptr<TestAssistantSetup> test_setup_;
std::unique_ptr<TestAssistantWebViewFactory> test_web_view_factory_;
std::unique_ptr<TestImageDownloader> test_image_downloader_;

std::vector<std::unique_ptr<aura::Window>> windows_;
std::vector<std::unique_ptr<views::Widget>> widgets_;
Expand Down
72 changes: 60 additions & 12 deletions ash/assistant/ui/main_stage/assistant_onboarding_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <memory>
#include <string>
#include <vector>

#include "ash/assistant/model/assistant_ui_model.h"
#include "ash/assistant/ui/assistant_ui_constants.h"
Expand All @@ -14,11 +15,15 @@
#include "ash/public/cpp/assistant/controller/assistant_ui_controller.h"
#include "ash/strings/grit/ash_strings.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "base/time/time.h"
#include "base/unguessable_token.h"
#include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/color_palette.h"
#include "ui/views/background.h"
#include "ui/views/border.h"
#include "ui/views/controls/button/button.h"
#include "ui/views/controls/image_view.h"
#include "ui/views/controls/label.h"
#include "ui/views/layout/box_layout.h"
Expand All @@ -30,6 +35,10 @@ namespace ash {

namespace {

using chromeos::assistant::mojom::AssistantSuggestion;
using chromeos::assistant::mojom::AssistantSuggestionPtr;
using chromeos::assistant::mojom::AssistantSuggestionType;

constexpr int kHorizontalMarginDip = 56;

// Greeting.
Expand Down Expand Up @@ -110,15 +119,23 @@ SkColor GetSuggestionTextColor(int index) {

// SuggestionView --------------------------------------------------------------

class SuggestionView : public views::View {
class SuggestionView : public views::Button, public views::ButtonListener {
public:
explicit SuggestionView(int index) { InitLayout(index); }
SuggestionView(AssistantViewDelegate* delegate,
AssistantSuggestionPtr suggestion,
int index)
: views::Button(this),
delegate_(delegate),
suggestion_(std::move(suggestion)),
index_(index) {
InitLayout();
}

SuggestionView(const SuggestionView&) = delete;
SuggestionView& operator=(const SuggestionView&) = delete;
~SuggestionView() override = default;

// views::View:
// views::Button:
const char* GetClassName() const override { return "SuggestionView"; }

int GetHeightForWidth(int width) const override {
Expand All @@ -129,11 +146,16 @@ class SuggestionView : public views::View {
PreferredSizeChanged();
}

// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override {
delegate_->OnSuggestionChipPressed(suggestion_.get());
}

private:
void InitLayout(int index) {
void InitLayout() {
// Background.
SetBackground(views::CreateRoundedRectBackground(
GetSuggestionBackgroundColor(index), kSuggestionsCornerRadiusDip));
GetSuggestionBackgroundColor(index_), kSuggestionsCornerRadiusDip));

// Layout.
SetLayoutManager(std::make_unique<views::FlexLayout>())
Expand All @@ -146,15 +168,19 @@ class SuggestionView : public views::View {

// Icon.
icon_ = AddChildView(std::make_unique<views::ImageView>());
icon_->SetBackground(views::CreateRoundedRectBackground(
SkColorSetA(SK_ColorBLACK, 0.04 * 0xFF), kSuggestionsIconSizeDip / 2));
icon_->SetImageSize({kSuggestionsIconSizeDip, kSuggestionsIconSizeDip});
icon_->SetPreferredSize({kSuggestionsIconSizeDip, kSuggestionsIconSizeDip});

if (suggestion_->icon_url.is_valid()) {
delegate_->DownloadImage(suggestion_->icon_url,
base::BindOnce(&SuggestionView::OnIconDownloaded,
weak_factory_.GetWeakPtr()));
}

// Label.
label_ = AddChildView(std::make_unique<views::Label>());
label_->SetAutoColorReadabilityEnabled(false);
label_->SetEnabledColor(GetSuggestionTextColor(index));
label_->SetEnabledColor(GetSuggestionTextColor(index_));
label_->SetFontList(assistant::ui::GetDefaultFontList().DeriveWithSizeDelta(
kSuggestionsLabelSizeDelta));
label_->SetHorizontalAlignment(gfx::HorizontalAlignment::ALIGN_LEFT);
Expand All @@ -167,11 +193,22 @@ class SuggestionView : public views::View {
views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero,
views::MaximumFlexSizeRule::kUnbounded,
/*adjust_height_for_width=*/true));
label_->SetText(base::UTF8ToUTF16(suggestion_->text));
}

private:
void OnIconDownloaded(const gfx::ImageSkia& icon) {
if (!icon.isNull())
icon_->SetImage(icon);
}

AssistantViewDelegate* const delegate_;
const AssistantSuggestionPtr suggestion_;
const int index_;

views::ImageView* icon_; // Owned by view hierarchy.
views::Label* label_; // Owned by view hierarchy.

base::WeakPtrFactory<SuggestionView> weak_factory_{this};
};

} // namespace
Expand Down Expand Up @@ -257,7 +294,6 @@ void AssistantOnboardingView::InitLayout() {
InitSuggestions();
}

// TODO(dmblack): Populate suggestions from model.
void AssistantOnboardingView::InitSuggestions() {
auto* grid = AddChildView(std::make_unique<views::View>());
grid->SetBorder(views::CreateEmptyBorder(kSuggestionsMarginTopDip, 0, 0, 0));
Expand All @@ -279,8 +315,19 @@ void AssistantOnboardingView::InitSuggestions() {
/*fixed_width=*/0, /*min_width=*/0);
}

// TODO(dmblack): Populate suggestions from model.
std::vector<AssistantSuggestionPtr> suggestions;
suggestions.push_back(AssistantSuggestion::New(
/*id=*/base::UnguessableToken::Create(),
/*type=*/AssistantSuggestionType::kBetterOnboarding,
/*text=*/"Square root of 144",
/*icon_url=*/
GURL("https://www.gstatic.com/images/branding/product/2x/"
"googleg_48dp.png"),
/*action_url=*/GURL()));

// Initialize suggestions.
for (int i = 0; i < kSuggestionsMaxCount; ++i) {
for (size_t i = 0; i < suggestions.size() && i < kSuggestionsMaxCount; ++i) {
if (i % kSuggestionsColumnCount == 0) {
if (i > 0) {
layout->StartRowWithPadding(
Expand All @@ -293,7 +340,8 @@ void AssistantOnboardingView::InitSuggestions() {
/*column_set_id=*/kSuggestionsColumnSetId);
}
}
layout->AddView(std::make_unique<SuggestionView>(/*index=*/i));
layout->AddView(std::make_unique<SuggestionView>(
delegate_, std::move(suggestions[i]), i));
}
}

Expand Down
Loading

0 comments on commit 2653bc7

Please sign in to comment.