Skip to content

Commit

Permalink
Cleanup SearchBoxViewBase initialization
Browse files Browse the repository at this point in the history 
Few improvements that reduce number of virtual methods in
SearchBoxViewBase, and make search box initialization easier to follow.

*   Makes SearchBoxViewBase::Init protected and non virtual - the
    SearchBoxView and KSVSearchBoxView now expose easier to use
    Initialize* methods that set up init params as required. Introduces
   `textfield_margins` param to be used for bubble launcher search box.

*   Removes virtual methods for setting up close and assistant buttons
    from SearchBoxViewBase - instead exposes methods to create close and
    assistant button, that SearchBoxView and KSVSearchBoxView can use to
    add buttons, and initialize them.

*   Moves UpdateSearchIcon() virtual method out of SearchBoxViewBase
    (it's only used from app list SearchBoxView).

BUG=1346358

Change-Id: I21700a71cbb44f2ce2bbf0d902c37c43ab5a8925
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3806802
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1031340}
  • Loading branch information
Toni Barzic authored and Chromium LUCI CQ committed Aug 4, 2022
1 parent 0fa9769 commit 4785d75
Show file tree
Hide file tree
Showing 11 changed files with 234 additions and 168 deletions.
8 changes: 1 addition & 7 deletions ash/app_list/views/app_list_bubble_view.cc
View file
Expand Up @@ -246,13 +246,7 @@ void AppListBubbleView::InitContentsView(

search_box_view_ = contents->AddChildView(std::make_unique<SearchBoxView>(
/*delegate=*/this, view_delegate_, /*app_list_view=*/nullptr));
SearchBoxViewBase::InitParams params;
// Show the assistant button until the user types text.
params.show_close_button_when_active = false;
params.create_background = false;
params.animate_changing_search_icon = false;
params.increase_child_view_padding = true;
search_box_view_->Init(params);
search_box_view_->InitializeForBubbleLauncher();

// Skip the assistant button on arrow up/down in app list.
button_focus_skipper_ = std::make_unique<ButtonFocusSkipper>();
Expand Down
6 changes: 1 addition & 5 deletions ash/app_list/views/app_list_view.cc
View file
Expand Up @@ -656,11 +656,7 @@ void AppListView::InitContents() {
auto app_list_main_view = std::make_unique<AppListMainView>(delegate_, this);
search_box_view_ =
new SearchBoxView(app_list_main_view.get(), delegate_, this);
SearchBoxViewBase::InitParams params;
params.show_close_button_when_active = true;
params.create_background = true;
params.animate_changing_search_icon = true;
search_box_view_->Init(params);
search_box_view_->InitializeForFullscreenLauncher();

// Assign |app_list_main_view_| here since it is accessed during Init().
app_list_main_view_ = app_list_main_view.get();
Expand Down
135 changes: 80 additions & 55 deletions ash/app_list/views/search_box_view.cc
View file
Expand Up @@ -214,25 +214,56 @@ SearchBoxView::SearchBoxView(SearchBoxViewDelegate* delegate,
is_tablet_mode_(view_delegate_->IsInTabletMode()) {
AppListModelProvider* const model_provider = AppListModelProvider::Get();
model_provider->AddObserver(this);
search_box_model_observer_.Observe(
model_provider->search_model()->search_box());
SearchBoxModel* const search_box_model =
model_provider->search_model()->search_box();
search_box_model_observer_.Observe(search_box_model);

views::ImageButton* close_button = CreateCloseButton(base::BindRepeating(
&SearchBoxView::CloseButtonPressed, base::Unretained(this)));
std::u16string close_button_label(
l10n_util::GetStringUTF16(IDS_APP_LIST_CLEAR_SEARCHBOX));
close_button->SetAccessibleName(close_button_label);
close_button->SetTooltipText(close_button_label);

views::ImageButton* assistant_button =
CreateAssistantButton(base::BindRepeating(
&SearchBoxView::AssistantButtonPressed, base::Unretained(this)));
assistant_button->SetFlipCanvasOnPaintForRTLUI(false);
std::u16string assistant_button_label(
l10n_util::GetStringUTF16(IDS_APP_LIST_START_ASSISTANT));
assistant_button->SetAccessibleName(assistant_button_label);
assistant_button->SetTooltipText(assistant_button_label);
SetShowAssistantButton(search_box_model->show_assistant_button());
}

SearchBoxView::~SearchBoxView() {
AppListModelProvider::Get()->RemoveObserver(this);
}

void SearchBoxView::Init(const InitParams& params) {
void SearchBoxView::InitializeForBubbleLauncher() {
SearchBoxViewBase::InitParams params;
params.show_close_button_when_active = false;
params.create_background = false;
params.animate_changing_search_icon = false;
params.increase_child_view_padding = true;
// Add margins to the text field because the BoxLayout vertical centering
// does not properly align the text baseline with the icons.
params.textfield_margins = kTextFieldMarginsForAppListBubble;

SearchBoxViewBase::Init(params);

UpdatePlaceholderTextAndAccessibleName();
}

void SearchBoxView::InitializeForFullscreenLauncher() {
SearchBoxViewBase::InitParams params;
params.show_close_button_when_active = true;
params.create_background = true;
params.animate_changing_search_icon = true;

SearchBoxViewBase::Init(params);

UpdatePlaceholderTextAndAccessibleName();
current_query_ = search_box()->GetText();
if (is_app_list_bubble_) {
// Add margins to the text field because the BoxLayout vertical centering
// does not properly align the text baseline with the icons.
search_box()->SetProperty(views::kMarginsKey,
kTextFieldMarginsForAppListBubble);
}
ShowAssistantChanged();
}

void SearchBoxView::SetResultSelectionController(
Expand Down Expand Up @@ -373,20 +404,6 @@ void SearchBoxView::UpdateModel(bool initiated_by_user) {
view_delegate_->StartSearch(new_query);
}

void SearchBoxView::UpdateSearchIcon() {
const bool search_engine_is_google =
AppListModelProvider::Get()->search_model()->search_engine_is_google();
const gfx::VectorIcon& google_icon = is_search_box_active()
? vector_icons::kGoogleColorIcon
: kGoogleBlackIcon;
const gfx::VectorIcon& icon =
search_engine_is_google ? google_icon : kSearchEngineNotGoogleIcon;
SetSearchIconImage(
gfx::CreateVectorIcon(icon, GetSearchBoxIconSize(),
AppListColorProvider::Get()->GetSearchBoxIconColor(
SkColorSetARGB(0xDE, 0x00, 0x00, 0x00))));
}

void SearchBoxView::UpdatePlaceholderTextStyle() {
if (is_app_list_bubble_) {
// The bubble launcher text is always side-aligned.
Expand Down Expand Up @@ -457,8 +474,16 @@ const char* SearchBoxView::GetClassName() const {

void SearchBoxView::OnThemeChanged() {
SearchBoxViewBase::OnThemeChanged();
SetupAssistantButton();
SetupCloseButton();
close_button()->SetImage(
views::ImageButton::STATE_NORMAL,
gfx::CreateVectorIcon(views::kIcCloseIcon, GetSearchBoxIconSize(),
AppListColorProvider::Get()->GetSearchBoxIconColor(
gfx::kGoogleGrey700)));
assistant_button()->SetImage(
views::ImageButton::STATE_NORMAL,
gfx::CreateVectorIcon(chromeos::kAssistantIcon, GetSearchBoxIconSize(),
AppListColorProvider::Get()->GetSearchBoxIconColor(
gfx::kGoogleGrey700)));
OnWallpaperColorsChanged();
}

Expand All @@ -480,20 +505,6 @@ void SearchBoxView::MaybeCreateFocusRing() {
}
}

void SearchBoxView::SetupCloseButton() {
views::ImageButton* close = close_button();
close->SetImage(
views::ImageButton::STATE_NORMAL,
gfx::CreateVectorIcon(views::kIcCloseIcon, GetSearchBoxIconSize(),
AppListColorProvider::Get()->GetSearchBoxIconColor(
gfx::kGoogleGrey700)));
close->SetVisible(false);
std::u16string close_button_label(
l10n_util::GetStringUTF16(IDS_APP_LIST_CLEAR_SEARCHBOX));
close->SetAccessibleName(close_button_label);
close->SetTooltipText(close_button_label);
}

void SearchBoxView::SetupBackButton() {
views::ImageButton* back = back_button();
ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
Expand Down Expand Up @@ -537,6 +548,11 @@ void SearchBoxView::RecordSearchBoxActivationHistogram(
}

void SearchBoxView::OnSearchBoxActiveChanged(bool active) {
UpdateSearchIcon();

// Clear ghost text when toggling search box active state.
MaybeSetAutocompleteGhostText(std::u16string(), std::u16string());

if (active) {
result_selection_controller_->ResetSelection(nullptr,
true /* default_selection */);
Expand Down Expand Up @@ -589,7 +605,7 @@ void SearchBoxView::OnKeyEvent(ui::KeyEvent* evt) {
return;
}

SearchBoxViewBase::OnKeyEvent(evt);
delegate()->OnSearchBoxKeyEvent(evt);
}

bool SearchBoxView::OnMouseWheel(const ui::MouseWheelEvent& event) {
Expand Down Expand Up @@ -837,6 +853,28 @@ int SearchBoxView::GetSearchBoxButtonSize() {
return kClassicSearchBoxButtonSizeDip;
}

void SearchBoxView::CloseButtonPressed() {
delegate()->CloseButtonPressed();
}

void SearchBoxView::AssistantButtonPressed() {
delegate()->AssistantButtonPressed();
}

void SearchBoxView::UpdateSearchIcon() {
const bool search_engine_is_google =
AppListModelProvider::Get()->search_model()->search_engine_is_google();
const gfx::VectorIcon& google_icon = is_search_box_active()
? vector_icons::kGoogleColorIcon
: kGoogleBlackIcon;
const gfx::VectorIcon& icon =
search_engine_is_google ? google_icon : kSearchEngineNotGoogleIcon;
SetSearchIconImage(
gfx::CreateVectorIcon(icon, GetSearchBoxIconSize(),
AppListColorProvider::Get()->GetSearchBoxIconColor(
SkColorSetARGB(0xDE, 0x00, 0x00, 0x00))));
}

bool SearchBoxView::IsValidAutocompleteText(
const std::u16string& autocomplete_text) {
// Don't set autocomplete text if it's the same as current search box
Expand Down Expand Up @@ -1273,17 +1311,4 @@ void SearchBoxView::ResetHighlightRange() {
highlight_range_.set_end(text_length);
}

void SearchBoxView::SetupAssistantButton() {
views::ImageButton* assistant = assistant_button();
assistant->SetImage(
views::ImageButton::STATE_NORMAL,
gfx::CreateVectorIcon(chromeos::kAssistantIcon, GetSearchBoxIconSize(),
AppListColorProvider::Get()->GetSearchBoxIconColor(
gfx::kGoogleGrey700)));
std::u16string assistant_button_label(
l10n_util::GetStringUTF16(IDS_APP_LIST_START_ASSISTANT));
assistant->SetAccessibleName(assistant_button_label);
assistant->SetTooltipText(assistant_button_label);
}

} // namespace ash
21 changes: 17 additions & 4 deletions ash/app_list/views/search_box_view.h
View file
Expand Up @@ -58,6 +58,14 @@ class ASH_EXPORT SearchBoxView : public SearchBoxViewBase,

~SearchBoxView() override;

// Initializes the search box style for usage in bubble (clamshell mode)
// launcher.
void InitializeForBubbleLauncher();

// Initializes the search box style for usage in fullscreen (tablet mode)
// launcher.
void InitializeForFullscreenLauncher();

// Must be called before the user interacts with the search box. Cannot be
// part of Init() because the controller isn't available until after Init()
// is called.
Expand All @@ -76,18 +84,14 @@ class ASH_EXPORT SearchBoxView : public SearchBoxViewBase,
void MaybeCreateFocusRing();

// Overridden from SearchBoxViewBase:
void Init(const InitParams& params) override;
void UpdateSearchTextfieldAccessibleNodeData(
ui::AXNodeData* node_data) override;
void ClearSearch() override;
void HandleSearchBoxEvent(ui::LocatedEvent* located_event) override;
void UpdateKeyboardVisibility() override;
void UpdateModel(bool initiated_by_user) override;
void UpdateSearchIcon() override;
void UpdatePlaceholderTextStyle() override;
void UpdateSearchBoxBorder() override;
void SetupAssistantButton() override;
void SetupCloseButton() override;
void SetupBackButton() override;
void RecordSearchBoxActivationHistogram(ui::EventType event_type) override;
void OnSearchBoxActiveChanged(bool active) override;
Expand Down Expand Up @@ -174,6 +178,15 @@ class ASH_EXPORT SearchBoxView : public SearchBoxViewBase,
private:
class FocusRingLayer;

// Called when the close button within the search box gets pressed.
void CloseButtonPressed();

// Called when the assistant button within the search box gets pressed.
void AssistantButtonPressed();

// Updates the icon shown left of the search box texfield.
void UpdateSearchIcon();

// Whether 'autocomplete_text' is a valid candidate for classic highlighted
// autocomplete.
bool IsValidAutocompleteText(const std::u16string& autocomplete_text);
Expand Down
39 changes: 22 additions & 17 deletions ash/app_list/views/search_box_view_unittest.cc
View file
Expand Up @@ -146,10 +146,11 @@ class SearchBoxViewTest : public views::test::WidgetTest,
view = std::make_unique<SearchBoxView>(this, &view_delegate_,
app_list_view_);
}
SearchBoxViewBase::InitParams params;
params.show_close_button_when_active = true;
params.create_background = true;
view->Init(params);

if (IsProductivityLauncherEnabled())
view->InitializeForBubbleLauncher();
else
view->InitializeForFullscreenLauncher();
view_ = widget_->GetContentsView()->AddChildView(std::move(view));

if (IsProductivityLauncherEnabled()) {
Expand Down Expand Up @@ -271,6 +272,12 @@ class SearchBoxViewTest : public views::test::WidgetTest,
->OnSearchResultContainerResultsChanged();
}

void SimulateQuery(const std::u16string& query) {
view()->search_box()->InsertText(
u"test",
ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
}

// Overridden from SearchBoxViewDelegate:
void QueryChanged(SearchBoxViewBase* sender) override {}
void AssistantButtonPressed() override {}
Expand Down Expand Up @@ -314,9 +321,10 @@ TEST_P(SearchBoxViewTest, CloseButtonVisibleAfterTyping) {

// Tests that the close button is still visible after the search box is
// activated (in zero state).
TEST_P(SearchBoxViewTest, CloseButtonIVisibleInZeroStateSearchBox) {
TEST_P(SearchBoxViewTest, CloseButtonVisibleInZeroStateSearchBox) {
SetSearchBoxActive(true, ui::ET_MOUSE_PRESSED);
EXPECT_TRUE(view()->close_button()->GetVisible());
EXPECT_EQ(!IsProductivityLauncherEnabled(),
view()->close_button()->GetVisible());
}

// Tests that the search box is inactive by default.
Expand Down Expand Up @@ -406,8 +414,7 @@ TEST_P(SearchBoxViewTest, SearchBoxActiveSearchEngineNotGoogle) {
// Tests that traversing search results is disabled while results are being
// updated.
TEST_P(SearchBoxViewTest, ChangeSelectionWhileResultsAreChanging) {
SetSearchBoxActive(true, ui::ET_UNKNOWN);
view()->search_box()->SetText(u"test");
SimulateQuery(u"test");
CreateSearchResult(ash::SearchResultDisplayType::kList, 0.7, u"tester",
std::u16string());
CreateSearchResult(ash::SearchResultDisplayType::kList, 0.5, u"testing",
Expand Down Expand Up @@ -452,8 +459,8 @@ TEST_P(SearchBoxViewTest, ChangeSelectionWhileResultsAreChanging) {
// Tests that traversing search results is disabled while the result that would
// be selected next is being removed from results.
TEST_P(SearchBoxViewTest, ChangeSelectionWhileResultsAreBeingRemoved) {
SetSearchBoxActive(true, ui::ET_UNKNOWN);
view()->search_box()->SetText(u"test");
SimulateQuery(u"test");

CreateSearchResult(ash::SearchResultDisplayType::kList, 0.7, u"tester",
std::u16string());
CreateSearchResult(ash::SearchResultDisplayType::kList, 0.5, u"testing",
Expand Down Expand Up @@ -494,8 +501,7 @@ TEST_P(SearchBoxViewTest, ChangeSelectionWhileResultsAreBeingRemoved) {
}

TEST_P(SearchBoxViewTest, UserSelectionNotOverridenByNewResults) {
SetSearchBoxActive(true, ui::ET_UNKNOWN);
view()->search_box()->SetText(u"test");
SimulateQuery(u"test");
CreateSearchResult(ash::SearchResultDisplayType::kList, 0.7, u"tester",
std::u16string());
CreateSearchResult(ash::SearchResultDisplayType::kList, 0.5, u"testing",
Expand Down Expand Up @@ -563,8 +569,7 @@ TEST_P(SearchBoxViewTest, UserSelectionNotOverridenByNewResults) {

TEST_P(SearchBoxViewTest,
UserSelectionInNonDefaultContainerNotOverridenByNewResults) {
SetSearchBoxActive(true, ui::ET_UNKNOWN);
view()->search_box()->SetText(u"test");
SimulateQuery(u"test");
CreateSearchResult(ash::SearchResultDisplayType::kList, 0.7, u"tester",
std::u16string());
CreateSearchResult(ash::SearchResultDisplayType::kList, 0.5, u"testing",
Expand Down Expand Up @@ -641,8 +646,7 @@ TEST_P(SearchBoxViewTest,
// Tests that the default selection is reset after resetting and reactivating
// the search box.
TEST_P(SearchBoxViewTest, ResetSelectionAfterResettingSearchBox) {
SetSearchBoxActive(true, ui::ET_UNKNOWN);
view()->search_box()->SetText(u"test");
SimulateQuery(u"test");
CreateSearchResult(ash::SearchResultDisplayType::kList, 0.7, u"test1",
std::u16string());
CreateSearchResult(ash::SearchResultDisplayType::kList, 0.5, u"test2",
Expand Down Expand Up @@ -926,7 +930,8 @@ TEST_P(SearchBoxViewAssistantButtonTest,

// Assistant button is not showing up under zero state.
KeyPress(ui::VKEY_BACK);
EXPECT_FALSE(view()->assistant_button()->GetVisible());
EXPECT_EQ(IsProductivityLauncherEnabled(),
view()->assistant_button()->GetVisible());
}

class SearchBoxViewAutocompleteTest : public SearchBoxViewTest {
Expand Down

0 comments on commit 4785d75

Please sign in to comment.