Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2899663002:
If page is untranslated and the user chooses "Always", translate the page. (Closed)
|
Created:
3 years, 7 months ago by ramyasharma Modified:
3 years, 7 months ago CC:
chromium-reviews, agrieve+watch_chromium.org, dfalcantara+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionIf page is untranslated and the user chooses "Always", translate the page.
When user selects "Always translate page", if the page is untranslated,
then translate the page, and make UI changes to indicate selected
tab on the infobar.
BUG=724670
Review-Url: https://codereview.chromium.org/2899663002
Cr-Commit-Position: refs/heads/master@{#473778}
Committed: https://chromium.googlesource.com/chromium/src/+/f32a232b18f75e5af490a160486468a55ef85641
Patch Set 1 #
Total comments: 2
Patch Set 2 #Patch Set 3 #Messages
Total messages: 26 (20 generated)
Description was changed from ========== If page is untranslated and the user choses "Always", translate the page. BUG=724670 ========== to ========== If page is untranslated and the user chooses "Always", translate the page. When user selects "Always translate page", if the page is untranslated, then auto-translate the page. BUG=724670 ==========
Description was changed from ========== If page is untranslated and the user chooses "Always", translate the page. When user selects "Always translate page", if the page is untranslated, then auto-translate the page. BUG=724670 ========== to ========== If page is untranslated and the user chooses "Always", translate the page. When user selects "Always translate page", if the page is untranslated, then translate the page, and make UI changes to indicate selected tab on the infobar. BUG=724670 ==========
ramyasharma@chromium.org changed reviewers: + googleo@chromium.org, mdjones@chromium.org
The CQ bit was checked by ramyasharma@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...
lgtm Thanks for the fixing. https://codereview.chromium.org/2899663002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java (right): https://codereview.chromium.org/2899663002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:469: startTranslating(mTabLayout.getSelectedTabPosition()); So we are going to start translating after snackbar is dismissed, right? Just one more thing, start translating may pop up another auto translating snackbar if it happens to reach the limit. It may confuse user. Please check the logic popup auto-translate snackbar.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #2 (id:20001) has been deleted
Thanks Leo. https://codereview.chromium.org/2899663002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java (right): https://codereview.chromium.org/2899663002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:469: startTranslating(mTabLayout.getSelectedTabPosition()); On 2017/05/22 06:08:34, Leo wrote: > So we are going to start translating after snackbar is dismissed, right? > > Just one more thing, start translating may pop up another auto translating > snackbar if it happens to reach the limit. It may confuse user. > > Please check the logic popup auto-translate snackbar. Yes, we will start translating only after the snackbar goes away by itself, and not cancelled by the user. regarding double snackbar for auto-translate, there is code in the native to check if !delegate->ShouldAlwaysTranslate(), and only then auto-always is triggered. So in this case, there shouldn't be a duplicate snackbar for auto-translate. But thanks for pointing this out, refactored a bit more so that the code is cleaner.
The CQ bit was checked by ramyasharma@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.
lgtm
The CQ bit was checked by ramyasharma@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 ramyasharma@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from googleo@chromium.org, mdjones@chromium.org Link to the patchset: https://codereview.chromium.org/2899663002/#ps60001 (title: "")
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": 60001, "attempt_start_ts": 1495503423274060,
"parent_rev": "4c535159fa525e13b5e41282be4e99c5b62149e7", "commit_rev":
"f32a232b18f75e5af490a160486468a55ef85641"}
Message was sent while issue was closed.
Description was changed from ========== If page is untranslated and the user chooses "Always", translate the page. When user selects "Always translate page", if the page is untranslated, then translate the page, and make UI changes to indicate selected tab on the infobar. BUG=724670 ========== to ========== If page is untranslated and the user chooses "Always", translate the page. When user selects "Always translate page", if the page is untranslated, then translate the page, and make UI changes to indicate selected tab on the infobar. BUG=724670 Review-Url: https://codereview.chromium.org/2899663002 Cr-Commit-Position: refs/heads/master@{#473778} Committed: https://chromium.googlesource.com/chromium/src/+/f32a232b18f75e5af490a1604864... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001) as https://chromium.googlesource.com/chromium/src/+/f32a232b18f75e5af490a1604864... |
