Bug 1312477 - Post: remove arbitrary peek and max heights from BottomSheet menu r=sebastian
authorAndrzej Hunt <ahunt@mozilla.com>
Wed, 02 Nov 2016 21:13:19 +0100
changeset 351593 ee67814f9015f33e893caefe9c8bad0b4daa46f4
parent 351592 ffdf65db6dba6bbc5eb3e62cdc869746371fdca6
child 351594 8e7eaf50e29406ba4dd5650bccad7b44751f63cc
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1312477
milestone52.0a1
Bug 1312477 - Post: remove arbitrary peek and max heights from BottomSheet menu r=sebastian Setting the max height is completely arbitrary, and largely unnecessary - I think it's better to let the bottom automatically handle this until we know more about how the bottom sheet is perceived by real users. The peek height is similarly arbitrary - we'll probably want to discuss a good default height algorithm with UX, but setting an arbitrary hardcoded value seems wrong, especially since it doesn't behave well in landscape mode. BottomSheetDialog already does an acceptable job of finding a default peek height. MozReview-Commit-ID: LyAYzcytR6H
mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/ActivityStreamContextMenu.java
mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/BottomSheetContextMenu.java
mobile/android/base/resources/layout/activity_stream_contextmenu_bottomsheet.xml
mobile/android/base/resources/values/dimens.xml
--- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/ActivityStreamContextMenu.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/ActivityStreamContextMenu.java
@@ -63,17 +63,17 @@ public abstract class ActivityStreamCont
 
         this.mode = mode;
 
         this.title = title;
         this.url = url;
         this.onUrlOpenListener = onUrlOpenListener;
         this.onUrlOpenInBackgroundListener = onUrlOpenInBackgroundListener;
     }
-    
+
     /**
      * Must be called before the menu is shown.
      * <p/>
      * Your implementation must be ready to return items from getItemByID() before postInit() is
      * called, i.e. you should probably inflate your menu items before this call.
      */
     protected void postInit() {
         // Disable "dismiss" for topsites until we have decided on its behaviour for topsites
--- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/BottomSheetContextMenu.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/BottomSheetContextMenu.java
@@ -77,19 +77,16 @@ import static org.mozilla.gecko.activity
                     public void onIconResponse(IconResponse response) {
                         faviconView.updateImage(response);
                     }
                 });
 
         navigationView = (NavigationView) content.findViewById(R.id.menu);
         navigationView.setNavigationItemSelectedListener(this);
 
-        BottomSheetBehavior<View> bsBehaviour = BottomSheetBehavior.from((View) content.getParent());
-        bsBehaviour.setPeekHeight(context.getResources().getDimensionPixelSize(R.dimen.activity_stream_contextmenu_peek_height));
-
         super.postInit();
     }
 
     @Override
     public MenuItem getItemByID(int id) {
         return navigationView.getMenu().findItem(id);
     }
 
--- a/mobile/android/base/resources/layout/activity_stream_contextmenu_bottomsheet.xml
+++ b/mobile/android/base/resources/layout/activity_stream_contextmenu_bottomsheet.xml
@@ -55,17 +55,17 @@
         android:layout_width="match_parent"
         android:layout_height="0.5dp"
         android:layout_marginTop="4dp"
         android:background="@color/disabled_grey"
         android:padding="4dp"/>
 
     <android.support.v4.widget.NestedScrollView
         android:layout_width="match_parent"
-        android:layout_height="@dimen/activity_stream_contextmenu_max_menu_height">
+        android:layout_height="wrap_content">
 
         <android.support.design.widget.NavigationView
             xmlns:android="http://schemas.android.com/apk/res/android"
             xmlns:app="http://schemas.android.com/apk/res-auto"
             android:id="@+id/menu"
             android:layout_width="match_parent"
             android:layout_height="wrap_content"
             app:itemTextAppearance="@style/ActivityStreamContextMenuText"
--- a/mobile/android/base/resources/values/dimens.xml
+++ b/mobile/android/base/resources/values/dimens.xml
@@ -217,16 +217,11 @@
 
     <item name="notification_media_cover" type="dimen">128dp</item>
 
     <item name="activity_stream_base_margin" type="dimen">10dp</item>
     <item name="activity_stream_desired_tile_width" type="dimen">90dp</item>
     <item name="activity_stream_desired_tile_height" type="dimen">70dp</item>
     <item name="activity_stream_top_sites_text_height" type="dimen">30dp</item>
 
-    <item name="activity_stream_contextmenu_peek_height" type="dimen">380dp</item>
-    <!-- note: max_menu_height only affects the scrolling menu, but doesnt' take into consideration
-         the header above it. -->
-    <item name="activity_stream_contextmenu_max_menu_height" type="dimen">350dp</item>
-
     <!-- Default touch target size for buttons/imageviews that might be of small size -->
     <item name="touch_target_size" type="dimen">48dp</item>
 </resources>