Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2885293002:
Add strings needed for the VR feedback infobar (Closed)
DescriptionAdd strings needed for the VR feedback infobar
Implementation will follow.
BUG=706438
Review-Url: https://codereview.chromium.org/2885293002
Cr-Commit-Position: refs/heads/master@{#472574}
Committed: https://chromium.googlesource.com/chromium/src/+/eaf14a90c04606bf4aa4b3cea2fb3f6ddcf6afd8
Patch Set 1 #
Total comments: 2
Patch Set 2 : nit #Messages
Total messages: 15 (6 generated)
ymalik@chromium.org changed reviewers: + mthiesse@chromium.org, tedchoc@chromium.org
PTAL :) Adding strings early for timely translations. Implementation to follow.
lgtm https://codereview.chromium.org/2885293002/diff/1/chrome/android/java/strings... File chrome/android/java/strings/android_chrome_strings.grd (right): https://codereview.chromium.org/2885293002/diff/1/chrome/android/java/strings... chrome/android/java/strings/android_chrome_strings.grd:2958: <message name="IDS_VR_SHELL_FEEDBACK_INFOBAR_FEEDBACK_BUTTON" desc="The 'Provide feedback' button in the VR feedback infobar."> Nit: Not a very descriptive description :P Perhaps "Brief button text asking the user to provide feedback in the VR feedback infobar."
https://codereview.chromium.org/2885293002/diff/1/chrome/android/java/strings... File chrome/android/java/strings/android_chrome_strings.grd (right): https://codereview.chromium.org/2885293002/diff/1/chrome/android/java/strings... chrome/android/java/strings/android_chrome_strings.grd:2958: <message name="IDS_VR_SHELL_FEEDBACK_INFOBAR_FEEDBACK_BUTTON" desc="The 'Provide feedback' button in the VR feedback infobar."> On 2017/05/17 03:36:39, mthiesse wrote: > Nit: Not a very descriptive description :P > > Perhaps "Brief button text asking the user to provide feedback in the VR > feedback infobar." Done.
tedchoc@chromium.org changed reviewers: + srahim@chromium.org
+ srahim for string sign off lgtm though -- What is the target for the infobar you're proposing BTW? What channels would this be targeted towards? Have you got this approved by ui-review (even via email)? It would be good to make that clear in the bug as that would certainly block the infobar from landing if it affects anything but the canary/dev channels
On 2017/05/17 14:23:49, Ted C wrote: > + srahim for string sign off > > lgtm though -- What is the target for the infobar you're proposing BTW? What > channels would this be targeted towards? Have you got this approved by > ui-review (even via email)? It would be good to make that clear in the bug as > that would certainly block the infobar from landing if it affects anything but > the canary/dev channels The target would be M60. I believe joshcarpenter@ ran the design by ui-review. I'll ask him to update the bug to confirm.
On 2017/05/17 14:35:27, ymalik wrote: > On 2017/05/17 14:23:49, Ted C wrote: > > + srahim for string sign off > > > > lgtm though -- What is the target for the infobar you're proposing BTW? What > > channels would this be targeted towards? Have you got this approved by > > ui-review (even via email)? It would be good to make that clear in the bug as > > that would certainly block the infobar from landing if it affects anything but > > the canary/dev channels > > The target would be M60. I believe joshcarpenter@ ran the design by ui-review. > I'll ask him to update the bug to confirm. I've done initial validation with Shimi and Chris Lee from Chrome UX team. We're going through ui-review with the implemented build later this week or early next, depending on engineering progress.
LGTM, thanks!
The CQ bit was checked by ymalik@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mthiesse@chromium.org Link to the patchset: https://codereview.chromium.org/2885293002/#ps20001 (title: "nit")
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": 20001, "attempt_start_ts": 1495052759164420,
"parent_rev": "721a3c1132e02f2f45c4eab292ef4418ff384bf3", "commit_rev":
"eaf14a90c04606bf4aa4b3cea2fb3f6ddcf6afd8"}
Message was sent while issue was closed.
Description was changed from ========== Add strings needed for the VR feedback infobar Implementation will follow. BUG=706438 ========== to ========== Add strings needed for the VR feedback infobar Implementation will follow. BUG=706438 Review-Url: https://codereview.chromium.org/2885293002 Cr-Commit-Position: refs/heads/master@{#472574} Committed: https://chromium.googlesource.com/chromium/src/+/eaf14a90c04606bf4aa4b3cea2fb... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/eaf14a90c04606bf4aa4b3cea2fb... |
