Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2661163003:
[Chrome OS] Fix the broken focus rectangle on Launcher icon. (Closed)
Description[Chrome OS] Fix the broken focus rectangle on Launcher icon.
BUG=683438
TEST=manually
Review-Url: https://codereview.chromium.org/2661163003
Cr-Commit-Position: refs/heads/master@{#447445}
Committed: https://chromium.googlesource.com/chromium/src/+/58116ee5b2710dd751d968024489b305ae18b038
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address estade@'s comment. #Messages
Total messages: 19 (12 generated)
xdai@chromium.org changed reviewers: + estade@chromium.org, oshima@chromium.org
oshima@, estade@, could you help review this CL please? Thanks!
The CQ bit was checked by xdai@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...
https://codereview.chromium.org/2661163003/diff/1/ash/common/shelf/app_list_b... File ash/common/shelf/app_list_button.cc (left): https://codereview.chromium.org/2661163003/diff/1/ash/common/shelf/app_list_b... ash/common/shelf/app_list_button.cc:173: // ring draws sharply and is centered at all scale factors. This logic looks pretty intentional. Are you sure you aren't breaking something? I'm pretty sure what you actually want is a ScopedCanvas to undo-undo-dsf.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
estade@, I've addressed your comment. Please take another look, thanks! https://codereview.chromium.org/2661163003/diff/1/ash/common/shelf/app_list_b... File ash/common/shelf/app_list_button.cc (left): https://codereview.chromium.org/2661163003/diff/1/ash/common/shelf/app_list_b... ash/common/shelf/app_list_button.cc:173: // ring draws sharply and is centered at all scale factors. On 2017/02/01 00:38:13, Evan Stade wrote: > This logic looks pretty intentional. Are you sure you aren't breaking something? > > I'm pretty sure what you actually want is a ScopedCanvas to undo-undo-dsf. Sorry for the previous bad fix. Modified as you suggested. Thanks!
The CQ bit was checked by xdai@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...
excellent, thanks. lgtm
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by xdai@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": 1485922096221280,
"parent_rev": "f24137b569eb8eb7a58f875c18f8d17c5aef8bac", "commit_rev":
"58116ee5b2710dd751d968024489b305ae18b038"}
Message was sent while issue was closed.
Description was changed from ========== [Chrome OS] Fix the broken focus rectangle on Launcher icon. BUG=683438 TEST=manually ========== to ========== [Chrome OS] Fix the broken focus rectangle on Launcher icon. BUG=683438 TEST=manually Review-Url: https://codereview.chromium.org/2661163003 Cr-Commit-Position: refs/heads/master@{#447445} Committed: https://chromium.googlesource.com/chromium/src/+/58116ee5b2710dd751d968024489... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/58116ee5b2710dd751d968024489... |
