New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 766369 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

[Chrome Home] Remove padding from omnibox results container

Project Member Reported by k...@chromium.org, Sep 18 2017

Issue description

Per implementation review, we should remove some of the padding from the omnibox results container (below the omnibox above the suggestions).

Assigning to hannah to clarify exactly what the padding should be.
 
Thanks! Redlines are here:
https://folio.googleplex.com/chrome-home-redlines-m62#%3Ff=hidden
but the suggestion row (56dp) should be 8dp from the omnibox. If you provide the height of the header (specs included the 16dp gap at the top so I'm not sure where the final build landed) I can provide more specific specs here. (Also let me know if you'd prefer to know the distance from the suggestion text to the header) Thanks!
Programatically we define the space between the bottom of the toolbar and suggestions, rather than between the omnibox and suggestions (as the red-lines show).

This is what the elements look like programatically:
 ____________________________________
|                                    |    }
| (  url bar                )  [ ] : |    }--> toolbar   
|____________________________________|    }
|                                    |
|          SUGGESTIONS               |


The current toolbar height is 56dp, rather than the 64dp (?) it will be once we implement adding extra space. I wouldn't expect the toolbar height to affect the padding above suggestions, but maybe it does? It'd be helpful if the mocks had the entire toolbar as a selectable unit. 
Cc: danielpark@chromium.org
Your askii wireframe is #amaze
I've attached redlines for the toolbar (with the added space on top / 64dp header). But regardless of header size, both basically use 0dp padding above the suggestion row just for the omnibox focused state since the grey omnibox is shorter/has no drop shadow that requires space.
Screen Shot 2017-09-25 at 16.19.06.png
55.0 KB View Download
Cc: hannahs@chromium.org
Owner: danielpark@chromium.org
Thanks Hannah! This is currently scoped for M64, is that correct?

Daniel, will you please take a look at this?
ack.
I was under the impression it was for M63 (Was feedback from implementation review for M62)- do you think this can make it in?
Cc: k...@chromium.org
+ktam@ for scoping refinement

This should be pretty easy if we want to pull it into 63.

Comment 9 by k...@chromium.org, Sep 29 2017

Labels: -Fine-Pri-3.0 Fine-Pri-2.8
Sure let's move into 63 if it's not too bad. I've been noticing this as well.
Owner: huayinz@chromium.org
huayinz@, will you please look at this for 63?
Screenshot
suggestions_list_top_padding.png
200 KB View Download
Perfect! Thanks so much for getting this change in last minute!!!! Really appreciate it :)
Cc: -danielpark@chromium.org
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 3 2017

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

commit b6dbdeb236c356fdf1b4af6f3cea1f517b6ffcbd
Author: Becky Zhou <huayinz@chromium.org>
Date: Tue Oct 03 17:30:12 2017

[Chrome Home] Remove top padding from omnibox suggestions list

Bug:  766369 
Change-Id: I322e6ca1ce8e609753d08cf48a29f7324bd36bd1
Reviewed-on: https://chromium-review.googlesource.com/695802
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Becky Zhou <huayinz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506087}
[modify] https://crrev.com/b6dbdeb236c356fdf1b4af6f3cea1f517b6ffcbd/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java

Status: Fixed (was: Assigned)

Sign in to add a comment