Bug 1072464 - Implement new tablet forward button dimensions and clean up animation code. r=lucasr
authorMichael Comella <michael.l.comella@gmail.com>
Fri, 17 Oct 2014 16:34:56 -0700
changeset 235536 1c6a933fb75cb3f3e733166f1fce460772d09106
parent 235535 73da2cbb36a55970e77dd8aa7c9367304000cc39
child 235537 cfb35f362bb907e4e8a525505d30fa8533f829b4
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslucasr
bugs1072464
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1072464 - Implement new tablet forward button dimensions and clean up animation code. r=lucasr
mobile/android/base/newtablet/res/layout-large-v11/new_tablet_browser_toolbar.xml
mobile/android/base/resources/values-large-v11/dimens.xml
mobile/android/base/resources/values-large-v11/styles.xml
mobile/android/base/resources/values/dimens.xml
mobile/android/base/toolbar/BrowserToolbarNewTablet.java
mobile/android/base/toolbar/ToolbarDisplayLayout.java
--- a/mobile/android/base/newtablet/res/layout-large-v11/new_tablet_browser_toolbar.xml
+++ b/mobile/android/base/newtablet/res/layout-large-v11/new_tablet_browser_toolbar.xml
@@ -6,50 +6,49 @@
 <merge xmlns:android="http://schemas.android.com/apk/res/android"
        xmlns:gecko="http://schemas.android.com/apk/res-auto">
 
     <ImageView android:id="@+id/url_bar_entry"
                android:layout_width="match_parent"
                android:layout_height="match_parent"
                android:layout_alignLeft="@+id/back"
                android:layout_toLeftOf="@id/menu_items"
-               android:layout_marginLeft="@dimen/back_button_width_half"
+               android:layout_marginLeft="@dimen/new_tablet_nav_button_width_half"
                android:layout_marginTop="10dp"
                android:layout_marginBottom="10dp"
                android:duplicateParentState="true"
                android:clickable="false"
                android:focusable="false"
                android:background="@drawable/url_bar_entry"/>
 
     <org.mozilla.gecko.toolbar.ForwardButton style="@style/UrlBar.ImageButton.Forward.NewTablet"
                                              android:id="@+id/forward"
                                              android:layout_alignLeft="@id/back"/>
 
     <org.mozilla.gecko.toolbar.BackButton android:id="@id/back"
                                           style="@style/UrlBar.ImageButton.NewTablet"
-                                          android:layout_width="@dimen/back_button_width"
-                                          android:layout_height="@dimen/back_button_width"
+                                          android:layout_width="@dimen/new_tablet_nav_button_width"
+                                          android:layout_height="@dimen/new_tablet_nav_button_width"
                                           android:layout_centerVertical="true"
                                           android:layout_marginLeft="12dp"
                                           android:layout_alignParentLeft="true"
                                           android:src="@drawable/new_tablet_ic_menu_back"
                                           android:contentDescription="@string/back"
                                           android:background="@drawable/new_tablet_url_bar_nav_button"/>
 
     <org.mozilla.gecko.toolbar.ToolbarEditLayout android:id="@+id/edit_layout"
                   style="@style/UrlBar.Button"
                   android:paddingLeft="12dp"
                   android:paddingRight="12dp"
                   android:visibility="gone"
                   android:orientation="horizontal"
                   android:layout_toRightOf="@id/back"
                   android:layout_toLeftOf="@id/menu_items"/>
 
-    <!-- Note: * Values of marginLeft are used to animate the forward button so don't change its value.
-               * We set the padding on the site security icon to increase its tappable area. -->
+    <!-- Note: we set the padding on the site security icon to increase its tappable area. -->
     <org.mozilla.gecko.toolbar.ToolbarDisplayLayout android:id="@+id/display_layout"
                   style="@style/UrlBar.Button.Container"
                   android:layout_toRightOf="@id/back"
                   android:layout_toLeftOf="@id/menu_items"
                   android:paddingRight="4dip"/>
 
     <LinearLayout android:id="@+id/menu_items"
                   android:layout_width="wrap_content"
--- a/mobile/android/base/resources/values-large-v11/dimens.xml
+++ b/mobile/android/base/resources/values-large-v11/dimens.xml
@@ -3,16 +3,12 @@
    - License, v. 2.0. If a copy of the MPL was not distributed with this
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 
 <resources>
 
     <dimen name="browser_toolbar_height">56dp</dimen>
     <dimen name="browser_toolbar_button_padding">16dp</dimen>
 
-    <!-- If you update one of these values, update the other. -->
-    <dimen name="back_button_width">42dp</dimen>
-    <dimen name="back_button_width_half">21dp</dimen>
-
     <dimen name="tabs_counter_size">26sp</dimen>
     <dimen name="panel_grid_view_column_width">200dp</dimen>
 
 </resources>
--- a/mobile/android/base/resources/values-large-v11/styles.xml
+++ b/mobile/android/base/resources/values-large-v11/styles.xml
@@ -20,25 +20,40 @@
         <item name="android:layout_centerVertical">true</item>
         <item name="android:src">@drawable/ic_menu_forward</item>
         <item name="android:background">@drawable/url_bar_nav_button</item>
         <!-- Start with the button hidden -->
         <item name="android:alpha">0</item>
         <item name="android:layout_marginLeft">@dimen/forward_default_offset</item>
     </style>
 
+    <!-- Note: this style is for the visible and expanded forward button.  We translate/hide
+         the forward button in code - see BrowserToolbarNewTablet.animateForwardButton. -->
     <style name="UrlBar.ImageButton.Forward.NewTablet">
-        <item name="android:layout_marginLeft">@dimen/new_tablet_forward_default_offset</item>
         <item name="android:layout_height">match_parent</item>
         <item name="android:paddingTop">0dp</item>
         <item name="android:paddingBottom">0dp</item>
         <item name="android:layout_marginTop">11.5dp</item>
         <item name="android:layout_marginBottom">11.5dp</item>
         <item name="android:src">@drawable/new_tablet_ic_menu_forward</item>
         <item name="android:background">@drawable/new_tablet_url_bar_nav_button</item>
+
+        <!-- The visible area of the forward button is a nav_button_width and the
+             non-visible area slides halfway under the back button. This non-visible
+             area is used to ensure the forward button background fully
+             covers the space to the right of the back button. -->
+        <item name="android:layout_width">@dimen/new_tablet_nav_button_width_plus_half</item>
+
+        <!-- (See note above) We left align with back,
+             but only need to hide halfway underneath. -->
+        <item name="android:layout_marginLeft">@dimen/new_tablet_nav_button_width_half</item>
+
+        <!-- We use left padding to center the arrow in the
+             visible area as opposed to the true width. -->
+        <item name="android:paddingLeft">18dp</item>
     </style>
 
     <style name="UrlBar.ImageButton.TabCount.NewTablet">
         <item name="android:background">@drawable/new_tablet_tabs_count</item>
 
         <!-- From UrlBar.ImageButton.NewTablet because we can't inherit directly. -->
         <item name="android:layout_width">@dimen/new_tablet_browser_toolbar_menu_item_width</item>
     </style>
--- a/mobile/android/base/resources/values/dimens.xml
+++ b/mobile/android/base/resources/values/dimens.xml
@@ -12,21 +12,25 @@
     <dimen name="browser_toolbar_button_padding">12dp</dimen>
     <dimen name="browser_toolbar_icon_width">48dp</dimen>
     <dimen name="browser_toolbar_site_security_width">12dp</dimen>
     <!-- favicon_size includes 4dp of right padding. We can't use margin (which would allow us to
          specify the actual size) because that would decrease the size of our hit target. -->
     <dimen name="browser_toolbar_favicon_size">21.33dip</dimen>
     <dimen name="browser_toolbar_shadow_size">2dp</dimen>
 
+    <!-- If you update one of these values, update the others. -->
+    <dimen name="new_tablet_nav_button_width">42dp</dimen>
+    <dimen name="new_tablet_nav_button_width_half">21dp</dimen>
+    <dimen name="new_tablet_nav_button_width_plus_half">63dp</dimen>
+
     <dimen name="new_tablet_tab_strip_height">48dp</dimen>
     <dimen name="new_tablet_tab_strip_item_width">250dp</dimen>
     <dimen name="new_tablet_tab_strip_item_margin">-30dp</dimen>
     <dimen name="new_tablet_tab_strip_favicon_size">16dp</dimen>
-    <dimen name="new_tablet_forward_default_offset">-6dp</dimen>
     <dimen name="new_tablet_site_security_height">60dp</dimen>
     <dimen name="new_tablet_site_security_width">34dp</dimen>
     <!-- We primarily use padding (instead of margins) to increase the hit area. -->
     <dimen name="new_tablet_site_security_padding_vertical">21dp</dimen>
     <dimen name="new_tablet_site_security_padding_horizontal">8dp</dimen>
     <dimen name="new_tablet_site_security_right_margin">1dp</dimen>
     <dimen name="new_tablet_browser_toolbar_height">60dp</dimen>
     <dimen name="new_tablet_browser_toolbar_menu_item_width">56dp</dimen>
--- a/mobile/android/base/toolbar/BrowserToolbarNewTablet.java
+++ b/mobile/android/base/toolbar/BrowserToolbarNewTablet.java
@@ -5,36 +5,38 @@
 
 package org.mozilla.gecko.toolbar;
 
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.animation.PropertyAnimator;
 import org.mozilla.gecko.animation.ViewHelper;
 
 import android.content.Context;
-import android.content.res.Resources;
 import android.util.AttributeSet;
 
 /**
  * A toolbar implementation for the tablet redesign (bug 1014156).
  * Expected to replace BrowserToolbarTablet once complete.
  */
 class BrowserToolbarNewTablet extends BrowserToolbarTabletBase {
 
     private static final int FORWARD_ANIMATION_DURATION = 450;
 
-    private final int urlBarViewOffset;
-    private final int defaultForwardMargin;
+    private final int forwardButtonTranslationWidth;
 
     public BrowserToolbarNewTablet(final Context context, final AttributeSet attrs) {
         super(context, attrs);
 
-        final Resources res = getResources();
-        urlBarViewOffset = res.getDimensionPixelSize(R.dimen.url_bar_offset_left);
-        defaultForwardMargin = res.getDimensionPixelSize(R.dimen.new_tablet_forward_default_offset);
+        forwardButtonTranslationWidth =
+                getResources().getDimensionPixelOffset(R.dimen.new_tablet_nav_button_width);
+
+        // The forward button is initially expanded (in the layout file)
+        // so translate it for start of the expansion animation; future
+        // iterations translate it to this position when hiding and will already be set up.
+        ViewHelper.setTranslationX(forwardButton, -forwardButtonTranslationWidth);
     }
 
     @Override
     public boolean isAnimating() {
         return false;
     }
 
     @Override
@@ -44,35 +46,34 @@ class BrowserToolbarNewTablet extends Br
 
     @Override
     protected void triggerStopEditingTransition() {
         hideUrlEditLayout();
     }
 
     @Override
     protected void animateForwardButton(final ForwardButtonAnimation animation) {
-        final boolean showing = (animation == ForwardButtonAnimation.SHOW);
+        final boolean willShowForward = (animation == ForwardButtonAnimation.SHOW);
 
-        // if the forward button's margin is non-zero, this means it has already
-        // been animated to be visible¬ł and vice-versa.
-        MarginLayoutParams fwdParams = (MarginLayoutParams) forwardButton.getLayoutParams();
-        if ((fwdParams.leftMargin > defaultForwardMargin && showing) ||
-            (fwdParams.leftMargin == defaultForwardMargin && !showing)) {
+        // If we're not in the appropriate state to start a particular animation,
+        // then we must be in the opposite state and do not need to animate.
+        final float forwardOffset = ViewHelper.getTranslationX(forwardButton);
+        if ((forwardOffset >= 0 && willShowForward) ||
+                forwardOffset < 0 && !willShowForward) {
             return;
         }
 
         // We want the forward button to show immediately when switching tabs
         final PropertyAnimator forwardAnim =
                 new PropertyAnimator(isSwitchingTabs ? 10 : FORWARD_ANIMATION_DURATION);
-        final int width = Math.round(forwardButton.getWidth() * .75f);
 
         forwardAnim.addPropertyAnimationListener(new PropertyAnimator.PropertyAnimationListener() {
             @Override
             public void onPropertyAnimationStart() {
-                if (!showing) {
+                if (!willShowForward) {
                     // Set the margin before the transition when hiding the forward button. We
                     // have to do this so that the favicon isn't clipped during the transition
                     MarginLayoutParams layoutParams =
                         (MarginLayoutParams) urlDisplayLayout.getLayoutParams();
                     layoutParams.leftMargin = 0;
 
                     // Do the same on the URL edit container
                     layoutParams = (MarginLayoutParams) urlEditLayout.getLayoutParams();
@@ -82,52 +83,49 @@ class BrowserToolbarNewTablet extends Br
                     // Note, we already translated the favicon, site security, and text field
                     // in prepareForwardAnimation, so they should appear to have not moved at
                     // all at this point.
                 }
             }
 
             @Override
             public void onPropertyAnimationEnd() {
-                if (showing) {
+                if (willShowForward) {
+                    // Increase the margins to ensure the text does not run outside the View.
                     MarginLayoutParams layoutParams =
                         (MarginLayoutParams) urlDisplayLayout.getLayoutParams();
-                    layoutParams.leftMargin = urlBarViewOffset;
+                    layoutParams.leftMargin = forwardButtonTranslationWidth;
 
                     layoutParams = (MarginLayoutParams) urlEditLayout.getLayoutParams();
-                    layoutParams.leftMargin = urlBarViewOffset;
+                    layoutParams.leftMargin = forwardButtonTranslationWidth;
                 }
 
                 urlDisplayLayout.finishForwardAnimation();
 
-                MarginLayoutParams layoutParams = (MarginLayoutParams) forwardButton.getLayoutParams();
-                layoutParams.leftMargin = defaultForwardMargin + (showing ? width : 0);
-                ViewHelper.setTranslationX(forwardButton, 0);
-
                 requestLayout();
             }
         });
 
-        prepareForwardAnimation(forwardAnim, animation, width);
+        prepareForwardAnimation(forwardAnim, animation, forwardButtonTranslationWidth);
         forwardAnim.start();
     }
 
     private void prepareForwardAnimation(PropertyAnimator anim, ForwardButtonAnimation animation, int width) {
         if (animation == ForwardButtonAnimation.HIDE) {
             anim.attach(forwardButton,
                       PropertyAnimator.Property.TRANSLATION_X,
                       -width);
             anim.attach(forwardButton,
                       PropertyAnimator.Property.ALPHA,
                       0);
 
         } else {
             anim.attach(forwardButton,
                       PropertyAnimator.Property.TRANSLATION_X,
-                      width);
+                      0);
             anim.attach(forwardButton,
                       PropertyAnimator.Property.ALPHA,
                       1);
         }
 
         urlDisplayLayout.prepareForwardAnimation(anim, animation, width);
     }
 
--- a/mobile/android/base/toolbar/ToolbarDisplayLayout.java
+++ b/mobile/android/base/toolbar/ToolbarDisplayLayout.java
@@ -530,16 +530,18 @@ public class ToolbarDisplayLayout extend
             return mSiteSecurity;
         }
     }
 
     void prepareForwardAnimation(PropertyAnimator anim, ForwardButtonAnimation animation, int width) {
         mForwardAnim = anim;
 
         if (animation == ForwardButtonAnimation.HIDE) {
+            // We animate these items individually, rather than this entire view,
+            // so that we don't animate certain views, e.g. the stop button.
             anim.attach(mTitle,
                         PropertyAnimator.Property.TRANSLATION_X,
                         0);
             anim.attach(mFavicon,
                         PropertyAnimator.Property.TRANSLATION_X,
                         0);
             anim.attach(mSiteSecurity,
                         PropertyAnimator.Property.TRANSLATION_X,