From fe28f9a06382b5ec0388db2179a9f8b855f9fc76 Mon Sep 17 00:00:00 2001 From: Matt Pietal Date: Fri, 22 Mar 2019 07:59:58 -0400 Subject: [PATCH] Sharesheet - Fix 4->8 direct share expansion glitch Fix the drag handle and spacing. Set the initial sharesheet size properly to allow for 2 rows + space to expand the direct share area. Bug: 128969671 Test: atest ChooserActivityTest and visual inspection Change-Id: I24f2ffd027c2edd69d90ab31c287ec773b7a7039 --- .../android/internal/app/ChooserActivity.java | 165 ++++++++++-------- .../internal/widget/ResolverDrawerLayout.java | 7 + core/res/res/drawable/ic_drag_handle.xml | 14 +- core/res/res/layout/chooser_grid.xml | 14 +- core/res/res/values/dimens.xml | 1 + core/res/res/values/symbols.xml | 1 + .../internal/app/ChooserActivityTest.java | 4 +- 7 files changed, 116 insertions(+), 90 deletions(-) diff --git a/core/java/com/android/internal/app/ChooserActivity.java b/core/java/com/android/internal/app/ChooserActivity.java index 54fff9b532f81..5c60ab015c6d6 100644 --- a/core/java/com/android/internal/app/ChooserActivity.java +++ b/core/java/com/android/internal/app/ChooserActivity.java @@ -162,6 +162,8 @@ public class ChooserActivity extends ResolverActivity { */ private static final int NO_DIRECT_SHARE_ANIM_IN_MILLIS = 200; + private static final float DIRECT_SHARE_EXPANSION_RATE = 0.7f; + // TODO(b/121287224): Re-evaluate this limit private static final int SHARE_TARGET_QUERY_PACKAGE_LIMIT = 20; @@ -434,9 +436,13 @@ public class ChooserActivity extends ResolverActivity { mChooserRowServiceSpacing = getResources() .getDimensionPixelSize(R.dimen.chooser_service_spacing); - // expand/shrink direct share 4 -> 8 viewgroup - if (mResolverDrawerLayout != null && isSendAction(target)) { - mResolverDrawerLayout.setOnScrollChangeListener(this::handleScroll); + if (mResolverDrawerLayout != null) { + mResolverDrawerLayout.addOnLayoutChangeListener(this::handleLayoutChange); + + // expand/shrink direct share 4 -> 8 viewgroup + if (isSendAction(target)) { + mResolverDrawerLayout.setOnScrollChangeListener(this::handleScroll); + } } if (DEBUG) { @@ -877,18 +883,9 @@ public class ChooserActivity extends ResolverActivity { mChooserListAdapter.addServiceResults(null, Lists.newArrayList(mCallerChooserTargets)); } mChooserRowAdapter = new ChooserRowAdapter(mChooserListAdapter); - mChooserRowAdapter.registerDataSetObserver(new OffsetDataSetObserver(adapterView)); if (listView != null) { listView.setItemsCanFocus(true); - listView.addOnLayoutChangeListener( - (v, left, top, right, bottom, oldLeft, oldTop, oldRight, oldBottom) -> { - if (mChooserRowAdapter.calculateMaxTargetsPerRow(right - left)) { - adapterView.setAdapter(mChooserRowAdapter); - } - }); } - - adapterView.setAdapter(mChooserRowAdapter); } @Override @@ -1727,6 +1724,66 @@ public class ChooserActivity extends ResolverActivity { } } + /* + * Need to dynamically adjust how many icons can fit per row before we add them, + * which also means setting the correct offset to initially show the content + * preview area + 2 rows of targets + */ + private void handleLayoutChange(View v, int left, int top, int right, int bottom, int oldLeft, + int oldTop, int oldRight, int oldBottom) { + if (mChooserRowAdapter == null || mAdapterView == null) { + return; + } + + if (mChooserRowAdapter.calculateMaxTargetsPerRow(right - left) + || mAdapterView.getAdapter() == null) { + mAdapterView.setAdapter(mChooserRowAdapter); + + getMainThreadHandler().post(() -> { + if (mResolverDrawerLayout == null || mChooserRowAdapter == null) { + return; + } + + int offset = 0; + int rowsToShow = mChooserRowAdapter.getContentPreviewRowCount() + + mChooserRowAdapter.getServiceTargetRowCount() + + mChooserRowAdapter.getCallerTargetRowCount(); + + // then this is most likely not a SEND_* action, so check + // the app target count + if (rowsToShow == 0) { + rowsToShow = mChooserRowAdapter.getCount(); + } + + // still zero? then use a default height and leave, which + // can happen when there are no targets to show + if (rowsToShow == 0) { + offset = getResources().getDimensionPixelSize( + R.dimen.chooser_max_collapsed_height); + mResolverDrawerLayout.setCollapsibleHeightReserved(offset); + return; + } + + int lastHeight = 0; + rowsToShow = Math.max(3, rowsToShow); + for (int i = 0; i < Math.min(rowsToShow, mAdapterView.getChildCount()); i++) { + lastHeight = mAdapterView.getChildAt(i).getHeight(); + offset += lastHeight; + } + + if (lastHeight != 0 && isSendAction(getTargetIntent())) { + // make sure to leave room for direct share 4->8 expansion + int expansionArea = + (int) (mResolverDrawerLayout.getUncollapsibleHeight() + / DIRECT_SHARE_EXPANSION_RATE); + offset = Math.min(offset, bottom - top - lastHeight - expansionArea); + } + + mResolverDrawerLayout.setCollapsibleHeightReserved(offset); + }); + } + } + public class ChooserListAdapter extends ResolveListAdapter { public static final int TARGET_BAD = -1; public static final int TARGET_CALLER = 0; @@ -2540,7 +2597,6 @@ public class ChooserActivity extends ResolverActivity { getRow(0).measure(spec, spec); getRow(1).measure(spec, spec); - // uses ChooserActiivty state variables to track height mDirectShareMinHeight = getRow(0).getMeasuredHeight(); mDirectShareCurrHeight = mDirectShareCurrHeight > 0 ? mDirectShareCurrHeight : mDirectShareMinHeight; @@ -2573,18 +2629,18 @@ public class ChooserActivity extends ResolverActivity { } public void handleScroll(AbsListView view, int y, int oldy, int maxTargetsPerRow) { - // only expand if we have more than 4 targets, and delay that decision until - // they start to scroll if (mHideDirectShareExpansion) { return; } + // only expand if we have more than maxTargetsPerRow, and delay that decision + // until they start to scroll if (mChooserListAdapter.getSelectableServiceTargetCount() <= maxTargetsPerRow) { mHideDirectShareExpansion = true; return; } - int yDiff = (int) ((oldy - y) * 0.7f); + int yDiff = (int) ((oldy - y) * DIRECT_SHARE_EXPANSION_RATE); int prevHeight = mDirectShareCurrHeight; mDirectShareCurrHeight = Math.min(mDirectShareCurrHeight + yDiff, @@ -2592,27 +2648,31 @@ public class ChooserActivity extends ResolverActivity { mDirectShareCurrHeight = Math.max(mDirectShareCurrHeight, mDirectShareMinHeight); yDiff = mDirectShareCurrHeight - prevHeight; - if (view == null || view.getChildCount() == 0) { + if (view == null || view.getChildCount() == 0 || yDiff == 0) { return; } - int index = mChooserRowAdapter.getContentPreviewRowCount(); + // locate the item to expand, and offset the rows below that one + boolean foundExpansion = false; + for (int i = 0; i < view.getChildCount(); i++) { + View child = view.getChildAt(i); - ViewGroup expansionGroup = (ViewGroup) view.getChildAt(index); - int widthSpec = MeasureSpec.makeMeasureSpec(expansionGroup.getWidth(), - MeasureSpec.EXACTLY); - int heightSpec = MeasureSpec.makeMeasureSpec(mDirectShareCurrHeight, - MeasureSpec.EXACTLY); - expansionGroup.measure(widthSpec, heightSpec); - expansionGroup.getLayoutParams().height = expansionGroup.getMeasuredHeight(); - expansionGroup.layout(expansionGroup.getLeft(), expansionGroup.getTop(), - expansionGroup.getRight(), - expansionGroup.getTop() + expansionGroup.getMeasuredHeight()); + if (foundExpansion) { + child.offsetTopAndBottom(yDiff); + } else { + if (child.getTag() != null && child.getTag() instanceof DirectShareViewHolder) { + int widthSpec = MeasureSpec.makeMeasureSpec(child.getWidth(), + MeasureSpec.EXACTLY); + int heightSpec = MeasureSpec.makeMeasureSpec(mDirectShareCurrHeight, + MeasureSpec.EXACTLY); + child.measure(widthSpec, heightSpec); + child.getLayoutParams().height = child.getMeasuredHeight(); + child.layout(child.getLeft(), child.getTop(), child.getRight(), + child.getTop() + child.getMeasuredHeight()); - // reposition list items - int items = view.getChildCount(); - for (int i = index + 1; i < items; i++) { - view.getChildAt(i).offsetTopAndBottom(yDiff); + foundExpansion = true; + } + } } } } @@ -2770,47 +2830,6 @@ public class ChooserActivity extends ResolverActivity { } } - class OffsetDataSetObserver extends DataSetObserver { - private final AbsListView mListView; - private int mCachedViewType = -1; - private View mCachedView; - - public OffsetDataSetObserver(AbsListView listView) { - mListView = listView; - } - - @Override - public void onChanged() { - if (mResolverDrawerLayout == null) { - return; - } - - final int chooserTargetRows = mChooserRowAdapter.getServiceTargetRowCount(); - int offset = 0; - for (int i = 0; i < chooserTargetRows; i++) { - final int pos = mChooserRowAdapter.getContentPreviewRowCount() + i; - final int vt = mChooserRowAdapter.getItemViewType(pos); - if (vt != mCachedViewType) { - mCachedView = null; - } - final View v = mChooserRowAdapter.getView(pos, mCachedView, mListView); - int height = ((RowViewHolder) (v.getTag())).getMeasuredRowHeight(); - - offset += (int) (height); - - if (vt >= 0) { - mCachedViewType = vt; - mCachedView = v; - } else { - mCachedViewType = -1; - } - } - - mResolverDrawerLayout.setCollapsibleHeightReserved(offset); - } - } - - /** * Used internally to round image corners while obeying view padding. */ diff --git a/core/java/com/android/internal/widget/ResolverDrawerLayout.java b/core/java/com/android/internal/widget/ResolverDrawerLayout.java index 9722fcb129f6d..a160b57fe2a19 100644 --- a/core/java/com/android/internal/widget/ResolverDrawerLayout.java +++ b/core/java/com/android/internal/widget/ResolverDrawerLayout.java @@ -868,6 +868,13 @@ public class ResolverDrawerLayout extends ViewGroup { setMeasuredDimension(sourceWidth, heightSize); } + /** + * @return The space reserved by views with 'alwaysShow=true' + */ + public int getUncollapsibleHeight() { + return mUncollapsibleHeight; + } + @Override protected void onLayout(boolean changed, int l, int t, int r, int b) { final int width = getWidth(); diff --git a/core/res/res/drawable/ic_drag_handle.xml b/core/res/res/drawable/ic_drag_handle.xml index 67ab84d4080d4..9b0e204608ff1 100644 --- a/core/res/res/drawable/ic_drag_handle.xml +++ b/core/res/res/drawable/ic_drag_handle.xml @@ -13,11 +13,9 @@ See the License for the specific language governing permissions and limitations under the License. --> - - - \ No newline at end of file + + + + diff --git a/core/res/res/layout/chooser_grid.xml b/core/res/res/layout/chooser_grid.xml index 2860ee478db88..1f8041741b46e 100644 --- a/core/res/res/layout/chooser_grid.xml +++ b/core/res/res/layout/chooser_grid.xml @@ -20,7 +20,7 @@ xmlns:android="http://schemas.android.com/apk/res/android" android:layout_width="match_parent" android:layout_height="match_parent" - android:maxCollapsedHeight="288dp" + android:maxCollapsedHeight="0dp" android:maxCollapsedHeightSmall="56dp" android:id="@id/contentPanel"> @@ -32,12 +32,12 @@ @@ -62,8 +62,8 @@ android:textAppearance="?attr/textAppearanceMedium" android:textSize="20sp" android:gravity="center" - android:paddingTop="18dp" - android:paddingBottom="18dp" + android:paddingTop="@dimen/chooser_view_spacing" + android:paddingBottom="@dimen/chooser_view_spacing" android:paddingLeft="24dp" android:paddingRight="24dp" android:layout_below="@id/profile_button" diff --git a/core/res/res/values/dimens.xml b/core/res/res/values/dimens.xml index 023fbaddfb831..feecd020b087b 100644 --- a/core/res/res/values/dimens.xml +++ b/core/res/res/values/dimens.xml @@ -725,4 +725,5 @@ 42dp 18dp 76dp + 288dp diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index 7ec5b5b5a4b9a..380d171e68420 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -2771,6 +2771,7 @@ + diff --git a/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java b/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java index 8c2375eaaf4c2..185fa0750ff11 100644 --- a/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java +++ b/core/tests/coretests/src/com/android/internal/app/ChooserActivityTest.java @@ -599,7 +599,7 @@ public class ChooserActivityTest { mActivityRule.launchActivity(Intent.createChooser(sendIntent, null)); waitForIdle(); - verify(mockLogger, Mockito.times(3)).write(logMakerCaptor.capture()); + verify(mockLogger, Mockito.times(2)).write(logMakerCaptor.capture()); // First invocation is from onCreate assertThat(logMakerCaptor.getAllValues().get(1).getCategory(), is(MetricsEvent.ACTION_SHARE_WITH_PREVIEW)); @@ -629,7 +629,7 @@ public class ChooserActivityTest { ArgumentCaptor logMakerCaptor = ArgumentCaptor.forClass(LogMaker.class); mActivityRule.launchActivity(Intent.createChooser(sendIntent, null)); waitForIdle(); - verify(mockLogger, Mockito.times(3)).write(logMakerCaptor.capture()); + verify(mockLogger, Mockito.times(2)).write(logMakerCaptor.capture()); // First invocation is from onCreate assertThat(logMakerCaptor.getAllValues().get(1).getCategory(), is(MetricsEvent.ACTION_SHARE_WITH_PREVIEW));