Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(251)

Issue 2489143002: Use new download notification strings (Closed)

Created:
4 years, 1 month ago by qinmin
Modified:
4 years ago
Reviewers:
dougarnett, gone, fgorski
CC:
chromium-reviews, asanka, dewittj+watch_chromium.org, fgorski+watch_chromium.org, romax+watch_chromium.org, petewil+watch_chromium.org, chili+watch_chromium.org, agrieve+watch_chromium.org, dimich+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Use new download notification strings This CL adds new download notification strings: [scheduled] - Download pending... [In progress] - Downloading... [Interrupted connection] - Download waiting... [complete] - Download complete BUG=658744 Committed: https://crrev.com/9f6bbeb42b766bc9b4385e409a6adb88a85e6c8d Cr-Commit-Position: refs/heads/master@{#431462}

Patch Set 1 #

Total comments: 2

Patch Set 2 : nit #

Patch Set 3 : merge waiting and pending state #

Total comments: 1

Messages

Total messages: 16 (5 generated)
qinmin
PTAL
4 years, 1 month ago (2016-11-09 22:59:16 UTC) #2
dewittj
drive-by comment https://codereview.chromium.org/2489143002/diff/1/chrome/android/java/strings/android_chrome_strings.grd File chrome/android/java/strings/android_chrome_strings.grd (right): https://codereview.chromium.org/2489143002/diff/1/chrome/android/java/strings/android_chrome_strings.grd#newcode1627 chrome/android/java/strings/android_chrome_strings.grd:1627: <message name="IDS_DOWNLOAD_NOTIFICATION_PENDING" desc="Download notification to be displayed ...
4 years, 1 month ago (2016-11-09 23:05:24 UTC) #3
gone
lgtm overall. Can land this while waiting for UX to answer the question on the ...
4 years, 1 month ago (2016-11-10 00:17:22 UTC) #4
fgorski
lgtm Adding Doug who is working on properly wiring the backend of this notification.
4 years, 1 month ago (2016-11-10 18:39:16 UTC) #6
qinmin
https://codereview.chromium.org/2489143002/diff/1/chrome/android/java/strings/android_chrome_strings.grd File chrome/android/java/strings/android_chrome_strings.grd (right): https://codereview.chromium.org/2489143002/diff/1/chrome/android/java/strings/android_chrome_strings.grd#newcode1627 chrome/android/java/strings/android_chrome_strings.grd:1627: <message name="IDS_DOWNLOAD_NOTIFICATION_PENDING" desc="Download notification to be displayed when a ...
4 years, 1 month ago (2016-11-10 19:07:25 UTC) #7
qinmin
https://codereview.chromium.org/2489143002/diff/1/chrome/android/java/strings/android_chrome_strings.grd File chrome/android/java/strings/android_chrome_strings.grd (right): https://codereview.chromium.org/2489143002/diff/1/chrome/android/java/strings/android_chrome_strings.grd#newcode1627 chrome/android/java/strings/android_chrome_strings.grd:1627: <message name="IDS_DOWNLOAD_NOTIFICATION_PENDING" desc="Download notification to be displayed when a ...
4 years, 1 month ago (2016-11-10 19:07:25 UTC) #8
dougarnett
lgtm
4 years, 1 month ago (2016-11-10 19:53:46 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2489143002/40001
4 years, 1 month ago (2016-11-11 00:38:28 UTC) #12
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 1 month ago (2016-11-11 02:08:02 UTC) #13
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/9f6bbeb42b766bc9b4385e409a6adb88a85e6c8d Cr-Commit-Position: refs/heads/master@{#431462}
4 years, 1 month ago (2016-11-11 02:14:32 UTC) #15
dougarnett
4 years ago (2016-11-23 19:00:09 UTC) #16
Message was sent while issue was closed.
https://codereview.chromium.org/2489143002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageNotificationBridge.java
(right):

https://codereview.chromium.org/2489143002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/offlinepages/downloads/OfflinePageNotificationBridge.java:81:
int percentage = isOfflining ? 0 : -1;
Min, I wonder if passing isOffling (or isPending (!isOfflining)) more directly
than coercing into percentage value would be better for getting the UX right.

Powered by Google App Engine
This is Rietveld 408576698