Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2668033004:
[ash] Fix message center maximum height for vertical shelf alignments. (Closed)
|
Created:
3 years, 10 months ago by Eliot Courtney Modified:
3 years, 10 months ago CC:
chromium-reviews, kalyank, sadrul Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[ash] Fix message center maximum height for vertical shelf alignments.
Previously, the maximum height was calculated using the bottom of the
status area, which is almost at the bottom of the screen. Instead, use
the bottom of WebNotificationTray.
BUG=669739
Review-Url: https://codereview.chromium.org/2668033004
Cr-Commit-Position: refs/heads/master@{#448844}
Committed: https://chromium.googlesource.com/chromium/src/+/438483b40f68a8309fc6ebe5b8135f5be7f14910
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address comments. #Messages
Total messages: 20 (11 generated)
edcourtney@chromium.org changed reviewers: + stevenjb@chromium.org, yoshiki@chromium.org
The CQ bit was checked by edcourtney@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 with nit https://codereview.chromium.org/2668033004/diff/1/ash/common/system/web_notif... File ash/common/system/web_notification/web_notification_tray.cc (right): https://codereview.chromium.org/2668033004/diff/1/ash/common/system/web_notif... ash/common/system/web_notification/web_notification_tray.cc:363: if (IsHorizontalAlignment(shelf_alignment())) nit: how about using a ternary conditional operator for readability, like int max_height = IsHorizontalAlignment(shelf_alignment()) ? ... : ...;
https://codereview.chromium.org/2668033004/diff/1/ash/common/system/web_notif... File ash/common/system/web_notification/web_notification_tray.cc (right): https://codereview.chromium.org/2668033004/diff/1/ash/common/system/web_notif... ash/common/system/web_notification/web_notification_tray.cc:363: if (IsHorizontalAlignment(shelf_alignment())) On 2017/02/02 03:25:23, yoshiki wrote: > nit: how about using a ternary conditional operator for readability, like > > int max_height = IsHorizontalAlignment(shelf_alignment()) ? ... : ...; Done.
The CQ bit was checked by edcourtney@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from yoshiki@chromium.org Link to the patchset: https://codereview.chromium.org/2668033004/#ps20001 (title: "Address comments.")
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
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...) ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
Steven, could you review for owners please?
lgtm
The CQ bit was checked by edcourtney@chromium.org
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": 20001, "attempt_start_ts": 1486516221483910,
"parent_rev": "0d21d6aa2404e45d9bc1d12afeec9bedc8114bbf", "commit_rev":
"438483b40f68a8309fc6ebe5b8135f5be7f14910"}
Message was sent while issue was closed.
Description was changed from ========== [ash] Fix message center maximum height for vertical shelf alignments. Previously, the maximum height was calculated using the bottom of the status area, which is almost at the bottom of the screen. Instead, use the bottom of WebNotificationTray. BUG=669739 ========== to ========== [ash] Fix message center maximum height for vertical shelf alignments. Previously, the maximum height was calculated using the bottom of the status area, which is almost at the bottom of the screen. Instead, use the bottom of WebNotificationTray. BUG=669739 Review-Url: https://codereview.chromium.org/2668033004 Cr-Commit-Position: refs/heads/master@{#448844} Committed: https://chromium.googlesource.com/chromium/src/+/438483b40f68a8309fc6ebe5b813... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/438483b40f68a8309fc6ebe5b813... |
