Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2607763002:
Adds button accessibility traits to the nativeAppItem cell. (Closed)
DescriptionAdds button accessibility traits to the nativeAppItem cell.
Cells will now be a single accessibility item: https://drive.google.com/open?id=0B51hlF9bHYPwNE9CZTg1dDVyYkk
VoiceOver will read the name of the App and then the switch state or "Double tap to install" string.
BUG=590064, 341403
Review-Url: https://codereview.chromium.org/2607763002
Cr-Commit-Position: refs/heads/master@{#442092}
Committed: https://chromium.googlesource.com/chromium/src/+/c70beeafc5b50c1f7c00ab6aeb9577268b251a3d
Patch Set 1 #Patch Set 2 : Remove accesibility hint #
Total comments: 2
Patch Set 3 : Add Accesibility hint and install text value. #
Total comments: 2
Messages
Total messages: 15 (7 generated)
Description was changed from ========== Adds button accessibility traits to the nativeAppItem cell. BUG=590064 ========== to ========== Adds button accessibility traits to the nativeAppItem cell. Cells will now be a single accessibility item: https://drive.google.com/open?id=0B51hlF9bHYPwNE9CZTg1dDVyYkk Voicer over will read the name of the App and then the switch state or "Double tap to install" string. BUG=590064, 341403 ==========
sczs@chromium.org changed reviewers: + lpromero@chromium.org
sczs@chromium.org changed reviewers: + sczs@chromium.org
PTAL
Description was changed from ========== Adds button accessibility traits to the nativeAppItem cell. Cells will now be a single accessibility item: https://drive.google.com/open?id=0B51hlF9bHYPwNE9CZTg1dDVyYkk Voicer over will read the name of the App and then the switch state or "Double tap to install" string. BUG=590064, 341403 ========== to ========== Adds button accessibility traits to the nativeAppItem cell. Cells will now be a single accessibility item: https://drive.google.com/open?id=0B51hlF9bHYPwNE9CZTg1dDVyYkk VoiceOver will read the name of the App and then the switch state or "Double tap to install" string. BUG=590064, 341403 ==========
https://codereview.chromium.org/2607763002/diff/20001/ios/chrome/browser/ui/s... File ios/chrome/browser/ui/settings/cells/native_app_item.mm (right): https://codereview.chromium.org/2607763002/diff/20001/ios/chrome/browser/ui/s... ios/chrome/browser/ui/settings/cells/native_app_item.mm:190: IDS_IOS_GOOGLE_APPS_INSTALL_BUTTON_ACCESSIBILITY_HINT)]; The hint reads: "Double tap to install", so I really think it should be in the accessibilityHint instead. What you want here is probably l10n_util::GetNSString(IDS_IOS_GOOGLE_APPS_INSTALL_BUTTON).
https://codereview.chromium.org/2607763002/diff/20001/ios/chrome/browser/ui/s... File ios/chrome/browser/ui/settings/cells/native_app_item.mm (right): https://codereview.chromium.org/2607763002/diff/20001/ios/chrome/browser/ui/s... ios/chrome/browser/ui/settings/cells/native_app_item.mm:190: IDS_IOS_GOOGLE_APPS_INSTALL_BUTTON_ACCESSIBILITY_HINT)]; On 2017/01/05 23:00:58, lpromero wrote: > The hint reads: "Double tap to install", so I really think it should be in the > accessibilityHint instead. > What you want here is probably > l10n_util::GetNSString(IDS_IOS_GOOGLE_APPS_INSTALL_BUTTON). You're right, the "INSTALL" text wasn't being read. Re-added the hint and added the install text value.
https://codereview.chromium.org/2607763002/diff/40001/ios/chrome/browser/ui/s... File ios/chrome/browser/ui/settings/cells/native_app_item.mm (right): https://codereview.chromium.org/2607763002/diff/40001/ios/chrome/browser/ui/s... ios/chrome/browser/ui/settings/cells/native_app_item.mm:105: _installButton.accessibilityHint = l10n_util::GetNSString( If the parent view is accessibility element, the button is not visible nor read by VoiceOver. You need to add this hint to the cell instead (line 171).
https://codereview.chromium.org/2607763002/diff/40001/ios/chrome/browser/ui/s... File ios/chrome/browser/ui/settings/cells/native_app_item.mm (right): https://codereview.chromium.org/2607763002/diff/40001/ios/chrome/browser/ui/s... ios/chrome/browser/ui/settings/cells/native_app_item.mm:105: _installButton.accessibilityHint = l10n_util::GetNSString( On 2017/01/06 09:41:27, lpromero wrote: > If the parent view is accessibility element, the button is not visible nor read > by VoiceOver. You need to add this hint to the cell instead (line 171). This currently gets read by VoiceOver since [self.accessoryView accessibilityHint] returns the installButton accessibilityHint. Currently voice over will read: "App Name Install..... Double tap to Install"
On 2017/01/06 21:59:09, sczs wrote: > https://codereview.chromium.org/2607763002/diff/40001/ios/chrome/browser/ui/s... > File ios/chrome/browser/ui/settings/cells/native_app_item.mm (right): > > https://codereview.chromium.org/2607763002/diff/40001/ios/chrome/browser/ui/s... > ios/chrome/browser/ui/settings/cells/native_app_item.mm:105: > _installButton.accessibilityHint = l10n_util::GetNSString( > On 2017/01/06 09:41:27, lpromero wrote: > > If the parent view is accessibility element, the button is not visible nor > read > > by VoiceOver. You need to add this hint to the cell instead (line 171). > > This currently gets read by VoiceOver since [self.accessoryView > accessibilityHint] returns the installButton accessibilityHint. > > Currently voice over will read: "App Name Install..... Double tap to Install" Lgtm
The CQ bit was checked by sczs@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483743976261280,
"parent_rev": "c5ffc3925eacc602f07e724560a4b0830edf108c", "commit_rev":
"c70beeafc5b50c1f7c00ab6aeb9577268b251a3d"}
Message was sent while issue was closed.
Description was changed from ========== Adds button accessibility traits to the nativeAppItem cell. Cells will now be a single accessibility item: https://drive.google.com/open?id=0B51hlF9bHYPwNE9CZTg1dDVyYkk VoiceOver will read the name of the App and then the switch state or "Double tap to install" string. BUG=590064, 341403 ========== to ========== Adds button accessibility traits to the nativeAppItem cell. Cells will now be a single accessibility item: https://drive.google.com/open?id=0B51hlF9bHYPwNE9CZTg1dDVyYkk VoiceOver will read the name of the App and then the switch state or "Double tap to install" string. BUG=590064, 341403 Review-Url: https://codereview.chromium.org/2607763002 Cr-Commit-Position: refs/heads/master@{#442092} Committed: https://chromium.googlesource.com/chromium/src/+/c70beeafc5b50c1f7c00ab6aeb95... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/c70beeafc5b50c1f7c00ab6aeb95... |
