Commit 3ed5a723 authored by Gauthier Ambard's avatar Gauthier Ambard Committed by Commit Bot

[iOS] Update tools menu VoiceOver

This CL removes the default VoiceOver action which is reading the title
of the item then the content of the badge.
Now the accessibility label is the title and a new property,
|additionalAccessibilityLabel|, if it is set.

This is a reland of crrev.com/c/1789397, fixing the failing tests.

Bug: 985764
Change-Id: I6abef0554fd30463d3381de7baf000752cafeb39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1833475Reviewed-by: default avatarJustin Cohen <justincohen@chromium.org>
Reviewed-by: default avataredchin <edchin@chromium.org>
Auto-Submit: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702782}
parent 23240ec0
...@@ -591,10 +591,10 @@ locale. The strings in this file are specific to iOS. ...@@ -591,10 +591,10 @@ locale. The strings in this file are specific to iOS.
Reading List Reading List
</message> </message>
<message name="IDS_IOS_CONTENT_SUGGESTIONS_READING_LIST_ACCESSIBILITY_LABEL_ONE_UNREAD" desc="The accessibility label to use for the reading list content suggestion cell when there is only a single unread article [Length: unlimited]"> <message name="IDS_IOS_CONTENT_SUGGESTIONS_READING_LIST_ACCESSIBILITY_LABEL_ONE_UNREAD" desc="The accessibility label to use for the reading list content suggestion cell when there is only a single unread article [Length: unlimited]">
1 unread Reading List article. 1 unread article.
</message> </message>
<message name="IDS_IOS_CONTENT_SUGGESTIONS_READING_LIST_ACCESSIBILITY_LABEL" desc="The accessibility label to use for the reading list content suggestion cell when there are multiple unread articles [Length: unlimited]"> <message name="IDS_IOS_CONTENT_SUGGESTIONS_READING_LIST_ACCESSIBILITY_LABEL" desc="The accessibility label to use for the reading list content suggestion cell when there are multiple unread articles [Length: unlimited]">
<ph name="UNREAD_COUNT">$1<ex>4</ex></ph> unread Reading List articles. <ph name="UNREAD_COUNT">$1<ex>4</ex></ph> unread articles.
</message> </message>
<message name="IDS_IOS_CONTENT_SUGGESTIONS_RECENT_TABS" desc="The Recent Tabs title on the new tab page [Length: 10em]"> <message name="IDS_IOS_CONTENT_SUGGESTIONS_RECENT_TABS" desc="The Recent Tabs title on the new tab page [Length: 10em]">
Recent Tabs Recent Tabs
......
...@@ -388,7 +388,12 @@ std::unique_ptr<net::test_server::HttpResponse> LoadFrenchPage( ...@@ -388,7 +388,12 @@ std::unique_ptr<net::test_server::HttpResponse> LoadFrenchPage(
assertWithMatcher:grey_notNil()]; assertWithMatcher:grey_notNil()];
// Close tools menu by tapping reload. // Close tools menu by tapping reload.
[[[EarlGrey selectElementWithMatcher:chrome_test_util::ReloadButton()] [[[EarlGrey
selectElementWithMatcher:grey_allOf(
chrome_test_util::ReloadButton(),
grey_ancestor(
chrome_test_util::ToolsMenuView()),
nil)]
usingSearchAction:grey_scrollInDirection(kGREYDirectionUp, 150) usingSearchAction:grey_scrollInDirection(kGREYDirectionUp, 150)
onElementWithMatcher:chrome_test_util::ToolsMenuView()] onElementWithMatcher:chrome_test_util::ToolsMenuView()]
performAction:grey_tap()]; performAction:grey_tap()];
......
...@@ -79,7 +79,9 @@ ...@@ -79,7 +79,9 @@
} }
self.accessibilityLabel = self.accessibilityLabel =
AccessibilityLabelForReadingListCellWithCount(self.count); [NSString stringWithFormat:@"%@, %@", self.title,
AccessibilityLabelForReadingListCellWithCount(
self.count)];
DCHECK(self.accessibilityLabel.length); DCHECK(self.accessibilityLabel.length);
} }
......
...@@ -195,8 +195,10 @@ const CGFloat kTopInset = 10; ...@@ -195,8 +195,10 @@ const CGFloat kTopInset = 10;
if (self.readingListBadgeValue > 0) { if (self.readingListBadgeValue > 0) {
cell.tile.countLabel.text = [@(self.readingListBadgeValue) stringValue]; cell.tile.countLabel.text = [@(self.readingListBadgeValue) stringValue];
cell.tile.countContainer.hidden = NO; cell.tile.countContainer.hidden = NO;
cell.accessibilityLabel = AccessibilityLabelForReadingListCellWithCount( cell.accessibilityLabel = [NSString
self.readingListBadgeValue); stringWithFormat:@"%@, %@", cell.accessibilityLabel,
AccessibilityLabelForReadingListCellWithCount(
self.readingListBadgeValue)];
} }
} }
} }
......
...@@ -62,6 +62,7 @@ source_set("popup_menu") { ...@@ -62,6 +62,7 @@ source_set("popup_menu") {
"//ios/chrome/browser/ui/commands", "//ios/chrome/browser/ui/commands",
"//ios/chrome/browser/ui/coordinators:chrome_coordinators", "//ios/chrome/browser/ui/coordinators:chrome_coordinators",
"//ios/chrome/browser/ui/list_model", "//ios/chrome/browser/ui/list_model",
"//ios/chrome/browser/ui/ntp_tile_views:constants",
"//ios/chrome/browser/ui/popup_menu/cells", "//ios/chrome/browser/ui/popup_menu/cells",
"//ios/chrome/browser/ui/popup_menu/public", "//ios/chrome/browser/ui/popup_menu/public",
"//ios/chrome/browser/ui/popup_menu/public:popup_menu_ui", "//ios/chrome/browser/ui/popup_menu/public:popup_menu_ui",
......
...@@ -17,14 +17,18 @@ ...@@ -17,14 +17,18 @@
@property(nonatomic, strong) UIImage* image; @property(nonatomic, strong) UIImage* image;
// Whether the cell associated with this item should be enabled. // Whether the cell associated with this item should be enabled.
@property(nonatomic, assign) BOOL enabled; @property(nonatomic, assign) BOOL enabled;
// Number to be displayed in the badge. If 0, the badge is hidden. // Number to be displayed in the badge. If 0, the badge is hidden. This is not
// read by VoiceOver.
@property(nonatomic, assign) NSInteger badgeNumber; @property(nonatomic, assign) NSInteger badgeNumber;
// Text to be displayed in the badge. Set to nil to hide the badge. The text // Text to be displayed in the badge. Set to nil to hide the badge. The text
// badge is only displayed if the numbered badge is hidden. // badge is only displayed if the numbered badge is hidden. This is not read by
// VoiceOver.
@property(nonatomic, copy) NSString* badgeText; @property(nonatomic, copy) NSString* badgeText;
// Whether the item is associated with a destructive action. If |YES|, then a // Whether the item is associated with a destructive action. If |YES|, then a
// specific styling is applied. // specific styling is applied.
@property(nonatomic, assign) BOOL destructiveAction; @property(nonatomic, assign) BOOL destructiveAction;
// Additional label. Read after |title| if not nil.
@property(nonatomic, strong) NSString* additionalAccessibilityLabel;
@end @end
...@@ -41,6 +45,9 @@ ...@@ -41,6 +45,9 @@
// specific styling is applied. // specific styling is applied.
@property(nonatomic, assign) BOOL destructiveAction; @property(nonatomic, assign) BOOL destructiveAction;
// Additional label. Read after |title| if not nil.
@property(nonatomic, strong) NSString* additionalAccessibilityLabel;
// Sets the number on the badge number. // Sets the number on the badge number.
- (void)setBadgeNumber:(NSInteger)badgeNumber; - (void)setBadgeNumber:(NSInteger)badgeNumber;
// Sets the text of the badge text. Hides the badge text if |badgeText| is nil. // Sets the text of the badge text. Hides the badge text if |badgeText| is nil.
......
...@@ -34,12 +34,6 @@ NSString* const kToolsMenuTextBadgeAccessibilityIdentifier = ...@@ -34,12 +34,6 @@ NSString* const kToolsMenuTextBadgeAccessibilityIdentifier =
@implementation PopupMenuToolsItem @implementation PopupMenuToolsItem
@synthesize actionIdentifier = _actionIdentifier; @synthesize actionIdentifier = _actionIdentifier;
@synthesize badgeNumber = _badgeNumber;
@synthesize badgeText = _badgeText;
@synthesize image = _image;
@synthesize title = _title;
@synthesize enabled = _enabled;
@synthesize destructiveAction = _destructiveAction;
- (instancetype)initWithType:(NSInteger)type { - (instancetype)initWithType:(NSInteger)type {
self = [super initWithType:type]; self = [super initWithType:type];
...@@ -60,6 +54,7 @@ NSString* const kToolsMenuTextBadgeAccessibilityIdentifier = ...@@ -60,6 +54,7 @@ NSString* const kToolsMenuTextBadgeAccessibilityIdentifier =
cell.destructiveAction = self.destructiveAction; cell.destructiveAction = self.destructiveAction;
[cell setBadgeNumber:self.badgeNumber]; [cell setBadgeNumber:self.badgeNumber];
[cell setBadgeText:self.badgeText]; [cell setBadgeText:self.badgeText];
cell.additionalAccessibilityLabel = self.additionalAccessibilityLabel;
} }
#pragma mark - PopupMenuItem #pragma mark - PopupMenuItem
...@@ -104,11 +99,6 @@ NSString* const kToolsMenuTextBadgeAccessibilityIdentifier = ...@@ -104,11 +99,6 @@ NSString* const kToolsMenuTextBadgeAccessibilityIdentifier =
@implementation PopupMenuToolsCell @implementation PopupMenuToolsCell
@synthesize imageView = _imageView; @synthesize imageView = _imageView;
@synthesize numberBadgeView = _numberBadgeView;
@synthesize textBadgeView = _textBadgeView;
@synthesize titleLabel = _titleLabel;
@synthesize titleToBadgeConstraint = _titleToBadgeConstraint;
@synthesize destructiveAction = _destructiveAction;
- (instancetype)initWithStyle:(UITableViewCellStyle)style - (instancetype)initWithStyle:(UITableViewCellStyle)style
reuseIdentifier:(NSString*)reuseIdentifier { reuseIdentifier:(NSString*)reuseIdentifier {
...@@ -316,6 +306,17 @@ NSString* const kToolsMenuTextBadgeAccessibilityIdentifier = ...@@ -316,6 +306,17 @@ NSString* const kToolsMenuTextBadgeAccessibilityIdentifier =
} }
} }
#pragma mark - Accessibility
- (NSString*)accessibilityLabel {
if (self.additionalAccessibilityLabel) {
return [NSString stringWithFormat:@"%@, %@", self.titleLabel.text,
self.additionalAccessibilityLabel];
} else {
return self.titleLabel.text;
}
}
#pragma mark - Private #pragma mark - Private
// Callback when the preferred Content Size change. // Callback when the preferred Content Size change.
......
...@@ -29,6 +29,7 @@ ...@@ -29,6 +29,7 @@
#import "ios/chrome/browser/ui/commands/browser_commands.h" #import "ios/chrome/browser/ui/commands/browser_commands.h"
#import "ios/chrome/browser/ui/commands/reading_list_add_command.h" #import "ios/chrome/browser/ui/commands/reading_list_add_command.h"
#import "ios/chrome/browser/ui/list_model/list_model.h" #import "ios/chrome/browser/ui/list_model/list_model.h"
#import "ios/chrome/browser/ui/ntp_tile_views/ntp_tile_constants.h"
#import "ios/chrome/browser/ui/popup_menu/cells/popup_menu_navigation_item.h" #import "ios/chrome/browser/ui/popup_menu/cells/popup_menu_navigation_item.h"
#import "ios/chrome/browser/ui/popup_menu/cells/popup_menu_tools_item.h" #import "ios/chrome/browser/ui/popup_menu/cells/popup_menu_tools_item.h"
#import "ios/chrome/browser/ui/popup_menu/popup_menu_constants.h" #import "ios/chrome/browser/ui/popup_menu/popup_menu_constants.h"
...@@ -899,8 +900,13 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID, ...@@ -899,8 +900,13 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
self.readingListItem = CreateTableViewItem( self.readingListItem = CreateTableViewItem(
IDS_IOS_TOOLS_MENU_READING_LIST, PopupMenuActionReadingList, IDS_IOS_TOOLS_MENU_READING_LIST, PopupMenuActionReadingList,
@"popup_menu_reading_list", kToolsMenuReadingListId); @"popup_menu_reading_list", kToolsMenuReadingListId);
self.readingListItem.badgeNumber = NSInteger numberOfUnreadArticles =
[self.readingListMenuNotifier readingListUnreadCount]; [self.readingListMenuNotifier readingListUnreadCount];
self.readingListItem.badgeNumber = numberOfUnreadArticles;
if (numberOfUnreadArticles) {
self.readingListItem.additionalAccessibilityLabel =
AccessibilityLabelForReadingListCellWithCount(numberOfUnreadArticles);
}
if (self.engagementTracker && if (self.engagementTracker &&
self.engagementTracker->ShouldTriggerHelpUI( self.engagementTracker->ShouldTriggerHelpUI(
feature_engagement::kIPHBadgedReadingListFeature)) { feature_engagement::kIPHBadgedReadingListFeature)) {
......
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