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 432800 6f2d25574cf0c0de146bae42f5acf6b5663e5cb7
parent 432799 606582e7babca659e24e8fdd898657be6f1a9160
child 535755 eee096d2a82725e332822447ad951cfdd8920694
push id34426
push userahunt@mozilla.com
push dateWed, 02 Nov 2016 20:14:04 +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/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/BottomSheetContextMenu.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/menu/BottomSheetContextMenu.java
@@ -72,19 +72,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>