Skip to content

Commit

Permalink
ash: Eliminate AppsGridViewFocusDelegate from app list
Browse files Browse the repository at this point in the history
Now that we have AppListKeyboardController and AppListViewProvider we
can inject the keyboard controller into the various apps grid views.
Then AppsGridView can directly call the method to move focus up from
the apps grid, eliminating a delegate interface.

Bug: 1346037
Test: existing ash_unittests
Change-Id: I41a04772e1104a47f39e50c4e71f5533fffb36aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3784458
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1028361}
  • Loading branch information
James Cook authored and Chromium LUCI CQ committed Jul 26, 2022
1 parent 7336d4a commit f57945e
Show file tree
Hide file tree
Showing 13 changed files with 27 additions and 75 deletions.
1 change: 0 additions & 1 deletion ash/app_list/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ source_set("app_list") {
"views/apps_grid_context_menu.h",
"views/apps_grid_view.cc",
"views/apps_grid_view.h",
"views/apps_grid_view_focus_delegate.h",
"views/apps_grid_view_folder_delegate.h",
"views/assistant/app_list_bubble_assistant_page.cc",
"views/assistant/app_list_bubble_assistant_page.h",
Expand Down
6 changes: 1 addition & 5 deletions ash/app_list/views/app_list_bubble_apps_page.cc
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ AppListBubbleAppsPage::AppListBubbleAppsPage(
scroll_contents->AddChildView(std::make_unique<ScrollableAppsGridView>(
a11y_announcer, view_delegate,
/*folder_delegate=*/nullptr, scroll_view_, folder_controller,
/*focus_delegate=*/this));
app_list_keyboard_controller_.get()));
scrollable_apps_grid_view_->SetDragAndDropHostOfCurrentAppList(
drag_and_drop_host);
scrollable_apps_grid_view_->Init();
Expand Down Expand Up @@ -590,10 +590,6 @@ void AppListBubbleAppsPage::OnNudgeRemoved() {
gfx::Tween::ACCEL_40_DECEL_100_3, base::DoNothing());
}

bool AppListBubbleAppsPage::MoveFocusUpFromAppsGrid(int column) {
return app_list_keyboard_controller_->MoveFocusUpFromAppsGrid(column);
}

ContinueSectionView* AppListBubbleAppsPage::GetContinueSectionView() {
return continue_section_;
}
Expand Down
5 changes: 0 additions & 5 deletions ash/app_list/views/app_list_bubble_apps_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "ash/app_list/app_list_view_provider.h"
#include "ash/app_list/views/app_list_nudge_controller.h"
#include "ash/app_list/views/app_list_toast_container_view.h"
#include "ash/app_list/views/apps_grid_view_focus_delegate.h"
#include "ash/ash_export.h"
#include "base/callback_forward.h"
#include "base/memory/weak_ptr.h"
Expand Down Expand Up @@ -60,7 +59,6 @@ class ASH_EXPORT AppListBubbleAppsPage
public views::ViewObserver,
public AppListModelProvider::Observer,
public AppListToastContainerView::Delegate,
public AppsGridViewFocusDelegate,
public AppListViewProvider {
public:
METADATA_HEADER(AppListBubbleAppsPage);
Expand Down Expand Up @@ -129,9 +127,6 @@ class ASH_EXPORT AppListBubbleAppsPage
// AppListToastContainerView::Delegate:
void OnNudgeRemoved() override;

// AppsGridViewFocusDelegate:
bool MoveFocusUpFromAppsGrid(int column) override;

// AppListViewProvider:
ContinueSectionView* GetContinueSectionView() override;
RecentAppsView* GetRecentAppsView() override;
Expand Down
4 changes: 2 additions & 2 deletions ash/app_list/views/app_list_folder_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ void AppListFolderView::CreatePagedAppsGrid(ContentsView* contents_view) {
PagedAppsGridView* items_grid_view =
contents_container_->AddChildView(std::make_unique<PagedAppsGridView>(
contents_view, a11y_announcer_, this, /*folder_controller=*/nullptr,
/*container_delegate=*/this, /*focus_delegate=*/nullptr));
/*container_delegate=*/this, /*keyboard_controller=*/nullptr));
contents_container_->layer()->SetMasksToBounds(true);
items_grid_view_ = items_grid_view;

Expand Down Expand Up @@ -785,7 +785,7 @@ void AppListFolderView::CreateScrollableAppsGrid() {
auto* items_grid_view =
scroll_contents->AddChildView(std::make_unique<ScrollableAppsGridView>(
a11y_announcer_, view_delegate_, this, scroll_view_,
/*folder_controller=*/nullptr, /*focus_delegate=*/nullptr));
/*folder_controller=*/nullptr, /*keyboard_controller=*/nullptr));
items_grid_view_ = items_grid_view;
items_grid_view->Init();
items_grid_view->SetMaxColumns(kMaxFolderColumns);
Expand Down
16 changes: 6 additions & 10 deletions ash/app_list/views/apps_container_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -324,12 +324,12 @@ AppsContainerView::AppsContainerView(ContentsView* contents_view)
std::make_unique<SuggestionChipContainerView>(contents_view), 0);
}

apps_grid_view_ = scrollable_container_->AddChildView(
std::make_unique<PagedAppsGridView>(contents_view, a11y_announcer,
/*folder_delegate=*/nullptr,
/*folder_controller=*/this,
/*container_delegate=*/this,
/*focus_delegate=*/this));
apps_grid_view_ =
scrollable_container_->AddChildView(std::make_unique<PagedAppsGridView>(
contents_view, a11y_announcer,
/*folder_delegate=*/nullptr,
/*folder_controller=*/this,
/*container_delegate=*/this, app_list_keyboard_controller_.get()));
apps_grid_view_->Init();
apps_grid_view_->pagination_model()->AddObserver(this);
if (features::IsProductivityLauncherEnabled())
Expand Down Expand Up @@ -738,10 +738,6 @@ void AppsContainerView::OnNudgeRemoved() {
apps_grid_view_->AnimateOnNudgeRemoved();
}

bool AppsContainerView::MoveFocusUpFromAppsGrid(int column) {
return app_list_keyboard_controller_->MoveFocusUpFromAppsGrid(column);
}

void AppsContainerView::UpdateForNewSortingOrder(
const absl::optional<AppListSortOrder>& new_order,
bool animate,
Expand Down
5 changes: 0 additions & 5 deletions ash/app_list/views/apps_container_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "ash/app_list/views/app_list_nudge_controller.h"
#include "ash/app_list/views/app_list_page.h"
#include "ash/app_list/views/app_list_toast_container_view.h"
#include "ash/app_list/views/apps_grid_view_focus_delegate.h"
#include "ash/app_list/views/paged_apps_grid_view.h"
#include "ash/app_list/views/search_result_page_dialog_controller.h"
#include "ash/ash_export.h"
Expand Down Expand Up @@ -52,7 +51,6 @@ class ASH_EXPORT AppsContainerView
public PaginationModelObserver,
public PagedAppsGridView::ContainerDelegate,
public AppListToastContainerView::Delegate,
public AppsGridViewFocusDelegate,
public views::FocusChangeListener,
public AppListViewProvider {
public:
Expand Down Expand Up @@ -182,9 +180,6 @@ class ASH_EXPORT AppsContainerView
// AppListToastContainerView::Delegate:
void OnNudgeRemoved() override;

// AppsGridViewFocusDelegate:
bool MoveFocusUpFromAppsGrid(int column) override;

// Handles `AppListController::UpdateAppListWithNewSortingOrder()` for the
// app list container.
void UpdateForNewSortingOrder(
Expand Down
10 changes: 5 additions & 5 deletions ash/app_list/views/apps_grid_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
#include "ash/app_list/views/app_list_drag_and_drop_host.h"
#include "ash/app_list/views/app_list_folder_controller.h"
#include "ash/app_list/views/app_list_item_view.h"
#include "ash/app_list/views/app_list_keyboard_controller.h"
#include "ash/app_list/views/app_list_view_util.h"
#include "ash/app_list/views/apps_grid_context_menu.h"
#include "ash/app_list/views/apps_grid_view_focus_delegate.h"
#include "ash/app_list/views/apps_grid_view_folder_delegate.h"
#include "ash/app_list/views/ghost_image_view.h"
#include "ash/app_list/views/pulsing_block_view.h"
Expand Down Expand Up @@ -261,12 +261,12 @@ AppsGridView::AppsGridView(AppListA11yAnnouncer* a11y_announcer,
AppListViewDelegate* app_list_view_delegate,
AppsGridViewFolderDelegate* folder_delegate,
AppListFolderController* folder_controller,
AppsGridViewFocusDelegate* focus_delegate)
AppListKeyboardController* keyboard_controller)
: folder_delegate_(folder_delegate),
folder_controller_(folder_controller),
a11y_announcer_(a11y_announcer),
app_list_view_delegate_(app_list_view_delegate),
focus_delegate_(focus_delegate) {
keyboard_controller_(keyboard_controller) {
DCHECK(a11y_announcer_);
DCHECK(app_list_view_delegate_);
// Top-level grids must have a folder controller.
Expand Down Expand Up @@ -1864,8 +1864,8 @@ bool AppsGridView::HandleVerticalFocusMovement(bool arrow_up) {

if (target_page < 0) {
// Move focus up outside the apps grid if target page is negative.
if (focus_delegate_ &&
focus_delegate_->MoveFocusUpFromAppsGrid(target_col)) {
if (keyboard_controller_ &&
keyboard_controller_->MoveFocusUpFromAppsGrid(target_col)) {
// The delegate handled the focus move.
return true;
}
Expand Down
9 changes: 5 additions & 4 deletions ash/app_list/views/apps_grid_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ class AppListFolderController;
class AppListItem;
class AppListItemList;
class AppListItemView;
class AppListKeyboardController;
class AppListModel;
class AppListViewDelegate;
class AppsGridContextMenu;
class AppsGridViewFocusDelegate;
class AppsGridViewFolderDelegate;
class PulsingBlockView;
class AppsGridRowChangeAnimator;
Expand Down Expand Up @@ -84,7 +84,7 @@ class ASH_EXPORT AppsGridView : public views::View,
AppListViewDelegate* app_list_view_delegate,
AppsGridViewFolderDelegate* folder_delegate,
AppListFolderController* folder_controller,
AppsGridViewFocusDelegate* focus_delegate);
AppListKeyboardController* keyboard_controller);
AppsGridView(const AppsGridView&) = delete;
AppsGridView& operator=(const AppsGridView&) = delete;
~AppsGridView() override;
Expand Down Expand Up @@ -945,8 +945,9 @@ class ASH_EXPORT AppsGridView : public views::View,
AppListA11yAnnouncer* const a11y_announcer_;
AppListViewDelegate* const app_list_view_delegate_;

// May be nullptr if this apps grid doesn't have custom focus handling.
AppsGridViewFocusDelegate* const focus_delegate_;
// May be nullptr if this apps grid doesn't have custom focus handling, for
// example, a folder apps grid.
AppListKeyboardController* const keyboard_controller_;

// Keeps the individual AppListItemView. Owned by views hierarchy.
views::View* items_container_ = nullptr;
Expand Down
29 changes: 0 additions & 29 deletions ash/app_list/views/apps_grid_view_focus_delegate.h

This file was deleted.

5 changes: 2 additions & 3 deletions ash/app_list/views/paged_apps_grid_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "ash/app_list/views/app_list_item_view.h"
#include "ash/app_list/views/app_list_main_view.h"
#include "ash/app_list/views/app_list_view.h"
#include "ash/app_list/views/apps_grid_view_focus_delegate.h"
#include "ash/app_list/views/contents_view.h"
#include "ash/app_list/views/ghost_image_view.h"
#include "ash/constants/ash_features.h"
Expand Down Expand Up @@ -196,12 +195,12 @@ PagedAppsGridView::PagedAppsGridView(
AppsGridViewFolderDelegate* folder_delegate,
AppListFolderController* folder_controller,
ContainerDelegate* container_delegate,
AppsGridViewFocusDelegate* focus_delegate)
AppListKeyboardController* keyboard_controller)
: AppsGridView(a11y_announcer,
contents_view->GetAppListMainView()->view_delegate(),
folder_delegate,
folder_controller,
focus_delegate),
keyboard_controller),
contents_view_(contents_view),
container_delegate_(container_delegate),
page_flip_delay_(kPageFlipDelay),
Expand Down
4 changes: 2 additions & 2 deletions ash/app_list/views/paged_apps_grid_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Layer;

namespace ash {

class AppsGridViewFocusDelegate;
class AppListKeyboardController;
class ContentsView;
class PaginationController;

Expand Down Expand Up @@ -71,7 +71,7 @@ class ASH_EXPORT PagedAppsGridView : public AppsGridView,
AppsGridViewFolderDelegate* folder_delegate,
AppListFolderController* folder_controller,
ContainerDelegate* container_delegate,
AppsGridViewFocusDelegate* focus_delegate);
AppListKeyboardController* keyboard_controller);
PagedAppsGridView(const PagedAppsGridView&) = delete;
PagedAppsGridView& operator=(const PagedAppsGridView&) = delete;
~PagedAppsGridView() override;
Expand Down
4 changes: 2 additions & 2 deletions ash/app_list/views/scrollable_apps_grid_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ ScrollableAppsGridView::ScrollableAppsGridView(
AppsGridViewFolderDelegate* folder_delegate,
views::ScrollView* parent_scroll_view,
AppListFolderController* folder_controller,
AppsGridViewFocusDelegate* focus_delegate)
AppListKeyboardController* keyboard_controller)
: AppsGridView(a11y_announcer,
view_delegate,
folder_delegate,
folder_controller,
focus_delegate),
keyboard_controller),
scroll_view_(parent_scroll_view) {
DCHECK(scroll_view_);
view_structure_.Init(PagedViewStructure::Mode::kSinglePage);
Expand Down
4 changes: 2 additions & 2 deletions ash/app_list/views/scrollable_apps_grid_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ class ScrollView;

namespace ash {

class AppListKeyboardController;
class AppListViewDelegate;
class AppsGridViewFocusDelegate;

// An apps grid that shows all the apps in a long scrolling list. Used for
// the clamshell mode bubble launcher. Implemented as a single "page" of apps.
Expand All @@ -35,7 +35,7 @@ class ASH_EXPORT ScrollableAppsGridView : public AppsGridView {
AppsGridViewFolderDelegate* folder_delegate,
views::ScrollView* scroll_view,
AppListFolderController* folder_controller,
AppsGridViewFocusDelegate* focus_delegate);
AppListKeyboardController* keyboard_controller);
ScrollableAppsGridView(const ScrollableAppsGridView&) = delete;
ScrollableAppsGridView& operator=(const ScrollableAppsGridView&) = delete;
~ScrollableAppsGridView() override;
Expand Down

0 comments on commit f57945e

Please sign in to comment.