Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
55973003:
[rAc OSX] Add support for drawing borders around dialog notifications. (Closed)
|
Created:
7 years, 1 month ago by Ilya Sherman Modified:
7 years, 1 month ago Reviewers:
groby-ooo-7-16 CC:
chromium-reviews, benquan, Dane Wallinga, dyu1, estade+watch_chromium.org, Ilya Sherman, rouslan+autofillwatch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
Description[rAc OSX] Add support for drawing borders around dialog notifications.
BUG=286528
TEST=Top notification in Wallet mode (i.e. when signed in) should have a subtle
darker gray border.
R=groby@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=232981
Patch Set 1 #
Total comments: 4
Patch Set 2 : Change designated initializer to -initWithNotification: #Patch Set 3 : Fix typo #Patch Set 4 : Rebase #Patch Set 5 : Rebase #Messages
Total messages: 30 (0 generated)
https://codereview.chromium.org/55973003/diff/1/chrome/browser/ui/cocoa/autof... File chrome/browser/ui/cocoa/autofill/autofill_notification_container.mm (right): https://codereview.chromium.org/55973003/diff/1/chrome/browser/ui/cocoa/autof... chrome/browser/ui/cocoa/autofill/autofill_notification_container.mm:78: notificationController([[AutofillNotificationController alloc] init]); At this point, it'd be better to have -initWithNotification: :) https://codereview.chromium.org/55973003/diff/1/chrome/browser/ui/cocoa/autof... File chrome/browser/ui/cocoa/autofill/autofill_notification_controller.mm (right): https://codereview.chromium.org/55973003/diff/1/chrome/browser/ui/cocoa/autof... chrome/browser/ui/cocoa/autofill/autofill_notification_controller.mm:178: - (NSColor*)borderColor { Added bonus of init - we can probably kill most of the setters/getters
https://codereview.chromium.org/55973003/diff/1/chrome/browser/ui/cocoa/autof... File chrome/browser/ui/cocoa/autofill/autofill_notification_container.mm (right): https://codereview.chromium.org/55973003/diff/1/chrome/browser/ui/cocoa/autof... chrome/browser/ui/cocoa/autofill/autofill_notification_container.mm:78: notificationController([[AutofillNotificationController alloc] init]); On 2013/11/01 21:56:40, groby wrote: > At this point, it'd be better to have -initWithNotification: :) Done. https://codereview.chromium.org/55973003/diff/1/chrome/browser/ui/cocoa/autof... File chrome/browser/ui/cocoa/autofill/autofill_notification_controller.mm (right): https://codereview.chromium.org/55973003/diff/1/chrome/browser/ui/cocoa/autof... chrome/browser/ui/cocoa/autofill/autofill_notification_controller.mm:178: - (NSColor*)borderColor { On 2013/11/01 21:56:40, groby wrote: > Added bonus of init - we can probably kill most of the setters/getters Done.
lgtm
lgtm lgtm
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/90001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/310001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/310001
Retried try job too often on linux_aura for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/isherman@chromium.org/55973003/310001
Message was sent while issue was closed.
Change committed as 232981 |
