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

Issue 1817613002: MacViews: draw Mac combobox arrows (Closed)

Created:
4 years, 9 months ago by Elly Fong-Jones
Modified:
4 years, 8 months ago
Reviewers:
tapted, sky
CC:
chromium-reviews, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

MacViews: draw Mac combobox arrows This CL adds Mac-specific combobox arrow drawing code to NativeThemeMac. BUG=543683 TEST=adhoc Build views_examples_with_content_exe and look at the Combobox page. Committed: https://crrev.com/0cdd64944cabbca4072e01f0da8326eea9b1cc9e Cr-Commit-Position: refs/heads/master@{#385759}

Patch Set 1 #

Total comments: 8

Patch Set 2 : Use vector icon #

Total comments: 2

Patch Set 3 : Move drawing to PlatformStyle/Combobox #

Total comments: 8

Patch Set 4 : Remove PaintComboboxArrow #

Total comments: 14

Patch Set 5 : gn updates &c #

Patch Set 6 : remove noop image_bounds #

Unified diffs Side-by-side diffs Delta from patch set Stats (+51 lines, -84 lines) Patch
M ui/gfx/BUILD.gn View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
A + ui/gfx/vector_icons/combobox_arrow_mac.icon View 1 1 chunk +7 lines, -2 lines 0 comments Download
M ui/native_theme/common_theme.h View 1 2 3 4 1 chunk +0 lines, -9 lines 0 comments Download
M ui/native_theme/common_theme.cc View 1 2 3 4 1 chunk +0 lines, -22 lines 0 comments Download
M ui/native_theme/native_theme.h View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M ui/native_theme/native_theme_aurawin.cc View 1 2 3 4 1 chunk +0 lines, -4 lines 0 comments Download
M ui/native_theme/native_theme_base.cc View 1 2 3 2 chunks +0 lines, -7 lines 0 comments Download
M ui/native_theme/native_theme_win.cc View 1 2 3 4 6 chunks +0 lines, -11 lines 0 comments Download
M ui/views/controls/combobox/combobox.cc View 1 2 3 4 5 2 chunks +4 lines, -28 lines 0 comments Download
M ui/views/style/platform_style.h View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download
M ui/views/style/platform_style.cc View 1 2 3 4 2 chunks +9 lines, -0 lines 0 comments Download
M ui/views/style/platform_style_mac.mm View 1 2 3 2 chunks +19 lines, -0 lines 0 comments Download

Messages

Total messages: 27 (9 generated)
Elly Fong-Jones
tapted: ptal? :)
4 years, 9 months ago (2016-03-18 19:51:35 UTC) #3
tapted
https://codereview.chromium.org/1817613002/diff/1/ui/native_theme/native_theme_mac.mm File ui/native_theme/native_theme_mac.mm (right): https://codereview.chromium.org/1817613002/diff/1/ui/native_theme/native_theme_mac.mm#newcode277 ui/native_theme/native_theme_mac.mm:277: // TODO(ellyjones): These colors aren't exactly right. What's the ...
4 years, 9 months ago (2016-03-21 06:58:45 UTC) #4
Elly Fong-Jones
tapted: ptal? https://codereview.chromium.org/1817613002/diff/1/ui/native_theme/native_theme_mac.mm File ui/native_theme/native_theme_mac.mm (right): https://codereview.chromium.org/1817613002/diff/1/ui/native_theme/native_theme_mac.mm#newcode277 ui/native_theme/native_theme_mac.mm:277: // TODO(ellyjones): These colors aren't exactly right. ...
4 years, 9 months ago (2016-03-21 21:04:23 UTC) #5
tapted
https://codereview.chromium.org/1817613002/diff/20001/ui/views/controls/combobox/combobox.cc File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/1817613002/diff/20001/ui/views/controls/combobox/combobox.cc#newcode753 ui/views/controls/combobox/combobox.cc:753: params.combobox_arrow.icon_scale_factor = canvas->image_scale(); I'm pretty sure we don't need ...
4 years, 9 months ago (2016-03-21 22:59:59 UTC) #6
Elly Fong-Jones
tapted, sky: ptal? https://codereview.chromium.org/1817613002/diff/20001/ui/views/controls/combobox/combobox.cc File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/1817613002/diff/20001/ui/views/controls/combobox/combobox.cc#newcode753 ui/views/controls/combobox/combobox.cc:753: params.combobox_arrow.icon_scale_factor = canvas->image_scale(); On 2016/03/21 22:59:59, ...
4 years, 9 months ago (2016-03-24 19:53:55 UTC) #8
tapted
Yeah - I think this is a good direction and a nice simplification. (hopefully sky ...
4 years, 8 months ago (2016-03-29 04:16:37 UTC) #9
Elly Fong-Jones
tapted, sky: ptal? :) https://codereview.chromium.org/1817613002/diff/40001/ui/native_theme/native_theme.gyp File ui/native_theme/native_theme.gyp (right): https://codereview.chromium.org/1817613002/diff/40001/ui/native_theme/native_theme.gyp#newcode20 ui/native_theme/native_theme.gyp:20: '../gfx/gfx.gyp:gfx_vector_icons', On 2016/03/29 04:16:36, tapted ...
4 years, 8 months ago (2016-04-04 17:09:55 UTC) #10
sky
LGTM
4 years, 8 months ago (2016-04-04 20:47:55 UTC) #11
tapted
https://codereview.chromium.org/1817613002/diff/60001/ui/gfx/vector_icons/combobox_arrow_mac.icon File ui/gfx/vector_icons/combobox_arrow_mac.icon (right): https://codereview.chromium.org/1817613002/diff/60001/ui/gfx/vector_icons/combobox_arrow_mac.icon#newcode1 ui/gfx/vector_icons/combobox_arrow_mac.icon:1: // Copyright 2016 The Chromium Authors. All rights reserved. ...
4 years, 8 months ago (2016-04-05 08:45:02 UTC) #12
Elly Fong-Jones
tapted: ptal? https://codereview.chromium.org/1817613002/diff/60001/ui/gfx/vector_icons/combobox_arrow_mac.icon File ui/gfx/vector_icons/combobox_arrow_mac.icon (right): https://codereview.chromium.org/1817613002/diff/60001/ui/gfx/vector_icons/combobox_arrow_mac.icon#newcode1 ui/gfx/vector_icons/combobox_arrow_mac.icon:1: // Copyright 2016 The Chromium Authors. All ...
4 years, 8 months ago (2016-04-05 20:04:24 UTC) #13
tapted
looks like your `git cl upload` flaked out - can you re-upload? https://codereview.chromium.org/1817613002/diff/60001/ui/views/controls/combobox/combobox.cc File ui/views/controls/combobox/combobox.cc ...
4 years, 8 months ago (2016-04-06 00:07:08 UTC) #14
Elly Fong-Jones
tapted: ptal? :) https://codereview.chromium.org/1817613002/diff/60001/ui/views/controls/combobox/combobox.cc File ui/views/controls/combobox/combobox.cc (right): https://codereview.chromium.org/1817613002/diff/60001/ui/views/controls/combobox/combobox.cc#newcode759 ui/views/controls/combobox/combobox.cc:759: canvas->DrawImageInt(arrow_image, image_bounds.x(), image_bounds.y()); On 2016/04/06 00:07:08, ...
4 years, 8 months ago (2016-04-06 16:20:21 UTC) #15
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1817613002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1817613002/100001
4 years, 8 months ago (2016-04-06 16:21:25 UTC) #17
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 8 months ago (2016-04-06 17:28:54 UTC) #19
tapted
lgtm
4 years, 8 months ago (2016-04-06 21:20:17 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1817613002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1817613002/100001
4 years, 8 months ago (2016-04-07 15:03:42 UTC) #23
commit-bot: I haz the power
Committed patchset #6 (id:100001)
4 years, 8 months ago (2016-04-07 15:08:41 UTC) #25
commit-bot: I haz the power
4 years, 8 months ago (2016-04-07 15:09:56 UTC) #27
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/0cdd64944cabbca4072e01f0da8326eea9b1cc9e
Cr-Commit-Position: refs/heads/master@{#385759}

Powered by Google App Engine
This is Rietveld 408576698