New issue
Advanced search Search tips

Issue 797128 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 755413



Sign in to add a comment

Trim fat from message_center::Notification and RichNotificationData

Project Member Reported by est...@chromium.org, Dec 21 2017

Issue description

1. there's not much logic in terms of which struct a field goes into
2. there are way too many fields that are used for one-off cases. The default in the past seems to be for every new tweak or feature, add a new field.
 

Comment 1 by est...@chromium.org, Dec 21 2017

Summary: Trim fat from message_center::Notification and RichNotificationData (was: Refactor message_center::Notification and RichNotificationData)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3bd8e36812d7bed7b90959a9341fd94549a1ab36

commit 3bd8e36812d7bed7b90959a9341fd94549a1ab36
Author: Evan Stade <estade@chromium.org>
Date: Fri Jan 12 20:38:26 2018

Remove RichNotificationData::use_image_as_icon

Instead, make it implicit by using a scaled down version of the image
in place of the icon only if the icon is empty (and the image is not).

This simplifies Notification/RichNotificationData. There is a slight
behavioral change, which is probably a good thing, in that web
notifications that specify an image but not an icon will now show a
small preview of the image even without being in an expanded state.

Bug: 797128
Change-Id: I8aaa01cf89c419f2407dc39644076932ef093b29
Reviewed-on: https://chromium-review.googlesource.com/841425
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529052}
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/chrome/browser/download/notification/download_item_notification.cc
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/chrome/browser/ui/ash/chrome_screenshot_grabber.cc
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/ui/message_center/mojo/notification.mojom
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/ui/message_center/mojo/notification_struct_traits.cc
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/ui/message_center/mojo/notification_struct_traits.h
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/ui/message_center/mojo/struct_traits_unittest.cc
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/ui/message_center/notification.cc
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/ui/message_center/notification.h
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/ui/message_center/views/notification_view_md.cc
[modify] https://crrev.com/3bd8e36812d7bed7b90959a9341fd94549a1ab36/ui/message_center/views/notification_view_md_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021

commit 0ac4fa20c20264d3b462def8b8bde0fe0b6cf021
Author: Evan Stade <estade@chromium.org>
Date: Tue Feb 06 17:35:55 2018

Get rid of message_center::ButtonType enum.

Intuit the button type from the presence or absence of the placeholder
text.

Bug: 797128
Change-Id: I6fe658f644e42b0556ffcaac9bc7322f44f88fea
Reviewed-on: https://chromium-review.googlesource.com/900184
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534704}
[modify] https://crrev.com/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021/chrome/browser/notifications/notification_platform_bridge_android.cc
[modify] https://crrev.com/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021/chrome/browser/notifications/notification_template_builder.cc
[modify] https://crrev.com/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021/chrome/browser/notifications/notification_template_builder_unittest.cc
[modify] https://crrev.com/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021/chrome/browser/notifications/platform_notification_service_impl.cc
[modify] https://crrev.com/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021/chrome/browser/notifications/platform_notification_service_unittest.cc
[modify] https://crrev.com/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021/ui/message_center/public/cpp/notification.h
[modify] https://crrev.com/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021/ui/message_center/views/notification_view_md.cc
[modify] https://crrev.com/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021/ui/message_center/views/notification_view_md.h
[modify] https://crrev.com/0ac4fa20c20264d3b462def8b8bde0fe0b6cf021/ui/message_center/views/notification_view_md_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27d8b648d53863a648991f2e95dfe17ddfa8cd6e

commit 27d8b648d53863a648991f2e95dfe17ddfa8cd6e
Author: Evan Stade <estade@chromium.org>
Date: Tue Mar 27 22:08:47 2018

Remove some MessageCenter UI flags from message_center::Notification.

These bools are internal details of NotificationList. They were added
to Notification purely for convenience.

TBR=dtrainor@chromium.org
BUG=797128

Change-Id: I69625efb0345f9a5e5c046f845076f11562d708d
Reviewed-on: https://chromium-review.googlesource.com/973882
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546289}
[modify] https://crrev.com/27d8b648d53863a648991f2e95dfe17ddfa8cd6e/chrome/browser/download/notification/download_item_notification_unittest.cc
[modify] https://crrev.com/27d8b648d53863a648991f2e95dfe17ddfa8cd6e/ui/message_center/message_center_impl.cc
[modify] https://crrev.com/27d8b648d53863a648991f2e95dfe17ddfa8cd6e/ui/message_center/notification_list.cc
[modify] https://crrev.com/27d8b648d53863a648991f2e95dfe17ddfa8cd6e/ui/message_center/notification_list.h
[modify] https://crrev.com/27d8b648d53863a648991f2e95dfe17ddfa8cd6e/ui/message_center/notification_list_unittest.cc
[modify] https://crrev.com/27d8b648d53863a648991f2e95dfe17ddfa8cd6e/ui/message_center/public/cpp/notification.cc
[modify] https://crrev.com/27d8b648d53863a648991f2e95dfe17ddfa8cd6e/ui/message_center/public/cpp/notification.h

Labels: Hotlist-DesktopUIChecked
***UI Mass Triage ***

If there is no pending work, please feel free to close the issue.

Sign in to add a comment