omnibox: use dark border stroke in Increase Contrast mode

omnibox: use dark border stroke in Increase Contrast mode

BUG=687227
Review-Url: https://codereview.chromium.org/2710003003
Cr-Commit-Position: refs/heads/master@{#452546}
Committed: https://chromium.googlesource.com/chromium/src/+/2c766c7081a39d69bdde6f008f1c5e0dc6d4388c

[Elly Fong-Jones, 2017-02-22 20:27:55]

Description was changed from

==========
omnibox: use dark border stroke in Increase Contrast mode

BUG=687227
==========

to

==========
omnibox: use dark border stroke in Increase Contrast mode

BUG=687227
==========

[Elly Fong-Jones, 2017-02-22 20:27:55]

ellyjones@chromium.org changed reviewers:
+ mark@chromium.org

[Elly Fong-Jones, 2017-02-22 20:28:00]

mark: ptal? :)

[Mark Mentovai, 2017-02-23 01:57:28]

LGTM

https://codereview.chromium.org/2710003003/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm
(right):

https://codereview.chromium.org/2710003003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm:367: const
CGFloat kNormalStrokeGray = 168 / 255.;
Scope tightly to where it’s used, inside the else branch within the if.

https://codereview.chromium.org/2710003003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm:373: const
CGFloat k30PercentAlpha = 0.3;
Me too.

https://codereview.chromium.org/2710003003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm:378: }
Not required, but consider factoring the -set out to be common.

  NSColor* color;
  if (!inDarkMode) {
    if (increaseContrast) {
      color = [NSColor blackColor];
  // … blah blah blah
    }
  }
  [color set];

[Elly Fong-Jones, 2017-02-23 17:32:16]

https://codereview.chromium.org/2710003003/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm
(right):

https://codereview.chromium.org/2710003003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm:367: const
CGFloat kNormalStrokeGray = 168 / 255.;
On 2017/02/23 01:57:28, Mark Mentovai wrote:
> Scope tightly to where it’s used, inside the else branch within the if.

Done.

https://codereview.chromium.org/2710003003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm:373: const
CGFloat k30PercentAlpha = 0.3;
On 2017/02/23 01:57:28, Mark Mentovai wrote:
> Me too.

Done.

https://codereview.chromium.org/2710003003/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_cell.mm:378: }
On 2017/02/23 01:57:28, Mark Mentovai wrote:
> Not required, but consider factoring the -set out to be common.
> 
>   NSColor* color;
>   if (!inDarkMode) {
>     if (increaseContrast) {
>       color = [NSColor blackColor];
>   // … blah blah blah
>     }
>   }
>   [color set];

I thought about doing this but I don't think it's significantly more or less
clear, so I just left it.

[Elly Fong-Jones, 2017-02-23 17:32:19]

The CQ bit was checked by ellyjones@chromium.org

[Elly Fong-Jones, 2017-02-23 17:32:19]

The patchset sent to the CQ was uploaded after l-g-t-m from mark@chromium.org
Link to the patchset: https://codereview.chromium.org/2710003003/#ps20001
(title: "relocate some constants")

[commit-bot: I haz the power, 2017-02-23 17:32:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 17:53:56]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1487871139442240,
"parent_rev": "48ef9e66f24c63b33dda22ab7fc1eaa98852fde7", "commit_rev":
"2c766c7081a39d69bdde6f008f1c5e0dc6d4388c"}

[commit-bot: I haz the power, 2017-02-23 17:54:37]

Description was changed from

==========
omnibox: use dark border stroke in Increase Contrast mode

BUG=687227
==========

to

==========
omnibox: use dark border stroke in Increase Contrast mode

BUG=687227

Review-Url: https://codereview.chromium.org/2710003003
Cr-Commit-Position: refs/heads/master@{#452546}
Committed:
https://chromium.googlesource.com/chromium/src/+/2c766c7081a39d69bdde6f008f1c...
==========

[commit-bot: I haz the power, 2017-02-23 17:54:46]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2c766c7081a39d69bdde6f008f1c...