Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(409)

Issue 2391393002: Harmony - Use FocusableBorder for comboboxes on Mac (Closed)

Created:
4 years, 2 months ago by Evan Stade
Modified:
4 years, 2 months ago
Reviewers:
Elly Fong-Jones, sadrul
CC:
chromium-reviews, tfarina
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Harmony - Use FocusableBorder for comboboxes on Mac BUG=653207 Committed: https://crrev.com/ca213587627e1b6dfd80040398db3cff98131579 Cr-Commit-Position: refs/heads/master@{#424224}

Patch Set 1 #

Total comments: 2

Patch Set 2 : also bg #

Patch Set 3 : more #

Total comments: 1

Patch Set 4 : remove include #

Patch Set 5 : delete border #

Patch Set 6 : fix mac #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -153 lines) Patch
M ui/views/BUILD.gn View 1 2 3 4 5 1 chunk +0 lines, -2 lines 0 comments Download
M ui/views/controls/combobox/combobox.cc View 1 2 3 4 2 chunks +1 line, -8 lines 0 comments Download
D ui/views/style/mac/combobox_background_mac.h View 1 1 chunk +0 lines, -42 lines 0 comments Download
D ui/views/style/mac/combobox_background_mac.cc View 1 1 chunk +0 lines, -70 lines 0 comments Download
M ui/views/style/platform_style.h View 1 2 3 4 5 1 chunk +0 lines, -7 lines 0 comments Download
M ui/views/style/platform_style.cc View 1 2 3 4 5 1 chunk +0 lines, -11 lines 0 comments Download
M ui/views/style/platform_style_mac.mm View 1 2 3 4 5 2 chunks +0 lines, -13 lines 0 comments Download

Messages

Total messages: 34 (20 generated)
Evan Stade
4 years, 2 months ago (2016-10-05 19:21:49 UTC) #2
Elly Fong-Jones
lgtm - I put some screenshots on the bug, too https://codereview.chromium.org/2391393002/diff/1/ui/views/controls/combobox/combobox.cc File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2391393002/diff/1/ui/views/controls/combobox/combobox.cc#newcode771 ...
4 years, 2 months ago (2016-10-06 14:15:36 UTC) #3
Evan Stade
https://codereview.chromium.org/2391393002/diff/1/ui/views/controls/combobox/combobox.cc File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2391393002/diff/1/ui/views/controls/combobox/combobox.cc#newcode771 ui/views/controls/combobox/combobox.cc:771: UseMd() ? new FocusableBorder() : PlatformStyle::CreateComboboxBorder()); On 2016/10/06 14:15:36, ...
4 years, 2 months ago (2016-10-06 15:51:08 UTC) #5
Elly Fong-Jones
On 2016/10/06 15:51:08, Evan Stade wrote: > https://codereview.chromium.org/2391393002/diff/1/ui/views/controls/combobox/combobox.cc > File ui/views/controls/combobox/combobox.cc (right): > > https://codereview.chromium.org/2391393002/diff/1/ui/views/controls/combobox/combobox.cc#newcode771 ...
4 years, 2 months ago (2016-10-06 15:54:27 UTC) #8
Evan Stade
https://codereview.chromium.org/2391393002/diff/40001/ui/views/controls/combobox/combobox.cc File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/2391393002/diff/40001/ui/views/controls/combobox/combobox.cc#newcode547 ui/views/controls/combobox/combobox.cc:547: set_background(Background::CreateBackgroundPainter( ah, look --- the bg was being overridden ...
4 years, 2 months ago (2016-10-06 15:56:37 UTC) #9
Evan Stade
to balance review load, -sky, +sadrul
4 years, 2 months ago (2016-10-06 15:57:11 UTC) #11
sadrul
lgtm (the build failure on mac bots are related, but looks easily fixable)
4 years, 2 months ago (2016-10-07 00:48:23 UTC) #18
Elly Fong-Jones
On 2016/10/07 00:48:23, sadrul wrote: > lgtm (the build failure on mac bots are related, ...
4 years, 2 months ago (2016-10-10 13:22:17 UTC) #19
Evan Stade
On 2016/10/10 13:22:17, Elly Fong-Jones wrote: > On 2016/10/07 00:48:23, sadrul wrote: > > lgtm ...
4 years, 2 months ago (2016-10-10 17:17:01 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2391393002/80001
4 years, 2 months ago (2016-10-10 17:46:41 UTC) #26
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/283277)
4 years, 2 months ago (2016-10-10 18:12:21 UTC) #28
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2391393002/100001
4 years, 2 months ago (2016-10-10 18:36:28 UTC) #31
commit-bot: I haz the power
Committed patchset #6 (id:100001)
4 years, 2 months ago (2016-10-10 20:33:51 UTC) #32
commit-bot: I haz the power
4 years, 2 months ago (2016-10-10 20:35:33 UTC) #34
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/ca213587627e1b6dfd80040398db3cff98131579
Cr-Commit-Position: refs/heads/master@{#424224}

Powered by Google App Engine
This is Rietveld 408576698