Skip to content

Commit

Permalink
Update a11y name for recent apps and apps grid. Also fixed toast focus.
Browse files Browse the repository at this point in the history
This cl did two things:
(1) Updated the a11y name of the recent apps and apps grid container.
(2) Fixed the focus issue when moving from or to app list undo toast.

Bug: 1298211
Change-Id: I3fc4817d344f87a711622b5ea45e88d6f09a0a00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3566415
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Commit-Queue: Wen-Chien Wang <wcwang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988559}
  • Loading branch information
Wen-Chien Wang authored and Chromium LUCI CQ committed Apr 4, 2022
1 parent 32b3834 commit f6d80bc
Show file tree
Hide file tree
Showing 12 changed files with 186 additions and 46 deletions.
5 changes: 2 additions & 3 deletions ash/app_list/app_list_test_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -399,12 +399,11 @@ bool AppListTestApi::IsFolderViewAnimating() const {
}

views::View* AppListTestApi::GetBubbleReorderUndoButton() {
return GetToastContainerViewFromBubble()->GetToastDismissButtonForTest();
return GetToastContainerViewFromBubble()->GetToastDismissButton();
}

views::View* AppListTestApi::GetFullscreenReorderUndoButton() {
return GetToastContainerViewFromFullscreenAppList()
->GetToastDismissButtonForTest();
return GetToastContainerViewFromFullscreenAppList()->GetToastDismissButton();
}

bool AppListTestApi::GetBubbleReorderUndoToastVisibility() const {
Expand Down
68 changes: 37 additions & 31 deletions ash/app_list/views/app_list_bubble_apps_page.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@
#include "ash/controls/scroll_view_gradient_helper.h"
#include "ash/public/cpp/metrics_util.h"
#include "ash/public/cpp/style/color_provider.h"
#include "ash/strings/grit/ash_strings.h"
#include "base/bind.h"
#include "base/check.h"
#include "base/metrics/histogram_functions.h"
#include "base/time/time.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/aura/window.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/compositor/animation_throughput_reporter.h"
#include "ui/compositor/layer.h"
Expand Down Expand Up @@ -179,7 +181,7 @@ AppListBubbleAppsPage::AppListBubbleAppsPage(
if (features::IsLauncherAppSortEnabled()) {
toast_container_ = scroll_contents->AddChildView(
std::make_unique<AppListToastContainerView>(
app_list_nudge_controller_.get(), a11y_announcer,
app_list_nudge_controller_.get(), a11y_announcer, /*delegate=*/this,
/*tablet_mode=*/false));
}

Expand Down Expand Up @@ -525,48 +527,37 @@ void AppListBubbleAppsPage::MoveFocusUpFromRecents() {
}

void AppListBubbleAppsPage::MoveFocusDownFromRecents(int column) {
// When showing the sort undo toast, default to the default behavior that
// moves focus to the toast.
if (toast_container_ && toast_container_->is_toast_visible() &&
toast_container_->current_toast() ==
AppListToastContainerView::ToastType::kReorderUndo) {
views::View* last_recent =
recent_apps_->GetItemViewAt(recent_apps_->GetItemViewCount() - 1);
views::View* next_view = GetFocusManager()->GetNextFocusableView(
last_recent, GetWidget(), /*reverse=*/false, /*dont_loop=*/false);
DCHECK(next_view);
next_view->RequestFocus();
// Check if the `toast_container_` can handle the focus.
if (toast_container_ && toast_container_->HandleFocus(column))
return;
}

int top_level_item_count =
scrollable_apps_grid_view_->view_model()->view_size();
if (top_level_item_count <= 0)
return;
// Attempt to focus the item at `column` in the first row, or the last item if
// there aren't enough items. This could happen if the user's apps are in a
// small number of folders.
int index = std::min(column, top_level_item_count - 1);
AppListItemView* item = scrollable_apps_grid_view_->GetItemViewAt(index);
DCHECK(item);
item->RequestFocus();
HandleMovingFocusToAppsGrid(column);
}

void AppListBubbleAppsPage::MoveFocusUpFromToast(int column) {
HandleMovingFocusToRecents(column);
}

void AppListBubbleAppsPage::MoveFocusDownFromToast(int column) {
HandleMovingFocusToAppsGrid(column);
}

bool AppListBubbleAppsPage::MoveFocusUpFromAppsGrid(int column) {
DVLOG(1) << __FUNCTION__;
// When showing the sort undo toast, default to the default behavior that
// moves focus to the toast.
if (toast_container_ && toast_container_->is_toast_visible() &&
toast_container_->current_toast() ==
AppListToastContainerView::ToastType::kReorderUndo) {
return false;
}
// Check if the `toast_container_` can handle the focus.
if (toast_container_ && toast_container_->HandleFocus(column))
return true;

return HandleMovingFocusToRecents(column);
}

bool AppListBubbleAppsPage::HandleMovingFocusToRecents(int column) {
const int recent_app_count = recent_apps_->GetItemViewCount();
// If there aren't any recent apps, don't change focus here. Fall back to the
// app grid's default behavior.
if (!recent_apps_->GetVisible() || recent_app_count <= 0)
return false;

// Attempt to focus the item at `column`, or the last item if there aren't
// enough items.
int index = std::min(column, recent_app_count - 1);
Expand All @@ -576,6 +567,21 @@ bool AppListBubbleAppsPage::MoveFocusUpFromAppsGrid(int column) {
return true;
}

void AppListBubbleAppsPage::HandleMovingFocusToAppsGrid(int column) {
int top_level_item_count =
scrollable_apps_grid_view_->view_model()->view_size();
if (top_level_item_count <= 0)
return;

// Attempt to focus the item at `column` in the first row, or the last item if
// there aren't enough items. This could happen if the user's apps are in a
// small number of folders.
int index = std::min(column, top_level_item_count - 1);
AppListItemView* item = scrollable_apps_grid_view_->GetItemViewAt(index);
DCHECK(item);
item->RequestFocus();
}

ui::Layer* AppListBubbleAppsPage::GetPageAnimationLayerForTest() {
return scroll_view_->contents()->layer();
}
Expand Down
22 changes: 16 additions & 6 deletions ash/app_list/views/app_list_bubble_apps_page.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "ash/app_list/app_list_model_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/app_list/views/recent_apps_view.h"
#include "ash/ash_export.h"
Expand Down Expand Up @@ -37,7 +38,6 @@ class ApplicationDragAndDropHost;
class AppListA11yAnnouncer;
class AppListFolderController;
class AppListNudgeController;
class AppListToastContainerView;
class AppListViewDelegate;
class ContinueSectionView;
class RecentAppsView;
Expand All @@ -50,11 +50,13 @@ class ScrollViewGradientHelper;
// - Continue section with recent tasks and recent apps
// - Grid of all apps
// Does not include the search box, which is owned by a parent view.
class ASH_EXPORT AppListBubbleAppsPage : public views::View,
public views::ViewObserver,
public AppListModelProvider::Observer,
public RecentAppsView::Delegate,
public AppsGridViewFocusDelegate {
class ASH_EXPORT AppListBubbleAppsPage
: public views::View,
public views::ViewObserver,
public AppListModelProvider::Observer,
public RecentAppsView::Delegate,
public AppListToastContainerView::Delegate,
public AppsGridViewFocusDelegate {
public:
METADATA_HEADER(AppListBubbleAppsPage);

Expand Down Expand Up @@ -122,9 +124,17 @@ class ASH_EXPORT AppListBubbleAppsPage : public views::View,
void MoveFocusUpFromRecents() override;
void MoveFocusDownFromRecents(int column) override;

// AppListToastContainerView::Delegate:
void MoveFocusUpFromToast(int column) override;
void MoveFocusDownFromToast(int column) override;

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

// Helper functions to move the focus to RecentAppsView/AppsGridView.
bool HandleMovingFocusToRecents(int column);
void HandleMovingFocusToAppsGrid(int column);

views::ScrollView* scroll_view() { return scroll_view_; }
ScrollableAppsGridView* scrollable_apps_grid_view() {
return scrollable_apps_grid_view_;
Expand Down
53 changes: 53 additions & 0 deletions ash/app_list/views/app_list_bubble_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/rect.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/controls/button/label_button.h"
#include "ui/views/controls/image_view.h"
#include "ui/views/controls/scroll_view.h"
#include "ui/views/controls/textfield/textfield.h"
Expand Down Expand Up @@ -873,6 +874,32 @@ TEST_F(AppListBubbleViewTest, DownArrowFromRecentsSelectsSameColumnInAppsGrid) {
}
}

TEST_F(AppListBubbleViewTest, DownArrowFromToastKeepsSameColumnInAppsGrid) {
AddRecentApps(5);
AddAppItems(5);
ShowAppList();

AppListController::Get()->UpdateAppListWithNewTemporarySortOrder(
AppListSortOrder::kColor,
/*animate=*/false, /*update_position_closure=*/base::OnceClosure());
ASSERT_TRUE(GetToastContainerView()->GetToastDismissButton());

for (int column = 0; column < 5; column++) {
// Pressing down arrow from an item in recent apps selects the app in the
// same column in the apps grid.
AppListItemView* recent_app = GetRecentAppsView()->GetItemViewAt(column);
recent_app->RequestFocus();
ASSERT_TRUE(recent_app->HasFocus());

PressAndReleaseKey(ui::VKEY_DOWN);
EXPECT_TRUE(GetToastContainerView()->GetToastDismissButton()->HasFocus());

PressAndReleaseKey(ui::VKEY_DOWN);
AppListItemView* app = GetAppsGridView()->GetItemViewAt(column);
EXPECT_TRUE(app->HasFocus()) << "Focus mismatch for column " << column;
}
}

TEST_F(AppListBubbleViewTest, DownArrowFromRecentsSelectsLastColumnInAppsGrid) {
AddRecentApps(5);
AddFolderWithApps(2);
Expand Down Expand Up @@ -937,6 +964,32 @@ TEST_F(AppListBubbleViewTest, UpArrowFromAppsGridSelectsSameColumnInRecents) {
}
}

TEST_F(AppListBubbleViewTest, UpArrowFromToastKeepsSameColumnInAppsGrid) {
AddRecentApps(5);
AddAppItems(5);
ShowAppList();

AppListController::Get()->UpdateAppListWithNewTemporarySortOrder(
AppListSortOrder::kColor,
/*animate=*/false, /*update_position_closure=*/base::OnceClosure());
ASSERT_TRUE(GetToastContainerView()->GetToastDismissButton());

for (int column = 0; column < 5; column++) {
// Pressing up arrow from an item in the apps grid selects the app in the
// same column in the recents list.
AppListItemView* app = GetAppsGridView()->GetItemViewAt(column);
app->RequestFocus();
ASSERT_TRUE(app->HasFocus());

PressAndReleaseKey(ui::VKEY_UP);
EXPECT_TRUE(GetToastContainerView()->GetToastDismissButton()->HasFocus());

PressAndReleaseKey(ui::VKEY_UP);
EXPECT_TRUE(GetRecentAppsView()->GetItemViewAt(column)->HasFocus())
<< "Focus mismatch for column " << column;
}
}

TEST_F(AppListBubbleViewTest, UpArrowFromAppsGridSelectsLastColumnInRecents) {
// Add 4 columns of recents, but 5 columns of apps.
AddRecentApps(4);
Expand Down
32 changes: 31 additions & 1 deletion ash/app_list/views/app_list_toast_container_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/strings/grit/ash_strings.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/views/controls/button/label_button.h"
#include "ui/views/layout/flex_layout.h"
#include "ui/views/layout/flex_layout_types.h"
#include "ui/views/view_class_properties.h"
Expand Down Expand Up @@ -44,9 +45,11 @@ constexpr auto kReorderUndoInteriorMargin = gfx::Insets::TLBR(8, 16, 8, 8);
AppListToastContainerView::AppListToastContainerView(
AppListNudgeController* nudge_controller,
AppListA11yAnnouncer* a11y_announcer,
Delegate* delegate,
bool tablet_mode)
: a11y_announcer_(a11y_announcer),
tablet_mode_(tablet_mode),
delegate_(delegate),
nudge_controller_(nudge_controller) {
DCHECK(a11y_announcer_);
SetLayoutManager(std::make_unique<views::FlexLayout>())
Expand All @@ -70,6 +73,33 @@ AppListToastContainerView::~AppListToastContainerView() {
toast_view_ = nullptr;
}

bool AppListToastContainerView::OnKeyPressed(const ui::KeyEvent& event) {
if (!delegate_)
return false;

if (event.key_code() == ui::VKEY_UP) {
delegate_->MoveFocusUpFromToast(focused_app_column_);
return true;
}

if (event.key_code() == ui::VKEY_DOWN) {
delegate_->MoveFocusDownFromToast(focused_app_column_);
return true;
}
return false;
}

bool AppListToastContainerView::HandleFocus(int column) {
// Only handle the focus if the button on the toast exists.
views::LabelButton* dismiss_button = GetToastDismissButton();
if (!dismiss_button)
return false;

focused_app_column_ = column;
dismiss_button->RequestFocus();
return true;
}

void AppListToastContainerView::MaybeUpdateReorderNudgeView() {
// If the expect state in `nudge_controller_` is different from the one in
// toast container, update the actual reorder nudge view in the toast
Expand Down Expand Up @@ -207,7 +237,7 @@ void AppListToastContainerView::AnnounceSortOrder(AppListSortOrder new_order) {
a11y_announcer_->Announce(CalculateToastTextFromOrder(new_order));
}

views::LabelButton* AppListToastContainerView::GetToastDismissButtonForTest() {
views::LabelButton* AppListToastContainerView::GetToastDismissButton() {
if (!toast_view_ || current_toast_ != ToastType::kReorderUndo)
return nullptr;

Expand Down
32 changes: 31 additions & 1 deletion ash/app_list/views/app_list_toast_container_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,39 @@ class AppListToastContainerView : public views::View {
kReorderUndo,
};

class Delegate {
public:
virtual ~Delegate() = default;

// Requests that focus move up and out (usually to the recent apps).
// `column` is the column of the item (could be from the recent apps or apps
// grid) that was focused before moving focus on this toast container. The
// delegate should choose an appropriate item to focus.
virtual void MoveFocusUpFromToast(int column) = 0;

// Requests that focus move down and out (usually to the apps grid).
// `column` is the column of the item (could be from the recent apps or apps
// grid) that was focused before moving focus on this toast container. The
// delegate should choose an appropriate item to focus.
virtual void MoveFocusDownFromToast(int column) = 0;
};

AppListToastContainerView(AppListNudgeController* nudge_controller_,
AppListA11yAnnouncer* a11y_announcer,
Delegate* delegate,
bool tablet_mode);
AppListToastContainerView(const AppListToastContainerView&) = delete;
AppListToastContainerView& operator=(const AppListToastContainerView&) =
delete;
~AppListToastContainerView() override;

// views::View:
bool OnKeyPressed(const ui::KeyEvent& event) override;

// Handle focus passed from the app on column `column` in AppsGridView or
// RecentAppsView.
bool HandleFocus(int column);

// Updates the toast container to show/hide the reorder nudge if needed.
void MaybeUpdateReorderNudgeView();

Expand Down Expand Up @@ -86,7 +111,7 @@ class AppListToastContainerView : public views::View {
void AnnounceSortOrder(AppListSortOrder new_order);

// This function expects that `toast_view_` exists.
views::LabelButton* GetToastDismissButtonForTest();
views::LabelButton* GetToastDismissButton();

bool is_toast_visible() const { return toast_view_; }
ToastType current_toast() const { return current_toast_; }
Expand All @@ -112,6 +137,7 @@ class AppListToastContainerView : public views::View {

AppListToastView* toast_view_ = nullptr;

Delegate* const delegate_;
AppListNudgeController* const nudge_controller_;

// Caches the current visibility state which is used to help tracking the
Expand All @@ -120,6 +146,10 @@ class AppListToastContainerView : public views::View {

// Caches the current toast type.
ToastType current_toast_ = ToastType::kNone;

// Caches the column of previously focused app. Used when passing focus
// between apps grid view and recent apps.
int focused_app_column_ = 0;
};

} // namespace ash
Expand Down
2 changes: 1 addition & 1 deletion ash/app_list/views/apps_container_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ AppsContainerView::AppsContainerView(ContentsView* contents_view)
toast_container_ = scrollable_container_->AddChildView(
std::make_unique<AppListToastContainerView>(
app_list_nudge_controller_.get(), a11y_announcer,
/*tablet_mode=*/true));
/*delegate=*/nullptr, /*tablet_mode=*/true));
toast_container_->SetPaintToLayer(ui::LAYER_NOT_DRAWN);
}
} else {
Expand Down

0 comments on commit f6d80bc

Please sign in to comment.