Rounded corners are showing on focused omnibox |
||||||||||||||||
Issue descriptionSee https://crbug.com/605054#c4 . We should expand the omnibox further horizontally to push the corners out of view.
,
Jun 17 2016
,
Jun 27 2016
,
Jun 29 2016
,
Jul 1 2016
,
Jul 1 2016
,
Jul 1 2016
,
Jul 1 2016
,
Jul 1 2016
,
Jul 8 2016
,
Jul 8 2016
,
Jul 18 2016
,
Jul 22 2016
,
Jul 29 2016
,
Jul 29 2016
,
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
,
Jul 31 2016
,
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
,
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 |
||||||||||||||||
Comment 1 by bauerb@chromium.org
, Jun 15 2016