Commit 375ff547 authored by Vadim Tryshev's avatar Vadim Tryshev Committed by Commit Bot

Starting cleaning up "pseudo-focus" logic.

It was replaced by setting actual focus. To avoid a huge CL start with
hardcoding the flag as true and some small obvious cleanups.

Bug: 766807
Change-Id: I3d16523e276ad4bddc772f516fc3ff1d5d5d8e5f
Reviewed-on: https://chromium-review.googlesource.com/804685
Commit-Queue: Vadim Tryshev <vadimt@chromium.org>
Reviewed-by: default avatarYury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523954}
parent fb56b10e
...@@ -19,7 +19,6 @@ ...@@ -19,7 +19,6 @@
#include "content/public/common/renderer_preferences.h" #include "content/public/common/renderer_preferences.h"
#include "net/http/http_response_headers.h" #include "net/http/http_response_headers.h"
#include "net/http/http_status_code.h" #include "net/http/http_status_code.h"
#include "ui/app_list/app_list_features.h"
#include "ui/app_list/views/app_list_view.h" #include "ui/app_list/views/app_list_view.h"
#include "ui/aura/window.h" #include "ui/aura/window.h"
#include "ui/views/controls/native/native_view_host.h" #include "ui/views/controls/native/native_view_host.h"
...@@ -73,8 +72,7 @@ class SearchAnswerWebView : public views::WebView { ...@@ -73,8 +72,7 @@ class SearchAnswerWebView : public views::WebView {
OnVisibilityEvent(false); OnVisibilityEvent(false);
// Focus Behavior is originally set in WebView::SetWebContents, but // Focus Behavior is originally set in WebView::SetWebContents, but
// overriden here because we do not want the webview to get focus. // overriden here because we do not want the webview to get focus.
if (features::IsAppListFocusEnabled()) SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
SetFocusBehavior(FocusBehavior::ACCESSIBLE_ONLY);
} }
void RemovedFromWidget() override { void RemovedFromWidget() override {
......
...@@ -16,8 +16,6 @@ const base::Feature kEnableBackgroundBlur{"EnableBackgroundBlur", ...@@ -16,8 +16,6 @@ const base::Feature kEnableBackgroundBlur{"EnableBackgroundBlur",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kEnablePlayStoreAppSearch{"EnablePlayStoreAppSearch", const base::Feature kEnablePlayStoreAppSearch{"EnablePlayStoreAppSearch",
base::FEATURE_ENABLED_BY_DEFAULT}; base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kEnableAppListFocus{"EnableAppListFocus",
base::FEATURE_ENABLED_BY_DEFAULT};
bool IsAnswerCardEnabled() { bool IsAnswerCardEnabled() {
// Not using local static variable to allow tests to change this value. // Not using local static variable to allow tests to change this value.
...@@ -40,7 +38,8 @@ bool IsPlayStoreAppSearchEnabled() { ...@@ -40,7 +38,8 @@ bool IsPlayStoreAppSearchEnabled() {
} }
bool IsAppListFocusEnabled() { bool IsAppListFocusEnabled() {
return base::FeatureList::IsEnabled(kEnableAppListFocus); // TODO(766807): Remove this method.
return true;
} }
std::string AnswerServerUrl() { std::string AnswerServerUrl() {
......
...@@ -29,13 +29,6 @@ APP_LIST_EXPORT extern const base::Feature kEnableBackgroundBlur; ...@@ -29,13 +29,6 @@ APP_LIST_EXPORT extern const base::Feature kEnableBackgroundBlur;
// Enables the Play Store app search. // Enables the Play Store app search.
APP_LIST_EXPORT extern const base::Feature kEnablePlayStoreAppSearch; APP_LIST_EXPORT extern const base::Feature kEnablePlayStoreAppSearch;
// Enables the app list focus. In this mode, many views become focusable. Focus
// transition are handled by FocusManager and accessibility focus transition can
// be triggered properly on search+arrow key as standard.
// TODO(weidongg/766807) Remove this flag when the related changes become
// stable.
APP_LIST_EXPORT extern const base::Feature kEnableAppListFocus;
bool APP_LIST_EXPORT IsAnswerCardEnabled(); bool APP_LIST_EXPORT IsAnswerCardEnabled();
bool APP_LIST_EXPORT IsBackgroundBlurEnabled(); bool APP_LIST_EXPORT IsBackgroundBlurEnabled();
bool APP_LIST_EXPORT IsFullscreenAppListEnabled(); bool APP_LIST_EXPORT IsFullscreenAppListEnabled();
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "ui/accessibility/ax_node_data.h" #include "ui/accessibility/ax_node_data.h"
#include "ui/app_list/app_list_constants.h" #include "ui/app_list/app_list_constants.h"
#include "ui/app_list/app_list_features.h"
#include "ui/app_list/app_list_switches.h" #include "ui/app_list/app_list_switches.h"
#include "ui/app_list/views/apps_grid_view.h" #include "ui/app_list/views/apps_grid_view.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -66,8 +65,7 @@ AppListItemView::AppListItemView(AppsGridView* apps_grid_view, ...@@ -66,8 +65,7 @@ AppListItemView::AppListItemView(AppsGridView* apps_grid_view,
icon_(new views::ImageView), icon_(new views::ImageView),
title_(new views::Label), title_(new views::Label),
progress_bar_(new views::ProgressBar) { progress_bar_(new views::ProgressBar) {
if (features::IsAppListFocusEnabled()) SetFocusBehavior(FocusBehavior::ALWAYS);
SetFocusBehavior(FocusBehavior::ALWAYS);
icon_->set_can_process_events_within_subtree(false); icon_->set_can_process_events_within_subtree(false);
icon_->SetVerticalAlignment(views::ImageView::LEADING); icon_->SetVerticalAlignment(views::ImageView::LEADING);
......
...@@ -264,11 +264,9 @@ AppListView::AppListView(AppListViewDelegate* delegate) ...@@ -264,11 +264,9 @@ AppListView::AppListView(AppListViewDelegate* delegate)
display_observer_.Add(display::Screen::GetScreen()); display_observer_.Add(display::Screen::GetScreen());
delegate_->AddObserver(this); delegate_->AddObserver(this);
} }
if (is_app_list_focus_enabled_) { // Enable arrow key in FocusManager. Arrow left/right and up/down triggers
// Enable arrow key in FocusManager. Arrow left/right and up/down triggers // the same focus movement as tab/shift+tab.
// the same focus movement as tab/shift+tab. views::FocusManager::set_arrow_key_traversal_enabled(true);
views::FocusManager::set_arrow_key_traversal_enabled(true);
}
} }
AppListView::~AppListView() { AppListView::~AppListView() {
...@@ -279,10 +277,8 @@ AppListView::~AppListView() { ...@@ -279,10 +277,8 @@ AppListView::~AppListView() {
animation_observer_.reset(); animation_observer_.reset();
// Remove child views first to ensure no remaining dependencies on delegate_. // Remove child views first to ensure no remaining dependencies on delegate_.
RemoveAllChildViews(true); RemoveAllChildViews(true);
if (is_app_list_focus_enabled_) { views::FocusManager::set_arrow_key_traversal_enabled(
views::FocusManager::set_arrow_key_traversal_enabled( previous_arrow_key_traversal_enabled_);
previous_arrow_key_traversal_enabled_);
}
} }
// static // static
...@@ -1409,9 +1405,6 @@ void AppListView::DraggingLayout() { ...@@ -1409,9 +1405,6 @@ void AppListView::DraggingLayout() {
} }
void AppListView::RedirectKeyEventToSearchBox(ui::KeyEvent* event) { void AppListView::RedirectKeyEventToSearchBox(ui::KeyEvent* event) {
if (!is_app_list_focus_enabled_)
return;
if (event->handled()) if (event->handled())
return; return;
......
...@@ -187,9 +187,6 @@ class AppListViewFocusTest : public views::ViewsTestBase, ...@@ -187,9 +187,6 @@ class AppListViewFocusTest : public views::ViewsTestBase,
base::i18n::SetICUDefaultLocale("he"); base::i18n::SetICUDefaultLocale("he");
} }
// Enable app list focus.
scoped_feature_list_.InitAndEnableFeature(features::kEnableAppListFocus);
// Initialize app list view. // Initialize app list view.
delegate_.reset(new AppListTestViewDelegate); delegate_.reset(new AppListTestViewDelegate);
view_ = new AppListView(delegate_.get()); view_ = new AppListView(delegate_.get());
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "ui/app_list/app_list_constants.h" #include "ui/app_list/app_list_constants.h"
#include "ui/app_list/app_list_features.h"
#include "ui/app_list/vector_icons/vector_icons.h" #include "ui/app_list/vector_icons/vector_icons.h"
#include "ui/app_list/views/app_list_view.h" #include "ui/app_list/views/app_list_view.h"
#include "ui/app_list/views/contents_view.h" #include "ui/app_list/views/contents_view.h"
...@@ -68,8 +67,7 @@ ExpandArrowView::ExpandArrowView(ContentsView* contents_view, ...@@ -68,8 +67,7 @@ ExpandArrowView::ExpandArrowView(ContentsView* contents_view,
contents_view_(contents_view), contents_view_(contents_view),
app_list_view_(app_list_view), app_list_view_(app_list_view),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
if (features::IsAppListFocusEnabled()) SetFocusBehavior(FocusBehavior::ALWAYS);
SetFocusBehavior(FocusBehavior::ALWAYS);
SetPaintToLayer(); SetPaintToLayer();
layer()->SetFillsBoundsOpaquely(false); layer()->SetFillsBoundsOpaquely(false);
......
...@@ -261,8 +261,6 @@ void FolderHeaderView::ContentsChanged(views::Textfield* sender, ...@@ -261,8 +261,6 @@ void FolderHeaderView::ContentsChanged(views::Textfield* sender,
bool FolderHeaderView::HandleKeyEvent(views::Textfield* sender, bool FolderHeaderView::HandleKeyEvent(views::Textfield* sender,
const ui::KeyEvent& key_event) { const ui::KeyEvent& key_event) {
if (!features::IsAppListFocusEnabled())
return false;
if (!CanProcessLeftRightKeyTraversal(key_event)) if (!CanProcessLeftRightKeyTraversal(key_event))
return false; return false;
return ProcessLeftRightKeyTraversalForTextfield(folder_name_view_, key_event); return ProcessLeftRightKeyTraversalForTextfield(folder_name_view_, key_event);
......
...@@ -123,8 +123,7 @@ class SearchBoxImageButton : public views::ImageButton { ...@@ -123,8 +123,7 @@ class SearchBoxImageButton : public views::ImageButton {
public: public:
explicit SearchBoxImageButton(views::ButtonListener* listener) explicit SearchBoxImageButton(views::ButtonListener* listener)
: ImageButton(listener), selected_(false) { : ImageButton(listener), selected_(false) {
if (features::IsAppListFocusEnabled()) SetFocusBehavior(FocusBehavior::ALWAYS);
SetFocusBehavior(FocusBehavior::ALWAYS);
} }
~SearchBoxImageButton() override {} ~SearchBoxImageButton() override {}
...@@ -190,17 +189,14 @@ class SearchBoxTextfield : public views::Textfield { ...@@ -190,17 +189,14 @@ class SearchBoxTextfield : public views::Textfield {
} }
void OnFocus() override { void OnFocus() override {
if (is_app_list_focus_enabled_) search_box_view_->SetSelected(true);
search_box_view_->SetSelected(true);
Textfield::OnFocus(); Textfield::OnFocus();
} }
void OnBlur() override { void OnBlur() override {
if (is_app_list_focus_enabled_) { search_box_view_->SetSelected(false);
search_box_view_->SetSelected(false); // Clear selection and set the caret to the end of the text.
// Clear selection and set the caret to the end of the text. ClearSelection();
ClearSelection();
}
Textfield::OnBlur(); Textfield::OnBlur();
} }
...@@ -494,7 +490,7 @@ void SearchBoxView::SetSearchBoxActive(bool active) { ...@@ -494,7 +490,7 @@ void SearchBoxView::SetSearchBoxActive(bool active) {
: search_box_color_); : search_box_color_);
search_box_->SetCursorEnabled(active); search_box_->SetCursorEnabled(active);
if (active && is_app_list_focus_enabled_) if (active)
search_box_->RequestFocus(); search_box_->RequestFocus();
search_box_right_space_->SetVisible(!active); search_box_right_space_->SetVisible(!active);
...@@ -718,12 +714,6 @@ void SearchBoxView::SetSelected(bool selected) { ...@@ -718,12 +714,6 @@ void SearchBoxView::SetSelected(bool selected) {
if (selected) { if (selected) {
// Set the ChromeVox focus to the search box. // Set the ChromeVox focus to the search box.
search_box_->NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true); search_box_->NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true);
if (!is_app_list_focus_enabled_ && !search_box_->text().empty()) {
// If query is not empty (including a string of spaces), we need to select
// the entire text range. When app list focus flag is enabled, query is
// automatically selected all when search box is focused.
search_box_->SelectAll(false);
}
} }
UpdateSearchBoxBorder(); UpdateSearchBoxBorder();
Layout(); Layout();
...@@ -747,10 +737,8 @@ void SearchBoxView::NotifyQueryChanged() { ...@@ -747,10 +737,8 @@ void SearchBoxView::NotifyQueryChanged() {
void SearchBoxView::ContentsChanged(views::Textfield* sender, void SearchBoxView::ContentsChanged(views::Textfield* sender,
const base::string16& new_contents) { const base::string16& new_contents) {
if (is_app_list_focus_enabled_) { // Set search box focused when query changes.
// Set search box focused when query changes. search_box_->RequestFocus();
search_box_->RequestFocus();
}
UpdateModel(true); UpdateModel(true);
view_delegate_->AutoLaunchCanceled(); view_delegate_->AutoLaunchCanceled();
NotifyQueryChanged(); NotifyQueryChanged();
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include "ui/accessibility/ax_node.h" #include "ui/accessibility/ax_node.h"
#include "ui/accessibility/ax_node_data.h" #include "ui/accessibility/ax_node_data.h"
#include "ui/app_list/app_list_constants.h" #include "ui/app_list/app_list_constants.h"
#include "ui/app_list/app_list_features.h"
#include "ui/app_list/app_list_view_delegate.h" #include "ui/app_list/app_list_view_delegate.h"
#include "ui/views/background.h" #include "ui/views/background.h"
#include "ui/views/controls/button/button.h" #include "ui/views/controls/button/button.h"
...@@ -34,8 +33,7 @@ class SearchResultAnswerCardView::SearchAnswerContainerView ...@@ -34,8 +33,7 @@ class SearchResultAnswerCardView::SearchAnswerContainerView
public: public:
explicit SearchAnswerContainerView(AppListViewDelegate* view_delegate) explicit SearchAnswerContainerView(AppListViewDelegate* view_delegate)
: Button(this), view_delegate_(view_delegate) { : Button(this), view_delegate_(view_delegate) {
if (features::IsAppListFocusEnabled()) SetFocusBehavior(FocusBehavior::ALWAYS);
SetFocusBehavior(FocusBehavior::ALWAYS);
// Center the card horizontally in the container. // Center the card horizontally in the container.
views::BoxLayout* answer_container_layout = views::BoxLayout* answer_container_layout =
new views::BoxLayout(views::BoxLayout::kHorizontal, new views::BoxLayout(views::BoxLayout::kHorizontal,
...@@ -94,16 +92,13 @@ class SearchResultAnswerCardView::SearchAnswerContainerView ...@@ -94,16 +92,13 @@ class SearchResultAnswerCardView::SearchAnswerContainerView
} }
void OnBlur() override { void OnBlur() override {
if (features::IsAppListFocusEnabled()) SetSelected(false);
SetSelected(false);
Button::OnBlur(); Button::OnBlur();
} }
void OnFocus() override { void OnFocus() override {
if (features::IsAppListFocusEnabled()) { SetSelected(true);
SetSelected(true); NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true);
NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true);
}
Button::OnFocus(); Button::OnFocus();
} }
......
...@@ -70,11 +70,8 @@ class ZeroWidthVerticalScrollBar : public views::OverlayScrollBar { ...@@ -70,11 +70,8 @@ class ZeroWidthVerticalScrollBar : public views::OverlayScrollBar {
int GetThickness() const override { return 0; } int GetThickness() const override { return 0; }
bool OnKeyPressed(const ui::KeyEvent& event) override { bool OnKeyPressed(const ui::KeyEvent& event) override {
if (!features::IsAppListFocusEnabled())
return OverlayScrollBar::OnKeyPressed(event);
// Arrow keys should be handled by FocusManager to move focus. When a search // Arrow keys should be handled by FocusManager to move focus. When a search
// result is focued, it will be set visible in scroll view. // result is focused, it will be set visible in scroll view.
return false; return false;
} }
......
...@@ -63,8 +63,7 @@ SearchResultView::SearchResultView(SearchResultListView* list_view) ...@@ -63,8 +63,7 @@ SearchResultView::SearchResultView(SearchResultListView* list_view)
actions_view_(new SearchResultActionsView(this)), actions_view_(new SearchResultActionsView(this)),
progress_bar_(new views::ProgressBar), progress_bar_(new views::ProgressBar),
is_app_list_focus_enabled_(features::IsAppListFocusEnabled()) { is_app_list_focus_enabled_(features::IsAppListFocusEnabled()) {
if (is_app_list_focus_enabled_) SetFocusBehavior(FocusBehavior::ALWAYS);
SetFocusBehavior(FocusBehavior::ALWAYS);
icon_->set_can_process_events_within_subtree(false); icon_->set_can_process_events_within_subtree(false);
badge_icon_->set_can_process_events_within_subtree(false); badge_icon_->set_can_process_events_within_subtree(false);
...@@ -142,7 +141,7 @@ void SearchResultView::SetSelected(bool selected) { ...@@ -142,7 +141,7 @@ void SearchResultView::SetSelected(bool selected) {
return; return;
selected_ = selected; selected_ = selected;
if (is_app_list_focus_enabled_ && selected_) { if (selected_) {
ScrollRectToVisible(GetLocalBounds()); ScrollRectToVisible(GetLocalBounds());
NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true); NotifyAccessibilityEvent(ui::AX_EVENT_SELECTION, true);
} }
......
...@@ -62,8 +62,7 @@ TileItemView::TileItemView() ...@@ -62,8 +62,7 @@ TileItemView::TileItemView()
icon_(new views::ImageView), icon_(new views::ImageView),
badge_(nullptr), badge_(nullptr),
title_(new views::Label) { title_(new views::Label) {
if (features::IsAppListFocusEnabled()) SetFocusBehavior(FocusBehavior::ALWAYS);
SetFocusBehavior(FocusBehavior::ALWAYS);
// Prevent the icon view from interfering with our mouse events. // Prevent the icon view from interfering with our mouse events.
icon_->set_can_process_events_within_subtree(false); icon_->set_can_process_events_within_subtree(false);
icon_->SetVerticalAlignment(views::ImageView::LEADING); icon_->SetVerticalAlignment(views::ImageView::LEADING);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment