New issue
Advanced search Search tips

Issue 820273 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Sep 19
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Task



Sign in to add a comment

[Android] Consolidate LocationBarLayout/Tablet/Phone

Project Member Reported by thildebr@chromium.org, Mar 8 2018

Issue description

There are mild differences in the layouts, particularly with regards to the URL bar margin calculation and the action buttons container, that we should attempt to bring together for simplicity's sake and to reduce duplication.

 
Labels: -Type-Bug OS-Android Type-Task
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 5 2018

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

commit cb5a488e3ec42b95485d31df18462a308bea9017
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Apr 05 22:03:44 2018

Bring the URL action container to LocationBarTablet.

On tablets, the layout for the location bar is overridden by the XML
in layout-sw600dp, which means instead of the url_action_container used
elsewhere, it's a series of FrameLayouts for action buttons with
manually calculated margins.

This CL breaks the url_action_container into its own file to be included
on both phone and tablet, and gets rid of the tablet-specific layout
update for calculating the margin of the action buttons.

Bug:  820273 
Change-Id: I104551325f49dbee0931e9a55c105d61e55f7796
Reviewed-on: https://chromium-review.googlesource.com/971404
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548580}
[modify] https://crrev.com/cb5a488e3ec42b95485d31df18462a308bea9017/chrome/android/java/res/layout-sw600dp/location_bar.xml
[modify] https://crrev.com/cb5a488e3ec42b95485d31df18462a308bea9017/chrome/android/java/res/layout/location_bar_base.xml
[add] https://crrev.com/cb5a488e3ec42b95485d31df18462a308bea9017/chrome/android/java/res/layout/url_action_container.xml
[modify] https://crrev.com/cb5a488e3ec42b95485d31df18462a308bea9017/chrome/android/java/res/values-v17/styles.xml
[modify] https://crrev.com/cb5a488e3ec42b95485d31df18462a308bea9017/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarTablet.java

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 5 2018

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

commit b343e31622571044e0e25eb9bf809302d021925f
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Thu Apr 05 22:29:45 2018

Revert "Bring the URL action container to LocationBarTablet."

This reverts commit cb5a488e3ec42b95485d31df18462a308bea9017.

Reason for revert: Forgot to remove the #getUrlContainerViewsForMargin to avoid overlap with the action container. Will create another patch that fixes this.

Original change's description:
> Bring the URL action container to LocationBarTablet.
> 
> On tablets, the layout for the location bar is overridden by the XML
> in layout-sw600dp, which means instead of the url_action_container used
> elsewhere, it's a series of FrameLayouts for action buttons with
> manually calculated margins.
> 
> This CL breaks the url_action_container into its own file to be included
> on both phone and tablet, and gets rid of the tablet-specific layout
> update for calculating the margin of the action buttons.
> 
> Bug:  820273 
> Change-Id: I104551325f49dbee0931e9a55c105d61e55f7796
> Reviewed-on: https://chromium-review.googlesource.com/971404
> Reviewed-by: Matthew Jones <mdjones@chromium.org>
> Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
> Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#548580}

TBR=yusufo@chromium.org,mdjones@chromium.org,thildebr@chromium.org

Change-Id: I274354f53cfa71a235abbb475acaec88580dbfba
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  820273 
Reviewed-on: https://chromium-review.googlesource.com/998593
Reviewed-by: Troy Hildebrandt <thildebr@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548588}
[modify] https://crrev.com/b343e31622571044e0e25eb9bf809302d021925f/chrome/android/java/res/layout-sw600dp/location_bar.xml
[modify] https://crrev.com/b343e31622571044e0e25eb9bf809302d021925f/chrome/android/java/res/layout/location_bar_base.xml
[delete] https://crrev.com/f096fde9eee15748f7031fa0de9e977f0cb9c026/chrome/android/java/res/layout/url_action_container.xml
[modify] https://crrev.com/b343e31622571044e0e25eb9bf809302d021925f/chrome/android/java/res/values-v17/styles.xml
[modify] https://crrev.com/b343e31622571044e0e25eb9bf809302d021925f/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarTablet.java

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 10 2018

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

commit c624f11bfae9b79fd173de866a66bb355902c77c
Author: Troy Hildebrandt <thildebr@chromium.org>
Date: Tue Apr 10 20:16:03 2018

Reland "Bring the URL action container to LocationBarTablet."

This fix removes the specialized LocationBarTablet call to
getUrlContainerViewsForMargin since the tablet doesn't need its own
version when using the action container.

Original Description:

On tablets, the layout for the location bar is overridden by the XML
in layout-sw600dp, which means instead of the url_action_container used
elsewhere, it's a series of FrameLayouts for action buttons with
manually calculated margins.

This CL breaks the url_action_container into its own file to be included
on both phone and tablet, and gets rid of the tablet-specific layout
update for calculating the margin of the action buttons.

Bug:  820273 
Change-Id: I84fe2a5ba1c8438bc287142bdb52d3e7ce42efa3
Reviewed-on: https://chromium-review.googlesource.com/999026
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549625}
[modify] https://crrev.com/c624f11bfae9b79fd173de866a66bb355902c77c/chrome/android/java/res/layout-sw600dp/location_bar.xml
[modify] https://crrev.com/c624f11bfae9b79fd173de866a66bb355902c77c/chrome/android/java/res/layout/location_bar_base.xml
[add] https://crrev.com/c624f11bfae9b79fd173de866a66bb355902c77c/chrome/android/java/res/layout/url_action_container.xml
[modify] https://crrev.com/c624f11bfae9b79fd173de866a66bb355902c77c/chrome/android/java/res/values-v17/styles.xml
[modify] https://crrev.com/c624f11bfae9b79fd173de866a66bb355902c77c/chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarTablet.java

Status: WontFix (was: Assigned)

Sign in to add a comment