Commit 44923f19 authored by Jinsuk Kim's avatar Jinsuk Kim Committed by Commit Bot

Android: Get back scrim on navigation popup

This CL restores the missing scrim when there is a single entry on the
navigation sheet. The repro step is unique in that it's the only
situation where the sheet with a single entry "Show full history" is
displayed. All the others show at least 2.

The scrim didn't appear since BottomSheet didn't trigger the event
|onSheetOpened|. The minimum swipable state is set to
peek(https://crrev.com/c/1880495), but the sheet height with a single
entry is smaller than that of the peek state (1.5 x entry height),
therefore didn't meet the condition that fires the event.

To address the issue, following changes were made:

1) BottomSheetContent disables peek/half state for this occasion so
that the sheet can go straight to full state.

2) Skip calling |expandSheet| but used BottomSheetContent state info
to have the same effect. The explicit call to the API, when combined
with the above change, causes the same state transition (full ->
full), which cancels the animation effect.

Bug: 1020408
Change-Id: I2b9d3d2f495a167dd844be82b9796bfeeb2fda81
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1893129Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711919}
parent 1153513c
...@@ -140,7 +140,7 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet ...@@ -140,7 +140,7 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet
return mLayoutInflater.inflate(R.layout.navigation_popup_item, null); return mLayoutInflater.inflate(R.layout.navigation_popup_item, null);
}, NavigationItemViewBinder::bind); }, NavigationItemViewBinder::bind);
mOpenSheetRunnable = () -> { mOpenSheetRunnable = () -> {
if (isHidden()) openSheet(false, true); if (isHidden()) openSheet(true, true);
}; };
mLongSwipePeekThreshold = Math.min( mLongSwipePeekThreshold = Math.min(
context.getResources().getDisplayMetrics().density * LONG_SWIPE_PEEK_THRESHOLD_DP, context.getResources().getDisplayMetrics().density * LONG_SWIPE_PEEK_THRESHOLD_DP,
...@@ -156,7 +156,7 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet ...@@ -156,7 +156,7 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet
} }
// Transition to either peeked or expanded state. // Transition to either peeked or expanded state.
private boolean openSheet(boolean fullyExpand, boolean animate) { private boolean openSheet(boolean expandIfSmall, boolean animate) {
mContentView = mContentView =
(NavigationSheetView) mLayoutInflater.inflate(R.layout.navigation_sheet, null); (NavigationSheetView) mLayoutInflater.inflate(R.layout.navigation_sheet, null);
ListView listview = (ListView) mContentView.findViewById(R.id.navigation_entries); ListView listview = (ListView) mContentView.findViewById(R.id.navigation_entries);
...@@ -170,8 +170,10 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet ...@@ -170,8 +170,10 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet
} }
mBottomSheetController.get().addObserver(mSheetObserver); mBottomSheetController.get().addObserver(mSheetObserver);
mSheetTriggered = true; mSheetTriggered = true;
mFullyExpand = fullyExpand; if (expandIfSmall && history.getEntryCount() <= SKIP_PEEK_COUNT) {
if (fullyExpand || history.getEntryCount() <= SKIP_PEEK_COUNT) expandSheet(); mFullyExpand = true;
expandSheet();
}
return true; return true;
} }
...@@ -188,6 +190,7 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet ...@@ -188,6 +190,7 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet
mForward = forward; mForward = forward;
mShowCloseIndicator = showCloseIndicator; mShowCloseIndicator = showCloseIndicator;
mSheetTriggered = false; mSheetTriggered = false;
mFullyExpand = false;
mOpenedAsPopup = false; mOpenedAsPopup = false;
} }
...@@ -195,7 +198,11 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet ...@@ -195,7 +198,11 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet
public boolean startAndExpand(boolean forward, boolean animate) { public boolean startAndExpand(boolean forward, boolean animate) {
start(forward, /* showCloseIndicator= */ false); start(forward, /* showCloseIndicator= */ false);
mOpenedAsPopup = true; mOpenedAsPopup = true;
boolean opened = openSheet(/* fullyExpand= */ true, animate);
// Enter the expanded state by disabling peek/half state rather than
// calling |expandSheet| explicilty. Otherwise it cause an extra
// state transition (full -> full), which cancels the animation effect.
boolean opened = openSheet(/* expandIfSmall */ false, animate);
if (opened) GestureNavMetrics.recordUserAction("Popup"); if (opened) GestureNavMetrics.recordUserAction("Popup");
return opened; return opened;
} }
...@@ -305,6 +312,7 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet ...@@ -305,6 +312,7 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet
@Override @Override
public int getPeekHeight() { public int getPeekHeight() {
if (mOpenedAsPopup) return BottomSheetContent.HeightMode.DISABLED;
// Makes peek state as 'not present' when bottom sheet is in expanded state (i.e. animating // Makes peek state as 'not present' when bottom sheet is in expanded state (i.e. animating
// from expanded to close state). It avoids the sheet animating in two distinct steps, which // from expanded to close state). It avoids the sheet animating in two distinct steps, which
// looks awkward. // looks awkward.
...@@ -315,8 +323,8 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet ...@@ -315,8 +323,8 @@ class NavigationSheetCoordinator implements BottomSheetContent, NavigationSheet
@Override @Override
public float getHalfHeightRatio() { public float getHalfHeightRatio() {
return mFullyExpand ? getFullHeightRatio() if (mOpenedAsPopup) return BottomSheetContent.HeightMode.DISABLED;
: getCappedHeightRatio(mParentView.getHeight() / 2 + mItemHeight / 2); return getCappedHeightRatio(mParentView.getHeight() / 2 + mItemHeight / 2);
} }
@Override @Override
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment