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

Issue 618955 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Not on Chrome anymore
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Rounded corners are showing on focused omnibox

Project Member Reported by bauerb@chromium.org, Jun 10 2016

Issue description

See  https://crbug.com/605054#c4 .

We should expand the omnibox further horizontally to push the corners out of view.
 

Comment 1 by bauerb@chromium.org, Jun 15 2016

Labels: zine-mr-iter-19

Comment 2 by bauerb@chromium.org, Jun 17 2016

Labels: zine-mr-iter-20

Comment 3 by bauerb@chromium.org, Jun 27 2016

Labels: zine-mr-iter-21

Comment 4 by bauerb@chromium.org, Jun 29 2016

Labels: -zine-pe zine-ntp-pe

Comment 5 by fi...@chromium.org, Jul 1 2016

Labels: zine-mr-MVP

Comment 6 by fi...@chromium.org, Jul 1 2016

Labels: zine-16-06-27
Labels: Hotlist-Fixit-PE2016

Comment 8 by finkm@google.com, Jul 1 2016

Labels: -zine-mr-mvp

Comment 9 by fi...@chromium.org, Jul 1 2016

Labels: zine-mr-MVP
Labels: -zine-mr-mile-mvp -zine-mr-mvp zine-client-v1
Labels: zine-16-07-11
Labels: zine-16-07-18
Labels: zine-16-07-25

Comment 14 by fi...@chromium.org, Jul 29 2016

Labels: zine-triaged

Comment 15 by fi...@chromium.org, Jul 29 2016

Labels: -zine-client-v1
Project Member

Comment 16 by bugdroid1@chromium.org, Jul 31 2016

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

commit 7a676cfc5291809bf2a95fd16ba1d57907f7961a
Author: bauerb <bauerb@chromium.org>
Date: Sun Jul 31 15:23:11 2016

Use only toolbar to transition from fakebox to real omnibox.

Previously, we would fade out the fakebox and fade in the real omnibox during
the scroll transition, which created discrepancies. With this change, the
toolbar omnibox does the full transition. The search box on the NTP is still
kept, both to get the starting point of the transition, and for the tablet
toolbar, where the omnibox stays at the top, so there are two boxes.

Because the two boxes now have to match, their dimensions and padding are
slightly updated. The height is 56dp, and the padding on the sides is 12dp.

Also, in the focused state the omnibox now has 1dp bleed at the sides,
which moves the rounded corners out of view.

The remaining changes are not user-visible:
* Opacity of the fakebox is now driven by the toolbar. This lets us centralize
  the logic (the toolbar can decide its own opacity and the opacity of the
  fakebox) and use different behavior on tablets, where the fakebox will simply
  fade out as before.
* Rename variables to (hopefully) make them more consistent / self-explanatory.
* |mLocationBarBackgroundBounds| (formerly |mUrlViewportBounds|) more accurately
  reflects the visible omnibox bounds in the absence of the NTP / when the NTP
  is scrolled so that the omnibox is at the top. To do that, we no longer
  include the Y translation we apply to position it where the fakebox is (it's
  instead applied to the |mLocationBarBackgroundNtpOffset|), and we always use
  an expansion value of 1 if the current tab is an NTP.
* Remove the |inset_textbox| drawable -- it was the same as |textbox|, but with
  a margin added at the top and bottom (but *not* left or right). Since we
  calculate the omnibox bounds manually anyway, this allows us to account for
  the omnibox padding in the same way in either dimension.
* Remove the |isInTabSwitcherMode| parameter from some methods in favor of using
  the member variable.

BUG= 605054 , 625108 , 618955 ,616728,612520

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

[delete] https://crrev.com/78c093f7cae68751ea9321f2d5c526cfdea62a5a/chrome/android/java/res/drawable/inset_textbox.xml
[modify] https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a/chrome/android/java/res/layout/new_tab_page_layout.xml
[modify] https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a/chrome/android/java/res/values/dimens.xml
[modify] https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[modify] https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarPhone.java
[modify] https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarPhone.java
[modify] https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarTablet.java

Status: Fixed (was: Assigned)
Project Member

Comment 18 by bugdroid1@chromium.org, Aug 1 2016

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

commit ed720fdd749d46e0934267bd8f935bbeb1ec0991
Author: gab <gab@chromium.org>
Date: Mon Aug 01 20:35:41 2016

Revert of Use only toolbar to transition from fakebox to real omnibox. (patchset #11 id:200001 of https://codereview.chromium.org/2134663002/ )

Reason for revert:
Suspect for  http://crbug.com/633321 , will re-land if not.

Original issue's description:
> Use only toolbar to transition from fakebox to real omnibox.
>
> Previously, we would fade out the fakebox and fade in the real omnibox during
> the scroll transition, which created discrepancies. With this change, the
> toolbar omnibox does the full transition. The search box on the NTP is still
> kept, both to get the starting point of the transition, and for the tablet
> toolbar, where the omnibox stays at the top, so there are two boxes.
>
> Because the two boxes now have to match, their dimensions and padding are
> slightly updated. The height is 56dp, and the padding on the sides is 12dp.
>
> Also, in the focused state the omnibox now has 1dp bleed at the sides,
> which moves the rounded corners out of view.
>
> The remaining changes are not user-visible:
> * Opacity of the fakebox is now driven by the toolbar. This lets us centralize
>   the logic (the toolbar can decide its own opacity and the opacity of the
>   fakebox) and use different behavior on tablets, where the fakebox will simply
>   fade out as before.
> * Rename variables to (hopefully) make them more consistent / self-explanatory.
> * |mLocationBarBackgroundBounds| (formerly |mUrlViewportBounds|) more accurately
>   reflects the visible omnibox bounds in the absence of the NTP / when the NTP
>   is scrolled so that the omnibox is at the top. To do that, we no longer
>   include the Y translation we apply to position it where the fakebox is (it's
>   instead applied to the |mLocationBarBackgroundNtpOffset|), and we always use
>   an expansion value of 1 if the current tab is an NTP.
> * Remove the |inset_textbox| drawable -- it was the same as |textbox|, but with
>   a margin added at the top and bottom (but *not* left or right). Since we
>   calculate the omnibox bounds manually anyway, this allows us to account for
>   the omnibox padding in the same way in either dimension.
> * Remove the |isInTabSwitcherMode| parameter from some methods in favor of using
>   the member variable.
>
> BUG= 605054 , 625108 , 618955 ,616728,612520
>
> Committed: https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a
> Cr-Commit-Position: refs/heads/master@{#408885}

TBR=peconn@chromium.org,mcwilliams@chromium.org,mdjones@chromium.org,tedchoc@chromium.org,bauerb@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 605054 , 625108 , 618955 ,616728,612520

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

[add] https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991/chrome/android/java/res/drawable/inset_textbox.xml
[modify] https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991/chrome/android/java/res/layout/new_tab_page_layout.xml
[modify] https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991/chrome/android/java/res/values/dimens.xml
[modify] https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[modify] https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarPhone.java
[modify] https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarPhone.java
[modify] https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarTablet.java

Project Member

Comment 19 by bugdroid1@chromium.org, Aug 1 2016

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

commit 62c1818efd6793549a21f744746a94b1a55dd2c1
Author: mathp <mathp@chromium.org>
Date: Mon Aug 01 21:41:23 2016

Reland of Use only toolbar to transition from fakebox to real omnibox. (patchset #1 id:1 of https://codereview.chromium.org/2198993002/ )

Reason for revert:
Was not the culprit for webkit_unit_tests failing.. relanding

Original issue's description:
> Revert of Use only toolbar to transition from fakebox to real omnibox. (patchset #11 id:200001 of https://codereview.chromium.org/2134663002/ )
>
> Reason for revert:
> Suspect for  http://crbug.com/633321 , will re-land if not.
>
> Original issue's description:
> > Use only toolbar to transition from fakebox to real omnibox.
> >
> > Previously, we would fade out the fakebox and fade in the real omnibox during
> > the scroll transition, which created discrepancies. With this change, the
> > toolbar omnibox does the full transition. The search box on the NTP is still
> > kept, both to get the starting point of the transition, and for the tablet
> > toolbar, where the omnibox stays at the top, so there are two boxes.
> >
> > Because the two boxes now have to match, their dimensions and padding are
> > slightly updated. The height is 56dp, and the padding on the sides is 12dp.
> >
> > Also, in the focused state the omnibox now has 1dp bleed at the sides,
> > which moves the rounded corners out of view.
> >
> > The remaining changes are not user-visible:
> > * Opacity of the fakebox is now driven by the toolbar. This lets us centralize
> >   the logic (the toolbar can decide its own opacity and the opacity of the
> >   fakebox) and use different behavior on tablets, where the fakebox will simply
> >   fade out as before.
> > * Rename variables to (hopefully) make them more consistent / self-explanatory.
> > * |mLocationBarBackgroundBounds| (formerly |mUrlViewportBounds|) more accurately
> >   reflects the visible omnibox bounds in the absence of the NTP / when the NTP
> >   is scrolled so that the omnibox is at the top. To do that, we no longer
> >   include the Y translation we apply to position it where the fakebox is (it's
> >   instead applied to the |mLocationBarBackgroundNtpOffset|), and we always use
> >   an expansion value of 1 if the current tab is an NTP.
> > * Remove the |inset_textbox| drawable -- it was the same as |textbox|, but with
> >   a margin added at the top and bottom (but *not* left or right). Since we
> >   calculate the omnibox bounds manually anyway, this allows us to account for
> >   the omnibox padding in the same way in either dimension.
> > * Remove the |isInTabSwitcherMode| parameter from some methods in favor of using
> >   the member variable.
> >
> > BUG= 605054 , 625108 , 618955 ,616728,612520
> >
> > Committed: https://crrev.com/7a676cfc5291809bf2a95fd16ba1d57907f7961a
> > Cr-Commit-Position: refs/heads/master@{#408885}
>
> TBR=peconn@chromium.org,mcwilliams@chromium.org,mdjones@chromium.org,tedchoc@chromium.org,bauerb@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= 605054 , 625108 , 618955 ,616728,612520
>
> Committed: https://crrev.com/ed720fdd749d46e0934267bd8f935bbeb1ec0991
> Cr-Commit-Position: refs/heads/master@{#409045}

TBR=peconn@chromium.org,mcwilliams@chromium.org,mdjones@chromium.org,tedchoc@chromium.org,bauerb@chromium.org,gab@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 605054 , 625108 , 618955 ,616728,612520

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

[delete] https://crrev.com/bd828670eaef26966373521fa23624d5b7b3ddd6/chrome/android/java/res/drawable/inset_textbox.xml
[modify] https://crrev.com/62c1818efd6793549a21f744746a94b1a55dd2c1/chrome/android/java/res/layout/new_tab_page_layout.xml
[modify] https://crrev.com/62c1818efd6793549a21f744746a94b1a55dd2c1/chrome/android/java/res/values/dimens.xml
[modify] https://crrev.com/62c1818efd6793549a21f744746a94b1a55dd2c1/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java
[modify] https://crrev.com/62c1818efd6793549a21f744746a94b1a55dd2c1/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
[modify] https://crrev.com/62c1818efd6793549a21f744746a94b1a55dd2c1/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
[modify] https://crrev.com/62c1818efd6793549a21f744746a94b1a55dd2c1/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarPhone.java
[modify] https://crrev.com/62c1818efd6793549a21f744746a94b1a55dd2c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarPhone.java
[modify] https://crrev.com/62c1818efd6793549a21f744746a94b1a55dd2c1/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarTablet.java

Sign in to add a comment