Hi, I have a CL here: https://codereview.chromium.org/2914163004/ , but I need feedback on two items. See screenshot.

 1) No vertical shadows. There's no provision in Skia for drawing those currently, but I can write it (in a separate CL). How do you guys want the vertical shadows to look like?

 2) The top-shadow of the Omnibox dropdown is actually one pixel above the bottom-border of the top-toolbar. In the mocks, it's the same line. I assume you guys want to shift the whole dropdown down one pixel in the matched-width mode right?

Thanks! I'm sending this CL to pkasting@ for review anyways, and we'll fix items #1 and #2 in a followup.

Deleted: Screenshot from 2017-06-02 11:14:14.png 33.1 KB

	Screenshot from 2017-06-02 11:14:14.png 33.1 KB View Download

Mac equivalent patch: https://codereview.chromium.org/2916413002

Mac still has the vertical shadow question -- and the rounded corners too... but no off-by-one-pixel issue.

Deleted: Screen Shot 2017-06-02 at 2.31.40 PM.png 397 KB

	Screen Shot 2017-06-02 at 2.31.40 PM.png 397 KB View Download

Deleted: Screen Shot 2017-06-02 at 2.32.29 PM.png 210 KB

	Screen Shot 2017-06-02 at 2.32.29 PM.png 210 KB View Download

Can you screenshot this on a black background page?

If we're returning to the old world of an omnibox-width dropdown, I think we also need to return to the old style of outlining the whole dialog, or there just isn't enough visual distinction from the page background.  I would suggest drawing an outline that exactly matches the omnibox' outline, whose top border is aligned with the toolbar bottom edge.  Note that this alignment may be tricky to get right in scale factors >1 so be sure to test there.

pkasting: He's a screenshot with a black background.

I agree with you on the need for some kind of outline by the way -- would it be fine to address the outline in a separate followup for each of Views and Cocoa?

Tommy

Deleted: Screenshot from 2017-06-05 09:46:40.png 58.1 KB

	Screenshot from 2017-06-05 09:46:40.png 58.1 KB View Download

Can you try to implement something like what I've attached?  I think that's the direction we want to go with this.

I'm OK with not necessarily landing all of this in one CL, especially for multiple platforms, but it'd be nice to sketch it out locally enough to know that your design is going to get there.

Also I really thought the pre-1993 UI was similar to this.  You may want to grab old Chromium builds, see if you can find something close, and look at how the source code did that.

Deleted: desired.png 95.6 KB

	desired.png 95.6 KB View Download

pkasting: Okay great, thanks for the desired.png and a pointer to the old Chromium builds.

I'll also double check with Max to see if he had any designer input.

After that I'll sketch out how I can proceed to the designed state and send it back to you for another look.

Thanks

Note that that mock doesn't put a border in the omnibox like it should have.  I was mainly just going for shape and position.

Err, in the dropdown I mean.

pkasting: 

Okay gotcha. I was going to hew as closely to Max's mock as possible:

https://gallery.googleplex.com/projects/MCHbtQVoQ2HCZePKuXhPkpqK/files/MCFUJVnt41h6xrvKx58okBThwMSJbxAVNpw

It looks like Max's mock has a 1px gray border all the way around as well as a drop shadow. The top border is aligned with the button-border and not rounded. The bottom is rounded.

Does that look right to you? If so, I think we're pretty much all in agreement and can proceed to implementation.

That's another possible route.  Since it's in "explorations" I dunno how much he thought about the details, or how much we care about the details at this point.  Max, want to chime in with your thoughts?

Hey pkasting: Here's a strawman CL that implements a simple rounded border and drops the shadows: https://codereview.chromium.org/2923073004/

Hopefully that is reassurance that we have a path forward to new styling choices.

I scheduled a meeting tomorrow morning at 9:30AM with Max to finalize styling for the narrow case. Let me know if you want to join.

Tommy

Deleted: Screenshot from 2017-06-06 12:37:35.png 84.0 KB

	Screenshot from 2017-06-06 12:37:35.png 84.0 KB View Download

Deleted: Screenshot from 2017-06-06 12:38:09.png 28.5 KB

	Screenshot from 2017-06-06 12:38:09.png 28.5 KB View Download

I can't make the meeting tomorrow (will be heading to the Chrome diversity summit), but I trust you and Max to put together something awesome :)

pkasting: Great. We'll use your mock in c#6 and Max's existing mocks as a starting point.

pkasting / others:

Just met with Emily and Max. Our conclusions:

 1. Yes to shadows on left and right. Follow harmony spec that Max will send.
 
 2. Max will send harmony spec to Tommy. In general we will follow its spec for popovers.
 
 3. Rounding of corners on bottom only ideally.
 
 4. No 1px overlap (see c#2). Probably abolish it in the non-narrow case too. It's just a legacy holdover. Suggestions not a security surface.

 5. In general, the original mock stands. Max to make more detailed revisions and send to Tommy later on.

Hey,

I updated this CL (https://codereview.chromium.org/2923073004/) to make it follow how other bubbles look (per our discussion this morning).

Attached are screenshots of the small shadow (what's actually in the CL) and the big shadow which is meant to just provide food for thought as to how big we want the shadow to actually be.

Once I get Max's specs, it's possible we will implement a custom omnibox-only shadow.

Deleted: Screenshot from 2017-06-07 15:00:08.png 101 KB

	Screenshot from 2017-06-07 15:00:08.png 101 KB View Download

Deleted: Screenshot from 2017-06-07 15:01:06.png 100 KB

	Screenshot from 2017-06-07 15:01:06.png 100 KB View Download

If possible, let's try to make the omnibox shadow not be custom, just for the sake of "fewer slightly different UI paradigms in Chrome" :)

I'm fine with comment 15.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0695a7245850b35c774a67028549af90941f481a

commit 0695a7245850b35c774a67028549af90941f481a
Author: tommycli <tommycli@chromium.org>
Date: Fri Jun 09 17:32:34 2017

Omnibox UI Experiments: Make suggestions dropdown match omnibox width.

Adds flag and Views implementation. Cocoa implementation to follow.

BUG= 728844 

Review-Url: https://codereview.chromium.org/2914163004
Cr-Commit-Position: refs/heads/master@{#478322}

[modify] https://crrev.com/0695a7245850b35c774a67028549af90941f481a/chrome/browser/about_flags.cc
[modify] https://crrev.com/0695a7245850b35c774a67028549af90941f481a/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/0695a7245850b35c774a67028549af90941f481a/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/0695a7245850b35c774a67028549af90941f481a/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/0695a7245850b35c774a67028549af90941f481a/components/omnibox/browser/omnibox_field_trial.cc
[modify] https://crrev.com/0695a7245850b35c774a67028549af90941f481a/components/omnibox/browser/omnibox_field_trial.h
[modify] https://crrev.com/0695a7245850b35c774a67028549af90941f481a/tools/metrics/histograms/enums.xml

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/619e52d6d7e054dfffb13053af73204fa6d0eb69

commit 619e52d6d7e054dfffb13053af73204fa6d0eb69
Author: tommycli <tommycli@chromium.org>
Date: Fri Jun 09 19:13:42 2017

Omnibox UI Experiments: Suggestions Dropdown width experiment on Cocoa

Implements https://codereview.chromium.org/2914163004/ on Cocoa.

Supports an experimental flag to match the Omnibox suggestion dropdown
width to Omnibox width.

BUG= 728844 

Review-Url: https://codereview.chromium.org/2916413002
Cr-Commit-Position: refs/heads/master@{#478363}

[modify] https://crrev.com/619e52d6d7e054dfffb13053af73204fa6d0eb69/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0770bb9b89c76b8923872054c3195b9a0aaa572

commit a0770bb9b89c76b8923872054c3195b9a0aaa572
Author: tommycli <tommycli@chromium.org>
Date: Mon Jun 12 15:46:02 2017

Omnibox UI Experiments: Simple border for narrow mode

Makes the narrow Omnibox follow the same paint pattern as other Chrome
bubbles -- until UI provides customizations.

BUG= 728844 

Review-Url: https://codereview.chromium.org/2923073004
Cr-Commit-Position: refs/heads/master@{#478637}

[modify] https://crrev.com/a0770bb9b89c76b8923872054c3195b9a0aaa572/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/183f3046f02992f1390d59a357de419689826c2e

commit 183f3046f02992f1390d59a357de419689826c2e
Author: tommycli <tommycli@chromium.org>
Date: Tue Jun 20 22:15:39 2017

Omnibox UI Experiments: Add border and shadow to narrow dropdown in Mac.

Adds the border and shadow to the narrow-mode dropdown in Mac.

This does not implement the rounded corners yet, as that will be a lot
more work. That's still forthcoming.

BUG= 728844 

Review-Url: https://codereview.chromium.org/2934873002
Cr-Commit-Position: refs/heads/master@{#480989}

[modify] https://crrev.com/183f3046f02992f1390d59a357de419689826c2e/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm

Sorry to nag... but is this still being worked on? It's relevant for bugs like  Issue 778474 . I'm trying to put this feature into one of

 a) Being worked on - Try not to break it.
 b) Ignore it for now - OK to break.
 c) Never gonna launch - Actively remove it.

Thanks!

This is still on our roadmap to experiment with but is not being actively worked on at the moment. (And likely not for another month or possibly more.)

So the answer is a/b. I'd prefer to avoid breaking this, but it's ok if the breakage doesn't put us in some sort of dead end, code-wise. And if you could document any known breakages here, that'd be appreciated.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/04d4e6f3eefac2529a333fb3f370f852739ec16f

commit 04d4e6f3eefac2529a333fb3f370f852739ec16f
Author: Trent Apted <tapted@chromium.org>
Date: Tue Jan 30 23:37:10 2018

Consolidate calls to kUIExperimentNarrowDropdown into omnibox_popup_contents_view.cc

features::kTouchableChrome gets more creative, but the narrow dropdown
is a good starting point. That is, features::kTouchableChrome may imply
a narrow dropdown regardless of kUIExperimentNarrowDropdown.

Consolidate the logic under c/b/ui/views before it gets more complex.
See linked bugs for context and links the the big picture CL.

Bug: 801583,  728844 
Change-Id: I0b6ca6f8f9c6cded9ee2b1ccf0e2b8b34e737cd9
Reviewed-on: https://chromium-review.googlesource.com/892603
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533084}
[modify] https://crrev.com/04d4e6f3eefac2529a333fb3f370f852739ec16f/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/04d4e6f3eefac2529a333fb3f370f852739ec16f/chrome/browser/ui/views/location_bar/location_bar_view.h
[modify] https://crrev.com/04d4e6f3eefac2529a333fb3f370f852739ec16f/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc
[modify] https://crrev.com/04d4e6f3eefac2529a333fb3f370f852739ec16f/chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.h

We are implementing this as part of the Material Refresh work.