Commit e2642568 authored by Andrew Xu's avatar Andrew Xu Committed by Chromium LUCI CQ

[Multipaste] Polish a11y announcements when deleting menu items

This CL does the following things:
(1) Note that a clipboard history menu item is removed asynchronously.
This CL ensures that only the menu item finally selected is announced.

(2) This CL also changes the a11y announcement when a manu item is
removed.

Bug: 1150503
Change-Id: I0ea16ff1b93cdb3a749833c2e0b08a3616e79108
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2513749Reviewed-by: default avatarDavid Black <dmblack@google.com>
Commit-Queue: Andrew Xu <andrewxu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835350}
parent b653e73d
...@@ -25,6 +25,32 @@ ...@@ -25,6 +25,32 @@
namespace ash { namespace ash {
// ClipboardHistoryMenuModelAdapter::ScopedA11yIgnore --------------------------
// The scoped class to disable a11y for all items views.
class ClipboardHistoryMenuModelAdapter::ScopedA11yIgnore {
public:
explicit ScopedA11yIgnore(
ClipboardHistoryMenuModelAdapter* menu_model_adapter)
: menu_model_adapter_(menu_model_adapter) {
SetIgnoreA11yForAllItemViews(true);
}
~ScopedA11yIgnore() { SetIgnoreA11yForAllItemViews(false); }
private:
void SetIgnoreA11yForAllItemViews(bool ignore) {
for (auto& item_view_command_id_pair :
menu_model_adapter_->item_views_by_command_id_) {
views::View* item_view = item_view_command_id_pair.second;
item_view->GetViewAccessibility().OverrideIsIgnored(ignore);
}
}
ClipboardHistoryMenuModelAdapter* const menu_model_adapter_;
};
// ClipboardHistoryMenuModelAdapter --------------------------------------------
// static // static
std::unique_ptr<ClipboardHistoryMenuModelAdapter> std::unique_ptr<ClipboardHistoryMenuModelAdapter>
ClipboardHistoryMenuModelAdapter::Create( ClipboardHistoryMenuModelAdapter::Create(
...@@ -52,8 +78,8 @@ void ClipboardHistoryMenuModelAdapter::Run( ...@@ -52,8 +78,8 @@ void ClipboardHistoryMenuModelAdapter::Run(
int command_id = ClipboardHistoryUtil::kFirstItemCommandId; int command_id = ClipboardHistoryUtil::kFirstItemCommandId;
const auto& items = clipboard_history_->GetItems(); const auto& items = clipboard_history_->GetItems();
// Do not include the final kDeleteCommandId item in histograms, because it is // Do not include the final kDeleteCommandId item in histograms, because it
// not shown. // is not shown.
UMA_HISTOGRAM_COUNTS_100( UMA_HISTOGRAM_COUNTS_100(
"Ash.ClipboardHistory.ContextMenu.NumberOfItemsShown", items.size()); "Ash.ClipboardHistory.ContextMenu.NumberOfItemsShown", items.size());
...@@ -124,12 +150,19 @@ void ClipboardHistoryMenuModelAdapter::SelectMenuItemWithCommandId( ...@@ -124,12 +150,19 @@ void ClipboardHistoryMenuModelAdapter::SelectMenuItemWithCommandId(
void ClipboardHistoryMenuModelAdapter::RemoveMenuItemWithCommandId( void ClipboardHistoryMenuModelAdapter::RemoveMenuItemWithCommandId(
int command_id) { int command_id) {
// Calculate `new_selected_command_id` before removing // Calculate `new_selected_command_id` before removing the item specified by
// the item specified by `command_id` from data structures because the item to // `command_id` from data structures because the item to be removed is
// be removed is needed in calculation. // needed in calculation.
base::Optional<int> new_selected_command_id = base::Optional<int> new_selected_command_id =
CalculateSelectedCommandIdAfterDeletion(command_id); CalculateSelectedCommandIdAfterDeletion(command_id);
// Disable a11y for all item views. It ensures that when deleting multiple
// item views, only the one finally selected is announced.
if (!item_deletion_in_progress_count_) {
DCHECK(!scoped_ignore_);
scoped_ignore_ = std::make_unique<ScopedA11yIgnore>(this);
}
// Update the menu item selection. // Update the menu item selection.
if (new_selected_command_id.has_value()) { if (new_selected_command_id.has_value()) {
SelectMenuItemWithCommandId(*new_selected_command_id); SelectMenuItemWithCommandId(*new_selected_command_id);
...@@ -138,14 +171,40 @@ void ClipboardHistoryMenuModelAdapter::RemoveMenuItemWithCommandId( ...@@ -138,14 +171,40 @@ void ClipboardHistoryMenuModelAdapter::RemoveMenuItemWithCommandId(
root_view_); root_view_);
} }
auto item_view_to_delete = item_views_by_command_id_.find(command_id); auto item_view_to_delete_iter = item_views_by_command_id_.find(command_id);
DCHECK(item_view_to_delete != item_views_by_command_id_.cend()); DCHECK(item_view_to_delete_iter != item_views_by_command_id_.cend());
views::View* item_view_to_delete = item_view_to_delete_iter->second;
// Disable views to be removed in order to prevent them from handling events. // Configure `item_view_to_delete` to serve a11y features.
views::ViewAccessibility& view_accessibility =
item_view_to_delete->GetViewAccessibility();
// Polish the a11y announcement for deletion operation.
view_accessibility.OverrideDescription(
l10n_util::GetStringUTF16(IDS_CLIPBOARD_HISTORY_ITEM_DELETION));
// Enable a11y announcement for the view to be deleted.
view_accessibility.OverrideIsIgnored(false);
// Disabling `item_view_to_delete` is more like implementation details.
// So do not expose it to users.
view_accessibility.OverrideViewEnablingState(true);
// Specify `item_view_to_delete`'s position in the set. Without calling
// `OverridePosInSet()`, the menu's size after deletion may be announced.
const int pos_in_set = std::distance(item_views_by_command_id_.begin(),
item_view_to_delete_iter) +
1;
view_accessibility.OverridePosInSet(pos_in_set,
item_views_by_command_id_.size());
// Disable views to be removed in order to prevent them from handling
// events.
root_view_->GetMenuItemByID(command_id)->SetEnabled(false); root_view_->GetMenuItemByID(command_id)->SetEnabled(false);
item_view_to_delete->second->SetEnabled(false); item_view_to_delete->SetEnabled(false);
item_views_by_command_id_.erase(item_view_to_delete); item_views_by_command_id_.erase(item_view_to_delete_iter);
auto item_to_delete = item_snapshots_.find(command_id); auto item_to_delete = item_snapshots_.find(command_id);
DCHECK(item_to_delete != item_snapshots_.end()); DCHECK(item_to_delete != item_snapshots_.end());
...@@ -153,6 +212,7 @@ void ClipboardHistoryMenuModelAdapter::RemoveMenuItemWithCommandId( ...@@ -153,6 +212,7 @@ void ClipboardHistoryMenuModelAdapter::RemoveMenuItemWithCommandId(
// The current selected menu item may be accessed after item deletion. So // The current selected menu item may be accessed after item deletion. So
// postpone the menu item deletion. // postpone the menu item deletion.
++item_deletion_in_progress_count_;
base::SequencedTaskRunnerHandle::Get()->PostTask( base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, FROM_HERE,
base::BindOnce(&ClipboardHistoryMenuModelAdapter::RemoveItemView, base::BindOnce(&ClipboardHistoryMenuModelAdapter::RemoveItemView,
...@@ -247,8 +307,8 @@ void ClipboardHistoryMenuModelAdapter::AdvancePseudoFocusFromSelectedItem( ...@@ -247,8 +307,8 @@ void ClipboardHistoryMenuModelAdapter::AdvancePseudoFocusFromSelectedItem(
// Advancing pseudo focus should precede the item selection. Because when an // Advancing pseudo focus should precede the item selection. Because when an
// item view is selected, the selected view does not overwrite its pseudo // item view is selected, the selected view does not overwrite its pseudo
// focus if its pseudo focus is non-empty. It can ensure that the pseudo focus // focus if its pseudo focus is non-empty. It can ensure that the pseudo
// and the corresponding UI appearance update only once. // focus and the corresponding UI appearance update only once.
next_focused_view->AdvancePseudoFocus(reverse); next_focused_view->AdvancePseudoFocus(reverse);
SelectMenuItemWithCommandId(next_selected_item_command); SelectMenuItemWithCommandId(next_selected_item_command);
} }
...@@ -285,6 +345,13 @@ void ClipboardHistoryMenuModelAdapter::RemoveItemView(int command_id) { ...@@ -285,6 +345,13 @@ void ClipboardHistoryMenuModelAdapter::RemoveItemView(int command_id) {
root_view_->RemoveMenuItem(root_view_->GetMenuItemByID(command_id)); root_view_->RemoveMenuItem(root_view_->GetMenuItemByID(command_id));
root_view_->ChildrenChanged(); root_view_->ChildrenChanged();
--item_deletion_in_progress_count_;
// Re-enable a11y for all item views when item deletion finally completes.
if (!item_deletion_in_progress_count_) {
DCHECK(scoped_ignore_);
scoped_ignore_.reset();
}
// `ChildrenChanged()` clears the selection. So restore the selection. // `ChildrenChanged()` clears the selection. So restore the selection.
if (original_selected_command_id.has_value()) if (original_selected_command_id.has_value())
SelectMenuItemWithCommandId(*original_selected_command_id); SelectMenuItemWithCommandId(*original_selected_command_id);
......
...@@ -95,6 +95,8 @@ class ASH_EXPORT ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter { ...@@ -95,6 +95,8 @@ class ASH_EXPORT ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter {
} }
private: private:
class ScopedA11yIgnore;
ClipboardHistoryMenuModelAdapter( ClipboardHistoryMenuModelAdapter(
std::unique_ptr<ui::SimpleMenuModel> model, std::unique_ptr<ui::SimpleMenuModel> model,
base::RepeatingClosure menu_closed_callback, base::RepeatingClosure menu_closed_callback,
...@@ -147,6 +149,12 @@ class ASH_EXPORT ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter { ...@@ -147,6 +149,12 @@ class ASH_EXPORT ClipboardHistoryMenuModelAdapter : views::MenuModelAdapter {
// ClipboardHistoryController. // ClipboardHistoryController.
const ClipboardHistoryResourceManager* const resource_manager_; const ClipboardHistoryResourceManager* const resource_manager_;
// Indicates the number of item deletion operations in progress. Note that
// a `ClipboardHistoryItemView` instance is deleted asynchronously.
int item_deletion_in_progress_count_ = 0;
std::unique_ptr<ScopedA11yIgnore> scoped_ignore_;
// Called when an item view is removed from the root menu. // Called when an item view is removed from the root menu.
base::RepeatingClosure item_removal_callback_for_test_; base::RepeatingClosure item_removal_callback_for_test_;
......
...@@ -1075,6 +1075,9 @@ need to be translated for each locale.--> ...@@ -1075,6 +1075,9 @@ need to be translated for each locale.-->
<message name="IDS_CLIPBOARD_HISTORY_DELETE_BUTTON" desc="Accessibility name of the button which functions to delete a clipboard history menu entry"> <message name="IDS_CLIPBOARD_HISTORY_DELETE_BUTTON" desc="Accessibility name of the button which functions to delete a clipboard history menu entry">
Remove from clipboard. Remove from clipboard.
</message> </message>
<message name="IDS_CLIPBOARD_HISTORY_ITEM_DELETION" desc="Accessibility text when a clipboard history menu entry is deleted">
Removed.
</message>
<!-- Strings describing the touch calibration UX --> <!-- Strings describing the touch calibration UX -->
<message name="IDS_DISPLAY_TOUCH_CALIBRATION_EXIT_LABEL" desc="A message to notify the user about using the escape key to exit the calibration mode."> <message name="IDS_DISPLAY_TOUCH_CALIBRATION_EXIT_LABEL" desc="A message to notify the user about using the escape key to exit the calibration mode.">
......
6511dbca5c3b68f603ab47dcd6b3cc39b1f50604
\ No newline at end of file
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