Skip to content

Commit

Permalink
Move events handler to a shared target
Browse files Browse the repository at this point in the history
This cl moves the pre target handler and focus search classes to a
place where can be used by both Quick Answers and Editor Menu.

This cl only moves the files and with minor non-functional changes.

Bug: b/295582803
Test: Tested manually and passed current tests.
Change-Id: I2ff6011ef82aa8fa97c8f823acd4ef498c57e1ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4802330
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Yuki Awano <yawano@google.com>
Cr-Commit-Position: refs/heads/main@{#1186609}
  • Loading branch information
Tao Wu authored and Chromium LUCI CQ committed Aug 22, 2023
1 parent 591516b commit a3cf327
Show file tree
Hide file tree
Showing 13 changed files with 129 additions and 98 deletions.
5 changes: 1 addition & 4 deletions chrome/browser/ui/quick_answers/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ source_set("quick_answers") {
"quick_answers_controller_impl.h",
"quick_answers_ui_controller.cc",
"quick_answers_ui_controller.h",
"ui/quick_answers_focus_search.cc",
"ui/quick_answers_focus_search.h",
"ui/quick_answers_pre_target_handler.cc",
"ui/quick_answers_pre_target_handler.h",
"ui/quick_answers_text_label.cc",
"ui/quick_answers_text_label.h",
"ui/quick_answers_util.cc",
Expand All @@ -41,6 +37,7 @@ source_set("quick_answers") {
deps = [
"//chrome/browser/profiles:profile",
"//chrome/browser/ui/color:color_headers",
"//chrome/browser/ui/views/editor_menu:utils",
"//chromeos/components/quick_answers",
"//chromeos/components/quick_answers/public/cpp:cpp",
"//chromeos/components/quick_answers/public/cpp:prefs",
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/quick_answers/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ include_rules = [
# both ash and Lacros browser.
"+ash",
"+content/browser/speech",
"+chrome/browser/ui/views/editor_menu/utils/focus_search.h",
"+chrome/browser/ui/views/editor_menu/utils/pre_target_handler.h",
]
7 changes: 4 additions & 3 deletions chrome/browser/ui/quick_answers/ui/quick_answers_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/ui/color/chrome_color_id.h"
#include "chrome/browser/ui/quick_answers/quick_answers_ui_controller.h"
#include "chrome/browser/ui/quick_answers/ui/quick_answers_pre_target_handler.h"
#include "chrome/browser/ui/quick_answers/ui/quick_answers_text_label.h"
#include "chrome/browser/ui/quick_answers/ui/quick_answers_util.h"
#include "chrome/browser/ui/views/editor_menu/utils/focus_search.h"
#include "chrome/browser/ui/views/editor_menu/utils/pre_target_handler.h"
#include "chromeos/components/quick_answers/quick_answers_model.h"
#include "chromeos/components/quick_answers/utils/quick_answers_metrics.h"
#include "chromeos/constants/chromeos_features.h"
Expand Down Expand Up @@ -324,8 +325,8 @@ QuickAnswersView::QuickAnswersView(
title_(title),
is_internal_(is_internal),
quick_answers_view_handler_(
std::make_unique<QuickAnswersPreTargetHandler>(this)),
focus_search_(std::make_unique<QuickAnswersFocusSearch>(
std::make_unique<chromeos::editor_menu::PreTargetHandler>(this)),
focus_search_(std::make_unique<chromeos::editor_menu::FocusSearch>(
this,
base::BindRepeating(&QuickAnswersView::GetFocusableViews,
base::Unretained(this)))) {
Expand Down
14 changes: 9 additions & 5 deletions chrome/browser/ui/quick_answers/ui/quick_answers_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/ui/quick_answers/ui/quick_answers_focus_search.h"
#include "ui/base/metadata/metadata_header_macros.h"
#include "ui/events/event_handler.h"
#include "ui/views/controls/image_view.h"
Expand All @@ -24,6 +23,11 @@ class LabelButton;
class WebView;
} // namespace views

namespace chromeos::editor_menu {
class FocusSearch;
class PreTargetHandler;
} // namespace chromeos::editor_menu

class QuickAnswersUiController;

namespace quick_answers {
Expand Down Expand Up @@ -94,8 +98,7 @@ class QuickAnswersView : public views::View {
void UpdateBounds();
void UpdateQuickAnswerResult(const quick_answers::QuickAnswer& quick_answer);

// QuickAnswersFocusSearch::GetFocusableViewsCallback to poll currently
// focusable views.
// FocusSearch::GetFocusableViewsCallback to poll currently focusable views.
std::vector<views::View*> GetFocusableViews();

// Invoked when user clicks the phonetics audio button.
Expand Down Expand Up @@ -125,8 +128,9 @@ class QuickAnswersView : public views::View {
// Invisible web view to play phonetics audio for definition results.
raw_ptr<views::WebView> phonetics_audio_web_view_ = nullptr;

std::unique_ptr<QuickAnswersPreTargetHandler> quick_answers_view_handler_;
std::unique_ptr<QuickAnswersFocusSearch> focus_search_;
std::unique_ptr<chromeos::editor_menu::PreTargetHandler>
quick_answers_view_handler_;
std::unique_ptr<chromeos::editor_menu::FocusSearch> focus_search_;
base::WeakPtrFactory<QuickAnswersView> weak_factory_{this};
};

Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/ui/quick_answers/ui/rich_answers_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "chrome/browser/ui/quick_answers/ui/rich_answers_pre_target_handler.h"
#include "chrome/browser/ui/quick_answers/ui/rich_answers_translation_view.h"
#include "chrome/browser/ui/quick_answers/ui/rich_answers_unit_conversion_view.h"
#include "chrome/browser/ui/views/editor_menu/utils/focus_search.h"
#include "chromeos/components/quick_answers/public/cpp/controller/quick_answers_controller.h"
#include "chromeos/components/quick_answers/quick_answers_model.h"
#include "chromeos/strings/grit/chromeos_strings.h"
Expand Down Expand Up @@ -87,7 +88,7 @@ RichAnswersView::RichAnswersView(
result_(result),
rich_answers_view_handler_(
std::make_unique<quick_answers::RichAnswersPreTargetHandler>(this)),
focus_search_(std::make_unique<QuickAnswersFocusSearch>(
focus_search_(std::make_unique<chromeos::editor_menu::FocusSearch>(
this,
base::BindRepeating(&RichAnswersView::GetFocusableViews,
base::Unretained(this)))) {
Expand Down
10 changes: 6 additions & 4 deletions chrome/browser/ui/quick_answers/ui/rich_answers_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/ui/quick_answers/ui/quick_answers_focus_search.h"
#include "chrome/browser/ui/quick_answers/ui/rich_answers_pre_target_handler.h"
#include "ui/base/metadata/metadata_header_macros.h"
#include "ui/events/event_handler.h"
Expand All @@ -24,6 +23,10 @@ class ImageButton;
class ImageView;
} // namespace views

namespace chromeos::editor_menu {
class FocusSearch;
} // namespace chromeos::editor_menu

class QuickAnswersUiController;

namespace quick_answers {
Expand Down Expand Up @@ -68,8 +71,7 @@ class RichAnswersView : public views::View {
void OnGoogleSearchLinkClicked();
void UpdateBounds();

// QuickAnswersFocusSearch::GetFocusableViewsCallback to poll currently
// focusable views.
// FocusSearch::GetFocusableViewsCallback to poll currently focusable views.
std::vector<views::View*> GetFocusableViews();

gfx::Rect anchor_view_bounds_;
Expand All @@ -87,7 +89,7 @@ class RichAnswersView : public views::View {

std::unique_ptr<quick_answers::RichAnswersPreTargetHandler>
rich_answers_view_handler_;
std::unique_ptr<QuickAnswersFocusSearch> focus_search_;
std::unique_ptr<chromeos::editor_menu::FocusSearch> focus_search_;
base::WeakPtrFactory<RichAnswersView> weak_factory_{this};
};

Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/quick_answers/ui/user_consent_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ void UserConsentView::InitButtonBar() {
// Allow button
auto allow_button = std::make_unique<CustomizedLabelButton>(
base::BindRepeating(
[](QuickAnswersPreTargetHandler* handler,
[](chromeos::editor_menu::PreTargetHandler* handler,
base::WeakPtr<QuickAnswersUiController> controller) {
// When user consent is accepted, QuickAnswersView will be
// displayed instead of dismissing the menu.
Expand Down
11 changes: 5 additions & 6 deletions chrome/browser/ui/quick_answers/ui/user_consent_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/ui/quick_answers/ui/quick_answers_focus_search.h"
#include "chrome/browser/ui/quick_answers/ui/quick_answers_pre_target_handler.h"
#include "chrome/browser/ui/views/editor_menu/utils/focus_search.h"
#include "chrome/browser/ui/views/editor_menu/utils/pre_target_handler.h"
#include "ui/base/metadata/metadata_header_macros.h"
#include "ui/views/view.h"
#include "ui/views/widget/unique_widget_ptr.h"
Expand Down Expand Up @@ -65,18 +65,17 @@ class UserConsentView : public views::View {
void InitButtonBar();
void UpdateWidgetBounds();

// QuickAnswersFocusSearch::GetFocusableViewsCallback to poll currently
// focusable views.
// FocusSearch::GetFocusableViewsCallback to poll currently focusable views.
std::vector<views::View*> GetFocusableViews();

// Cached bounds of the anchor this view is tied to.
gfx::Rect anchor_view_bounds_;
// Cached title text.
std::u16string title_text_;

QuickAnswersPreTargetHandler event_handler_;
chromeos::editor_menu::PreTargetHandler event_handler_;
base::WeakPtr<QuickAnswersUiController> controller_;
QuickAnswersFocusSearch focus_search_;
chromeos::editor_menu::FocusSearch focus_search_;

// Owned by view hierarchy.
raw_ptr<views::View> main_view_ = nullptr;
Expand Down
17 changes: 17 additions & 0 deletions chrome/browser/ui/views/editor_menu/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@

assert(is_chromeos)

source_set("utils") {
sources = [
"utils/focus_search.cc",
"utils/focus_search.h",
"utils/pre_target_handler.cc",
"utils/pre_target_handler.h",
]

deps = [
"//base",
"//ui/aura",
"//ui/events",
"//ui/gfx",
"//ui/views",
]
}

source_set("editor_menu") {
sources = [
"editor_menu_controller_impl.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,23 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/ui/quick_answers/ui/quick_answers_focus_search.h"
#include "chrome/browser/ui/views/editor_menu/utils/focus_search.h"

namespace quick_answers {
#include "ui/views/focus/focus_search.h"

QuickAnswersFocusSearch::QuickAnswersFocusSearch(
views::View* view,
const GetFocusableViewsCallback& callback)
: FocusSearch(/*root=*/view, /*cycle=*/true, /*accessibility_mode=*/true),
namespace chromeos::editor_menu {

FocusSearch::FocusSearch(views::View* view,
const GetFocusableViewsCallback& callback)
: views::FocusSearch(/*root=*/view,
/*cycle=*/true,
/*accessibility_mode=*/true),
view_(view),
get_focusable_views_callback_(callback) {}

QuickAnswersFocusSearch::~QuickAnswersFocusSearch() = default;
FocusSearch::~FocusSearch() = default;

views::View* QuickAnswersFocusSearch::FindNextFocusableView(
views::View* FocusSearch::FindNextFocusableView(
views::View* starting_view,
SearchDirection search_direction,
TraversalDirection traversal_direction,
Expand All @@ -27,8 +30,9 @@ views::View* QuickAnswersFocusSearch::FindNextFocusableView(

// The callback provided by |view_| polls the currently focusable views.
auto focusable_views = get_focusable_views_callback_.Run();
if (focusable_views.empty())
if (focusable_views.empty()) {
return nullptr;
}

int delta =
search_direction == FocusSearch::SearchDirection::kForwards ? 1 : -1;
Expand All @@ -49,16 +53,16 @@ views::View* QuickAnswersFocusSearch::FindNextFocusableView(
: focusable_views.back();
}

views::FocusSearch* QuickAnswersFocusSearch::GetFocusSearch() {
views::FocusSearch* FocusSearch::GetFocusSearch() {
return this;
}

views::FocusTraversable* QuickAnswersFocusSearch::GetFocusTraversableParent() {
views::FocusTraversable* FocusSearch::GetFocusTraversableParent() {
return nullptr;
}

views::View* QuickAnswersFocusSearch::GetFocusTraversableParentView() {
views::View* FocusSearch::GetFocusTraversableParentView() {
return nullptr;
}

} // namespace quick_answers
} // namespace chromeos::editor_menu
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,32 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_UI_QUICK_ANSWERS_UI_QUICK_ANSWERS_FOCUS_SEARCH_H_
#define CHROME_BROWSER_UI_QUICK_ANSWERS_UI_QUICK_ANSWERS_FOCUS_SEARCH_H_
#ifndef CHROME_BROWSER_UI_VIEWS_EDITOR_MENU_UTILS_FOCUS_SEARCH_H_
#define CHROME_BROWSER_UI_VIEWS_EDITOR_MENU_UTILS_FOCUS_SEARCH_H_

#include "base/functional/callback.h"
#include <vector>

#include "base/functional/callback_forward.h"
#include "base/memory/raw_ptr.h"
#include "ui/views/focus/focus_manager.h"
#include "ui/views/focus/focus_search.h"

namespace quick_answers {
namespace chromeos::editor_menu {

// This class manages the focus traversal order for elements inside
// Quick-Answers related views.
// Quick Answers or Editor Menu related views.
// TODO(siabhijeet): QuickAnswersView is a menu-companion, so ideally should
// avoid disturbing existing focus. Explore other ways for keyboard
// accessibility.
class QuickAnswersFocusSearch : public views::FocusSearch,
public views::FocusTraversable {
class FocusSearch : public views::FocusSearch, public views::FocusTraversable {
public:
using GetFocusableViewsCallback =
base::RepeatingCallback<std::vector<views::View*>(void)>;

explicit QuickAnswersFocusSearch(views::View* view,
const GetFocusableViewsCallback& callback);

~QuickAnswersFocusSearch() override;
FocusSearch(views::View* view, const GetFocusableViewsCallback& callback);
FocusSearch(const FocusSearch&) = delete;
FocusSearch& operator=(const FocusSearch) = delete;
~FocusSearch() override;

// views::FocusSearch:
views::View* FindNextFocusableView(
Expand All @@ -44,10 +45,10 @@ class QuickAnswersFocusSearch : public views::FocusSearch,
views::View* GetFocusTraversableParentView() override;

private:
const raw_ptr<views::View> view_;
const raw_ptr<views::View> view_ = nullptr;
const GetFocusableViewsCallback get_focusable_views_callback_;
};

} // namespace quick_answers
} // namespace chromeos::editor_menu

#endif // CHROME_BROWSER_UI_QUICK_ANSWERS_UI_QUICK_ANSWERS_FOCUS_SEARCH_H_
#endif // CHROME_BROWSER_UI_VIEWS_EDITOR_MENU_UTILS_FOCUS_SEARCH_H_

0 comments on commit a3cf327

Please sign in to comment.