Download notification layout is broken in new-style notification |
||||
Issue descriptionDownload notification layout is different from the mock. * The caption string "Download Manager" is not shown in the header. * The small icon and its color is different. * The message string is broken. It should show remaining time on the right and other information at the bottom. Also, it should not show the control character "NL".
,
Jul 20 2017
yoshiki@: This item might also need UI string modification, but details are not yet decided, so it's hard to add UI strings before M61 branch point.
,
Jul 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/763959aadf1e04d49c0b997cda3d31296f63f172 commit 763959aadf1e04d49c0b997cda3d31296f63f172 Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Thu Jul 20 10:06:04 2017 Add header UI string that will be used in MD notification. Add "Chrome OS system" string that will be used for system notification when new-style notification is enabled. BUG=738779, 726241 , 740807 , 740813 TEST=manual Change-Id: I2e4ce586385065752abcc4e38445b7e67c6bb2ef Reviewed-on: https://chromium-review.googlesource.com/578696 Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#488182} [modify] https://crrev.com/763959aadf1e04d49c0b997cda3d31296f63f172/ui/strings/ui_strings.grd
,
Jul 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/05ee598c956d4c69d492678dc1f64e573c763de1 commit 05ee598c956d4c69d492678dc1f64e573c763de1 Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Thu Jul 20 10:09:15 2017 Add UI strings for MD style download notification. For better MD style download notification representation, we're going to add new attributes to Notification class, and going to use these strings. TEST=manual BUG= 740807 Change-Id: I05a5f1a2094814b579aee886297732b0df2c56b5 Reviewed-on: https://chromium-review.googlesource.com/578752 Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#488184} [modify] https://crrev.com/05ee598c956d4c69d492678dc1f64e573c763de1/chrome/app/generated_resources.grd
,
Jul 21 2017
,
Jul 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4fe74ca713e81b5122f01edec2d3ce05c96687cd commit 4fe74ca713e81b5122f01edec2d3ce05c96687cd Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Sat Jul 29 04:40:50 2017 Improve download notification representation in MD. Improve the representation of download notification in MD style notification. * Add status attribute to RichNotificationData. * Add status_view_ in NotificationViewMD and show if status is available. Use message instead if they are not available. * Add GetSubStatusString in DownloadItemNotification. Add short_form argument in GetStatusString. BUG= 740807 TEST=manual Change-Id: Iab9756e0f8eafc9bfce67aade64454e5f586a7ef Reviewed-on: https://chromium-review.googlesource.com/578540 Reviewed-by: David Trainor <dtrainor@chromium.org> Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Reviewed-by: Naoki Fukino <fukino@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#490622} [modify] https://crrev.com/4fe74ca713e81b5122f01edec2d3ce05c96687cd/chrome/browser/download/notification/download_item_notification.cc [modify] https://crrev.com/4fe74ca713e81b5122f01edec2d3ce05c96687cd/chrome/browser/download/notification/download_item_notification.h [modify] https://crrev.com/4fe74ca713e81b5122f01edec2d3ce05c96687cd/ui/message_center/message_center.cc [modify] https://crrev.com/4fe74ca713e81b5122f01edec2d3ce05c96687cd/ui/message_center/message_center.h [modify] https://crrev.com/4fe74ca713e81b5122f01edec2d3ce05c96687cd/ui/message_center/notification.cc [modify] https://crrev.com/4fe74ca713e81b5122f01edec2d3ce05c96687cd/ui/message_center/notification.h [modify] https://crrev.com/4fe74ca713e81b5122f01edec2d3ce05c96687cd/ui/message_center/views/notification_view_md.cc [modify] https://crrev.com/4fe74ca713e81b5122f01edec2d3ce05c96687cd/ui/message_center/views/notification_view_md.h
,
Jul 31 2017
,
Aug 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01f38b46d3446cc7ae3f482d59dbc1841f625479 commit 01f38b46d3446cc7ae3f482d59dbc1841f625479 Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Thu Aug 03 01:16:26 2017 Add first-party notification styling in MD notification. In new-style notification, first-party notification should have different styling compared to general web notifications. To do this, all system notifications should be created by new CreateSystemNotification. (Previously, it was not a requirement.) By using the function, * the display source defaults to string "ChromeOS system". * the notification has single accent color based on SystemNotificationWarningLevel. * gfx::VectorIcon will be passed as small_image instead of gfx::Image to perform icon coloring on CreateSystemNotification side. TEST=out/Debug/message_center_unittests BUG=738779, 726241 , 740807 , 740813 Change-Id: I9e85b3f1de33b6499297f060b5305d0b02157eef Reviewed-on: https://chromium-review.googlesource.com/566765 Reviewed-by: James Cook <jamescook@chromium.org> Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#491592} [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/chrome/browser/chromeos/chrome_browser_main_chromeos.cc [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/BUILD.gn [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/fake_message_center.cc [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/fake_message_center.h [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/message_center.h [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/message_center_impl.cc [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/message_center_impl.h [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/message_center_style.h [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/notification.cc [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/notification.h [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/views/notification_header_view.cc [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/views/notification_header_view.h [modify] https://crrev.com/01f38b46d3446cc7ae3f482d59dbc1841f625479/ui/message_center/views/notification_view_md.cc
,
Aug 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e3561e0926074e531406ce0fb035e1e3a0d9c39 commit 9e3561e0926074e531406ce0fb035e1e3a0d9c39 Author: John Mellor <johnme@chromium.org> Date: Thu Aug 03 11:10:58 2017 Revert "Add first-party notification styling in MD notification." This reverts commit 01f38b46d3446cc7ae3f482d59dbc1841f625479. Reason for revert: Sorry, this broke compilation on 3 bots: - https://build.chromium.org/p/chromium.android/builders/Android%20x86%20Builder%20%28dbg%29 - https://build.chromium.org/p/chromium.android/builders/Android%20x64%20Builder%20%28dbg%29 - https://build.chromium.org/p/chromium.android/builders/Android%20MIPS%20Builder%20%28dbg%29 With the following linker error: obj/ui/message_center/message_center/notification.o: In function `message_center::Notification::CreateSystemNotification(...)': ../../ui/message_center/notification.cc:182: undefined reference to `gfx::kNoneIcon' obj/ui/message_center/message_center/notification.o: In function `message_center::Notification::CreateSystemNotification(...)': ../../ui/message_center/notification.cc:232: undefined reference to `gfx::CreateVectorIcon(gfx::VectorIcon const&, unsigned int)' Original change's description: > Add first-party notification styling in MD notification. > > In new-style notification, first-party notification should have > different styling compared to general web notifications. > To do this, all system notifications should be created by > new CreateSystemNotification. (Previously, it was not a requirement.) > > By using the function, > * the display source defaults to string "ChromeOS system". > * the notification has single accent color based on > SystemNotificationWarningLevel. > * gfx::VectorIcon will be passed as small_image instead of gfx::Image > to perform icon coloring on CreateSystemNotification side. > > TEST=out/Debug/message_center_unittests > BUG=738779, 726241 , 740807 , 740813 > > Change-Id: I9e85b3f1de33b6499297f060b5305d0b02157eef > Reviewed-on: https://chromium-review.googlesource.com/566765 > Reviewed-by: James Cook <jamescook@chromium.org> > Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> > Reviewed-by: Peter Beverloo <peter@chromium.org> > Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> > Cr-Commit-Position: refs/heads/master@{#491592} TBR=jamescook@chromium.org,yoshiki@chromium.org,peter@chromium.org,fukino@chromium.org,tetsui@chromium.org Change-Id: I10fe30dbdad6012fbafe32c17a439065f4d816d3 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 738779, 726241 , 740807 , 740813 Reviewed-on: https://chromium-review.googlesource.com/600067 Reviewed-by: John Mellor <johnme@chromium.org> Commit-Queue: John Mellor <johnme@chromium.org> Cr-Commit-Position: refs/heads/master@{#491696} [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/chrome/browser/chromeos/chrome_browser_main_chromeos.cc [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/BUILD.gn [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/fake_message_center.cc [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/fake_message_center.h [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/message_center.h [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/message_center_impl.cc [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/message_center_impl.h [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/message_center_style.h [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/notification.cc [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/notification.h [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/views/notification_header_view.cc [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/views/notification_header_view.h [modify] https://crrev.com/9e3561e0926074e531406ce0fb035e1e3a0d9c39/ui/message_center/views/notification_view_md.cc
,
Aug 9 2017
,
Aug 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ead8b5b61dacc735f3840152ecc5a11c80c724da commit ead8b5b61dacc735f3840152ecc5a11c80c724da Author: Tetsui Ohkubo <tetsui@chromium.org> Date: Thu Aug 10 02:14:46 2017 Reland "Add first-party notification styling in MD notification." This is a reland of 01f38b46d3446cc7ae3f482d59dbc1841f625479 Fix from the original CL: * Link gfx::VectorIcon and gfx::PaintVectorIcon on Android build, as they are now used in message_center::Notification, which is common for all the platforms that uses ui/message_center. Original change's description: > Add first-party notification styling in MD notification. > > In new-style notification, first-party notification should have > different styling compared to general web notifications. > To do this, all system notifications should be created by > new CreateSystemNotification. (Previously, it was not a requirement.) > > By using the function, > * the display source defaults to string "ChromeOS system". > * the notification has single accent color based on > SystemNotificationWarningLevel. > * gfx::VectorIcon will be passed as small_image instead of gfx::Image > to perform icon coloring on CreateSystemNotification side. > > TEST=out/Debug/message_center_unittests > BUG=738779, 726241 , 740807 , 740813 > > Change-Id: I9e85b3f1de33b6499297f060b5305d0b02157eef > Reviewed-on: https://chromium-review.googlesource.com/566765 > Reviewed-by: James Cook <jamescook@chromium.org> > Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> > Reviewed-by: Peter Beverloo <peter@chromium.org> > Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> > Cr-Commit-Position: refs/heads/master@{#491592} Bug: 738779, 726241 , 740807 , 740813 Change-Id: I62fa4644347ce62d629f3218d319b09ac74db097 Reviewed-on: https://chromium-review.googlesource.com/601611 Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Reviewed-by: Alexei Svitkine (very slow) <asvitkine@chromium.org> Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org> Reviewed-by: James Cook <jamescook@chromium.org> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org> Cr-Commit-Position: refs/heads/master@{#493228} [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/chrome/browser/chromeos/chrome_browser_main_chromeos.cc [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/gfx/BUILD.gn [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/BUILD.gn [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/fake_message_center.cc [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/fake_message_center.h [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/message_center.h [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/message_center_impl.cc [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/message_center_impl.h [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/message_center_style.h [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/notification.cc [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/notification.h [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/views/notification_header_view.cc [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/views/notification_header_view.h [modify] https://crrev.com/ead8b5b61dacc735f3840152ecc5a11c80c724da/ui/message_center/views/notification_view_md.cc
,
Sep 6 2017
mkarkada@: The feature was not launched in M61, so there's no plan for this fix to be backported. It's OK if it is working on M62. Thank you for verifying.
,
Sep 6 2017
@tetsui:Thank you for confirming. |
||||
►
Sign in to add a comment |
||||
Comment 1 by tetsui@chromium.org
, Jul 11 2017