Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2796303007:
[Mac] Added Accessibility Labels to the Default Touch Bar (Closed)
Description[Mac] Added Accessibility Labels to the Default Touch Bar
BUG=696557
Review-Url: https://codereview.chromium.org/2796303007
Cr-Commit-Position: refs/heads/master@{#463011}
Committed: https://chromium.googlesource.com/chromium/src/+/b3e132a8f0a9aa4fe17aaa90246d09efd05cfcc3
Patch Set 1 #Patch Set 2 : Nit #
Total comments: 6
Patch Set 3 : Fixes for ellyjones #
Messages
Total messages: 27 (16 generated)
The CQ bit was checked by spqchan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
spqchan@chromium.org changed reviewers: + ellyjones@chromium.org
PTAL
The CQ bit was checked by spqchan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2796303007/diff/20001/chrome/app/generated_re... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2796303007/diff/20001/chrome/app/generated_re... chrome/app/generated_resources.grd:1935: desc="Text for the search button in the touch bar when Google is not the default search provider. Pressing the button will bring focus to the omnibox. $1 is the name of the search provider (Ex/ Bing)"> this is usually done using <ex></ex> tags in the message body instead. https://codereview.chromium.org/2796303007/diff/20001/chrome/app/generated_re... chrome/app/generated_resources.grd:1939: <message name="IDS_TOOLTIP_TOUCH_BAR_BACK" desc="The tooltip for the touch bar back button"> nit: these should have trailing dots https://codereview.chromium.org/2796303007/diff/20001/chrome/app/generated_re... chrome/app/generated_resources.grd:1940: Click to go back. Touch to go back?
oh and also lgtm :)
On 2017/04/07 13:35:46, Elly Fong-Jones wrote: > oh and also lgtm :) Should all of the labels have a trailing dot? If that's the case, I should create new text for each button.
On 2017/04/07 17:07:23, spqchan wrote: > On 2017/04/07 13:35:46, Elly Fong-Jones wrote: > > oh and also lgtm :) > > Should all of the labels have a trailing dot? If that's the case, I should > create new text for each button. Oh, sorry, I meant the desc="" values should have trailing dots :)
On 2017/04/07 17:19:29, Elly Fong-Jones wrote: > On 2017/04/07 17:07:23, spqchan wrote: > > On 2017/04/07 13:35:46, Elly Fong-Jones wrote: > > > oh and also lgtm :) > > > > Should all of the labels have a trailing dot? If that's the case, I should > > create new text for each button. > > Oh, sorry, I meant the desc="" values should have trailing dots :) Ohhhhhh gotcha, thanks!
spqchan@chromium.org changed reviewers: + avi@chromium.org
+avi for OWNERS https://codereview.chromium.org/2796303007/diff/20001/chrome/app/generated_re... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/2796303007/diff/20001/chrome/app/generated_re... chrome/app/generated_resources.grd:1935: desc="Text for the search button in the touch bar when Google is not the default search provider. Pressing the button will bring focus to the omnibox. $1 is the name of the search provider (Ex/ Bing)"> On 2017/04/07 13:35:30, Elly Fong-Jones wrote: > this is usually done using <ex></ex> tags in the message body instead. Done. https://codereview.chromium.org/2796303007/diff/20001/chrome/app/generated_re... chrome/app/generated_resources.grd:1939: <message name="IDS_TOOLTIP_TOUCH_BAR_BACK" desc="The tooltip for the touch bar back button"> On 2017/04/07 13:35:30, Elly Fong-Jones wrote: > nit: these should have trailing dots Done. https://codereview.chromium.org/2796303007/diff/20001/chrome/app/generated_re... chrome/app/generated_resources.grd:1940: Click to go back. On 2017/04/07 13:35:30, Elly Fong-Jones wrote: > Touch to go back? Done.
lgtm
On 2017/04/07 19:10:33, Avi (OOO 10-12 April) wrote: > lgtm thanks!
The CQ bit was checked by spqchan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by spqchan@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ellyjones@chromium.org Link to the patchset: https://codereview.chromium.org/2796303007/#ps40001 (title: "Fixes for ellyjones")
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": 1491597857940490,
"parent_rev": "083658601292da9c95167513264e69a776ae97af", "commit_rev":
"b3e132a8f0a9aa4fe17aaa90246d09efd05cfcc3"}
Message was sent while issue was closed.
Description was changed from ========== [Mac] Added Accessibility Labels to the Default Touch Bar BUG=696557 ========== to ========== [Mac] Added Accessibility Labels to the Default Touch Bar BUG=696557 Review-Url: https://codereview.chromium.org/2796303007 Cr-Commit-Position: refs/heads/master@{#463011} Committed: https://chromium.googlesource.com/chromium/src/+/b3e132a8f0a9aa4fe17aaa90246d... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/b3e132a8f0a9aa4fe17aaa90246d... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
