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

Issue 2187473004: Make normal MD text buttons *on infobars* white. (Closed)

Created:
4 years, 4 months ago by Evan Stade
Modified:
4 years, 4 months ago
Reviewers:
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

Make normal MD text buttons *on infobars* white. This is a second attempt to fix the infobar button color. The first attempt was 886005cc219 but that broke the download shelf (particularly noticeable in incognito). This fix is a bandaid in that we should rely on the native theme. However that is difficult right now because we can't use ButtonBackgroundColor until all buttons are MD. Being able to set a custom bg color will also be useful for crbug.com/617478 BUG=629658 Committed: https://crrev.com/032442dec365358e954842c326bc7b1f37397805 Cr-Commit-Position: refs/heads/master@{#407986}

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -30 lines) Patch
M chrome/browser/ui/views/infobars/confirm_infobar.cc View 2 chunks +13 lines, -5 lines 0 comments Download
M chrome/browser/ui/views/infobars/infobar_view.h View 2 chunks +1 line, -4 lines 0 comments Download
M chrome/browser/ui/views/infobars/infobar_view.cc View 3 chunks +2 lines, -12 lines 0 comments Download
M ui/views/controls/button/md_text_button.h View 3 chunks +5 lines, -0 lines 0 comments Download
M ui/views/controls/button/md_text_button.cc View 1 chunk +9 lines, -9 lines 2 comments Download

Messages

Total messages: 15 (8 generated)
Evan Stade
4 years, 4 months ago (2016-07-26 22:12:14 UTC) #3
sky
LGTM https://codereview.chromium.org/2187473004/diff/1/ui/views/controls/button/md_text_button.cc File ui/views/controls/button/md_text_button.cc (right): https://codereview.chromium.org/2187473004/diff/1/ui/views/controls/button/md_text_button.cc#newcode263 ui/views/controls/button/md_text_button.cc:263: bg_color_override_ I'm happy you found a way to ...
4 years, 4 months ago (2016-07-26 23:27:27 UTC) #4
Evan Stade
https://codereview.chromium.org/2187473004/diff/1/ui/views/controls/button/md_text_button.cc File ui/views/controls/button/md_text_button.cc (right): https://codereview.chromium.org/2187473004/diff/1/ui/views/controls/button/md_text_button.cc#newcode263 ui/views/controls/button/md_text_button.cc:263: bg_color_override_ On 2016/07/26 23:27:27, sky wrote: > I'm happy ...
4 years, 4 months ago (2016-07-26 23:29:56 UTC) #5
sky
While I would have gone with if/else there are others that like ternaries. It's one ...
4 years, 4 months ago (2016-07-26 23:44:14 UTC) #8
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/2187473004/1
4 years, 4 months ago (2016-07-26 23:46:45 UTC) #11
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 4 months ago (2016-07-27 00:24:54 UTC) #13
commit-bot: I haz the power
4 years, 4 months ago (2016-07-27 00:26:21 UTC) #15
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/032442dec365358e954842c326bc7b1f37397805
Cr-Commit-Position: refs/heads/master@{#407986}

Powered by Google App Engine
This is Rietveld 408576698