Bug 1312477 - Post: remove arbitrary peek and max heights from BottomSheet menu r?sebastian draft
authorAndrzej Hunt <ahunt@mozilla.com>
Wed, 02 Nov 2016 21:13:19 +0100
changeset 434815 5d57a8220958517fd5860d8c261e68aaaf51a433
parent 434814 36f95a615362d047c6ef9396f0886912f53cf7d8
child 434816 4a0cb41f6fe590d20ca1b7d4d9667e1dbff5b1d5
push id34834
push userahunt@mozilla.com
push dateMon, 07 Nov 2016 12:22:00 +0000
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>