Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2513083002:
[Mac] Fixed the incognito omnibox icon background color (Closed)
Description[Mac] Fixed the incognito omnibox icon background color
BUG=588377
Committed: https://crrev.com/40b30591e4bb05a0f5fcf4ea85708927ab177c45
Cr-Commit-Position: refs/heads/master@{#433338}
Patch Set 1 #
Total comments: 3
Messages
Total messages: 19 (10 generated)
The CQ bit was checked by spqchan@chromium.org to run a CQ dry run
Description was changed from ========== [Mac] Fixed the incognito omnibox icon background color BUG=588377 ========== to ========== [Mac] Fixed the incognito omnibox icon background color BUG=588377 ==========
spqchan@chromium.org changed reviewers: + rsesek@chromium.org
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.
PTAL
LGTM
The CQ bit was checked by spqchan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [Mac] Fixed the incognito omnibox icon background color BUG=588377 ========== to ========== [Mac] Fixed the incognito omnibox icon background color BUG=588377 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
shrike@chromium.org changed reviewers: + shrike@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/2513083002/diff/1/chrome/browser/ui/cocoa/loc... File chrome/browser/ui/cocoa/location_bar/location_bar_decoration.mm (left): https://codereview.chromium.org/2513083002/diff/1/chrome/browser/ui/cocoa/loc... chrome/browser/ui/cocoa/location_bar/location_bar_decoration.mm:180: in_dark_mode ? kHoverDarkBackgroundColor : kHoverBackgroundColor; This line computes the Pressed state color (based on the if statement) but here you are returning the hover color. As a result, when I run it the hover state is lighter than the pressed state.
Message was sent while issue was closed.
https://codereview.chromium.org/2513083002/diff/1/chrome/browser/ui/cocoa/loc... File chrome/browser/ui/cocoa/location_bar/location_bar_decoration.mm (left): https://codereview.chromium.org/2513083002/diff/1/chrome/browser/ui/cocoa/loc... chrome/browser/ui/cocoa/location_bar/location_bar_decoration.mm:180: in_dark_mode ? kHoverDarkBackgroundColor : kHoverBackgroundColor; On 2016/11/19 00:01:23, shrike wrote: > This line computes the Pressed state color (based on the if statement) but here > you are returning the hover color. As a result, when I run it the hover state is > lighter than the pressed state. I'd expect hover to be lighter than pressed?
Message was sent while issue was closed.
On 2016/11/19 00:02:43, Robert Sesek wrote: > https://codereview.chromium.org/2513083002/diff/1/chrome/browser/ui/cocoa/loc... > File chrome/browser/ui/cocoa/location_bar/location_bar_decoration.mm (left): > > https://codereview.chromium.org/2513083002/diff/1/chrome/browser/ui/cocoa/loc... > chrome/browser/ui/cocoa/location_bar/location_bar_decoration.mm:180: > in_dark_mode ? kHoverDarkBackgroundColor : kHoverBackgroundColor; > On 2016/11/19 00:01:23, shrike wrote: > > This line computes the Pressed state color (based on the if statement) but > here > > you are returning the hover color. As a result, when I run it the hover state > is > > lighter than the pressed state. > > I'd expect hover to be lighter than pressed? Not in incognito mode.
Message was sent while issue was closed.
Description was changed from ========== [Mac] Fixed the incognito omnibox icon background color BUG=588377 ========== to ========== [Mac] Fixed the incognito omnibox icon background color BUG=588377 Committed: https://crrev.com/40b30591e4bb05a0f5fcf4ea85708927ab177c45 Cr-Commit-Position: refs/heads/master@{#433338} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/40b30591e4bb05a0f5fcf4ea85708927ab177c45 Cr-Commit-Position: refs/heads/master@{#433338}
Message was sent while issue was closed.
https://codereview.chromium.org/2513083002/diff/1/chrome/browser/ui/cocoa/loc... File chrome/browser/ui/cocoa/location_bar/location_bar_decoration.mm (left): https://codereview.chromium.org/2513083002/diff/1/chrome/browser/ui/cocoa/loc... chrome/browser/ui/cocoa/location_bar/location_bar_decoration.mm:180: in_dark_mode ? kHoverDarkBackgroundColor : kHoverBackgroundColor; On 2016/11/19 00:02:42, Robert Sesek wrote: > On 2016/11/19 00:01:23, shrike wrote: > > This line computes the Pressed state color (based on the if statement) but > here > > you are returning the hover color. As a result, when I run it the hover state > is > > lighter than the pressed state. > > I'd expect hover to be lighter than pressed? I can't reproduce that. It's matching the specs for me |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
