From 0b320545f8460dd1d7517f4a62670843794f9ebf Mon Sep 17 00:00:00 2001 From: Alina Zaidi Date: Mon, 21 Jun 2021 17:56:44 +0100 Subject: [PATCH] Compare maxSpanSize for WidgetsListContentEntries when updating visible entries in widget picker. This is so that widgets are rearranged according to maxSpanSize in widget table Bug: 191336852 Test: Manually rotated widget picker with one of the headers expanded and checked that widgets in widget table adjust according to screen's horizontal size. Change-Id: I2406ca9eccd18f4bb32b396acaa188c37f0059ee --- .../picker/WidgetsDiffReporterTest.java | 26 ++++- .../model/WidgetsListContentEntryTest.java | 105 ++++++++++++++++-- .../widget/model/WidgetsListContentEntry.java | 54 ++++++++- .../widget/picker/WidgetsDiffReporter.java | 10 +- .../widget/picker/WidgetsListAdapter.java | 23 ++-- .../WidgetsListTableViewHolderBinder.java | 8 +- .../widget/util/WidgetsTableUtils.java | 7 ++ 7 files changed, 197 insertions(+), 36 deletions(-) diff --git a/robolectric_tests/src/com/android/launcher3/widget/picker/WidgetsDiffReporterTest.java b/robolectric_tests/src/com/android/launcher3/widget/picker/WidgetsDiffReporterTest.java index c946c7218f..b9f183c713 100644 --- a/robolectric_tests/src/com/android/launcher3/widget/picker/WidgetsDiffReporterTest.java +++ b/robolectric_tests/src/com/android/launcher3/widget/picker/WidgetsDiffReporterTest.java @@ -58,7 +58,7 @@ import java.util.List; @RunWith(RobolectricTestRunner.class) public final class WidgetsDiffReporterTest { - private static final String TEST_PACKAGE_PREFIX = "com.google.test"; + private static final String TEST_PACKAGE_PREFIX = "com.android.test"; private static final WidgetListBaseRowEntryComparator COMPARATOR = new WidgetListBaseRowEntryComparator(); @@ -241,6 +241,30 @@ public final class WidgetsDiffReporterTest { assertThat(currentList).containsExactlyElementsIn(newList); } + @Test + public void headersContentsMix_contentMaxSpanSizeModified_shouldInvokeCorrectCallbacks() { + // GIVEN the current list has app headers [A, B, E content]. + ArrayList currentList = new ArrayList<>( + List.of(mHeaderA, mHeaderB, mContentE)); + // GIVEN the new list has max span size in "E content" modified. + List newList = List.of( + mHeaderA, + mHeaderB, + new WidgetsListContentEntry( + mContentE.mPkgItem, + mContentE.mTitleSectionName, + mContentE.mWidgets, + mContentE.getMaxSpanSizeInCells() + 1)); + + // WHEN computing the list difference. + mWidgetsDiffReporter.process(currentList, newList, COMPARATOR); + + // THEN notify "E content" has been changed. + verify(mAdapter).notifyItemChanged(/* position= */ 2); + // THEN the current list contains all elements from the new list. + assertThat(currentList).containsExactlyElementsIn(newList); + } + private WidgetsListHeaderEntry createWidgetsHeaderEntry(String packageName, String appName, int numOfWidgets) { diff --git a/robolectric_tests/src/com/android/launcher3/widget/picker/model/WidgetsListContentEntryTest.java b/robolectric_tests/src/com/android/launcher3/widget/picker/model/WidgetsListContentEntryTest.java index 2d22c45e2c..106cac0fdf 100644 --- a/robolectric_tests/src/com/android/launcher3/widget/picker/model/WidgetsListContentEntryTest.java +++ b/robolectric_tests/src/com/android/launcher3/widget/picker/model/WidgetsListContentEntryTest.java @@ -49,8 +49,10 @@ import java.util.Map; @RunWith(RobolectricTestRunner.class) public final class WidgetsListContentEntryTest { - private static final String PACKAGE_NAME = "com.google.test"; - private final PackageItemInfo mPackageItemInfo = new PackageItemInfo(PACKAGE_NAME); + private static final String PACKAGE_NAME = "com.android.test"; + private static final String PACKAGE_NAME_2 = "com.android.test2"; + private final PackageItemInfo mPackageItemInfo1 = new PackageItemInfo(PACKAGE_NAME); + private final PackageItemInfo mPackageItemInfo2 = new PackageItemInfo(PACKAGE_NAME_2); private final ComponentName mWidget1 = ComponentName.createRelative(PACKAGE_NAME, ".mWidget1"); private final ComponentName mWidget2 = ComponentName.createRelative(PACKAGE_NAME, ".mWidget2"); private final ComponentName mWidget3 = ComponentName.createRelative(PACKAGE_NAME, ".mWidget3"); @@ -91,7 +93,7 @@ public final class WidgetsListContentEntryTest { WidgetItem widgetItem3 = createWidgetItem(mWidget3, /* spanX= */ 2, /* spanY= */ 3); // WHEN creates a WidgetsListRowEntry with the unsorted widgets. - WidgetsListContentEntry widgetsListRowEntry = new WidgetsListContentEntry(mPackageItemInfo, + WidgetsListContentEntry widgetsListRowEntry = new WidgetsListContentEntry(mPackageItemInfo1, /* titleSectionName= */ "T", List.of(widgetItem1, widgetItem2, widgetItem3)); @@ -100,7 +102,7 @@ public final class WidgetsListContentEntryTest { .containsExactly(widgetItem3, widgetItem1, widgetItem2) .inOrder(); assertThat(widgetsListRowEntry.mTitleSectionName).isEqualTo("T"); - assertThat(widgetsListRowEntry.mPkgItem).isEqualTo(mPackageItemInfo); + assertThat(widgetsListRowEntry.mPkgItem).isEqualTo(mPackageItemInfo1); } @Test @@ -114,7 +116,7 @@ public final class WidgetsListContentEntryTest { WidgetItem widgetItem3 = createWidgetItem(mWidget1, /* spanX= */ 2, /* spanY= */ 2); // WHEN creates a WidgetsListRowEntry with the unsorted widgets. - WidgetsListContentEntry widgetsListRowEntry = new WidgetsListContentEntry(mPackageItemInfo, + WidgetsListContentEntry widgetsListRowEntry = new WidgetsListContentEntry(mPackageItemInfo1, /* titleSectionName= */ "T", List.of(widgetItem1, widgetItem2, widgetItem3)); @@ -124,7 +126,7 @@ public final class WidgetsListContentEntryTest { .containsExactly(widgetItem2, widgetItem3, widgetItem1) .inOrder(); assertThat(widgetsListRowEntry.mTitleSectionName).isEqualTo("T"); - assertThat(widgetsListRowEntry.mPkgItem).isEqualTo(mPackageItemInfo); + assertThat(widgetsListRowEntry.mPkgItem).isEqualTo(mPackageItemInfo1); } @Test @@ -140,7 +142,7 @@ public final class WidgetsListContentEntryTest { WidgetItem widgetItem4 = createWidgetItem(mWidget3, /* spanX= */ 2, /* spanY= */ 2); // WHEN creates a WidgetsListRowEntry with the unsorted widgets. - WidgetsListContentEntry widgetsListRowEntry = new WidgetsListContentEntry(mPackageItemInfo, + WidgetsListContentEntry widgetsListRowEntry = new WidgetsListContentEntry(mPackageItemInfo1, /* titleSectionName= */ "T", List.of(widgetItem1, widgetItem2, widgetItem3, widgetItem4)); @@ -151,9 +153,96 @@ public final class WidgetsListContentEntryTest { .containsExactly(widgetItem4, widgetItem2, widgetItem1, widgetItem3) .inOrder(); assertThat(widgetsListRowEntry.mTitleSectionName).isEqualTo("T"); - assertThat(widgetsListRowEntry.mPkgItem).isEqualTo(mPackageItemInfo); + assertThat(widgetsListRowEntry.mPkgItem).isEqualTo(mPackageItemInfo1); } + @Test + public void equals_entriesWithDifferentPackageItemInfo_returnFalse() { + WidgetItem widgetItem1 = createWidgetItem(mWidget1, /* spanX= */ 2, /* spanY= */ 3); + WidgetsListContentEntry widgetsListRowEntry1 = new WidgetsListContentEntry( + mPackageItemInfo1, + /* titleSectionName= */ "T", + List.of(widgetItem1), + /* maxSpanSizeInCells= */ 3); + WidgetsListContentEntry widgetsListRowEntry2 = new WidgetsListContentEntry( + mPackageItemInfo2, + /* titleSectionName= */ "T", + List.of(widgetItem1), + /* maxSpanSizeInCells= */ 3); + + assertThat(widgetsListRowEntry1.equals(widgetsListRowEntry2)).isFalse(); + } + + @Test + public void equals_entriesWithDifferentTitleSectionName_returnFalse() { + WidgetItem widgetItem1 = createWidgetItem(mWidget1, /* spanX= */ 2, /* spanY= */ 3); + WidgetsListContentEntry widgetsListRowEntry1 = new WidgetsListContentEntry( + mPackageItemInfo1, + /* titleSectionName= */ "T", + List.of(widgetItem1), + /* maxSpanSizeInCells= */ 3); + WidgetsListContentEntry widgetsListRowEntry2 = new WidgetsListContentEntry( + mPackageItemInfo1, + /* titleSectionName= */ "S", + List.of(widgetItem1), + /* maxSpanSizeInCells= */ 3); + + assertThat(widgetsListRowEntry1.equals(widgetsListRowEntry2)).isFalse(); + } + + @Test + public void equals_entriesWithDifferentWidgetsList_returnFalse() { + WidgetItem widgetItem1 = createWidgetItem(mWidget1, /* spanX= */ 2, /* spanY= */ 3); + WidgetItem widgetItem2 = createWidgetItem(mWidget2, /* spanX= */ 2, /* spanY= */ 3); + WidgetsListContentEntry widgetsListRowEntry1 = new WidgetsListContentEntry( + mPackageItemInfo1, + /* titleSectionName= */ "T", + List.of(widgetItem1), + /* maxSpanSizeInCells= */ 3); + WidgetsListContentEntry widgetsListRowEntry2 = new WidgetsListContentEntry( + mPackageItemInfo1, + /* titleSectionName= */ "T", + List.of(widgetItem2), + /* maxSpanSizeInCells= */ 3); + + assertThat(widgetsListRowEntry1.equals(widgetsListRowEntry2)).isFalse(); + } + + @Test + public void equals_entriesWithDifferentMaxSpanSize_returnFalse() { + WidgetItem widgetItem1 = createWidgetItem(mWidget1, /* spanX= */ 2, /* spanY= */ 3); + WidgetsListContentEntry widgetsListRowEntry1 = new WidgetsListContentEntry( + mPackageItemInfo1, + /* titleSectionName= */ "T", + List.of(widgetItem1), + /* maxSpanSizeInCells= */ 3); + WidgetsListContentEntry widgetsListRowEntry2 = new WidgetsListContentEntry( + mPackageItemInfo1, + /* titleSectionName= */ "T", + List.of(widgetItem1), + /* maxSpanSizeInCells= */ 2); + + assertThat(widgetsListRowEntry1.equals(widgetsListRowEntry2)).isFalse(); + } + + @Test + public void equals_entriesWithSameContents_returnTrue() { + WidgetItem widgetItem1 = createWidgetItem(mWidget1, /* spanX= */ 2, /* spanY= */ 3); + WidgetsListContentEntry widgetsListRowEntry1 = new WidgetsListContentEntry( + mPackageItemInfo1, + /* titleSectionName= */ "T", + List.of(widgetItem1), + /* maxSpanSizeInCells= */ 3); + WidgetsListContentEntry widgetsListRowEntry2 = new WidgetsListContentEntry( + mPackageItemInfo1, + /* titleSectionName= */ "T", + List.of(widgetItem1), + /* maxSpanSizeInCells= */ 3); + + assertThat(widgetsListRowEntry1.equals(widgetsListRowEntry2)).isTrue(); + } + + private WidgetItem createWidgetItem(ComponentName componentName, int spanX, int spanY) { String label = mWidgetsToLabels.get(componentName); ShadowPackageManager packageManager = shadowOf(mContext.getPackageManager()); diff --git a/src/com/android/launcher3/widget/model/WidgetsListContentEntry.java b/src/com/android/launcher3/widget/model/WidgetsListContentEntry.java index 0328cf68ba..73b17f1d4f 100644 --- a/src/com/android/launcher3/widget/model/WidgetsListContentEntry.java +++ b/src/com/android/launcher3/widget/model/WidgetsListContentEntry.java @@ -26,14 +26,39 @@ import java.util.List; */ public final class WidgetsListContentEntry extends WidgetsListBaseEntry { + private final int mMaxSpanSizeInCells; + + /** + * Constructor for {@link WidgetsListContentEntry}. + * + * @param pkgItem package info associated with the entry + * @param titleSectionName title section name associated with the entry. + * @param items list of widgets for the package. + */ public WidgetsListContentEntry(PackageItemInfo pkgItem, String titleSectionName, List items) { + this(pkgItem, titleSectionName, items, /* maxSpanSizeInCells= */ 0); + } + + /** + * Constructor for {@link WidgetsListContentEntry}. + * + * @param pkgItem package info associated with the entry + * @param titleSectionName title section name associated with the entry. + * @param items list of widgets for the package. + * @param maxSpanSizeInCells the max horizontal span in cells that is allowed for grouping more + * than one widgets in a table row. + */ + public WidgetsListContentEntry(PackageItemInfo pkgItem, String titleSectionName, + List items, int maxSpanSizeInCells) { super(pkgItem, titleSectionName, items); + mMaxSpanSizeInCells = maxSpanSizeInCells; } @Override public String toString() { - return "Content:" + mPkgItem.packageName + ":" + mWidgets.size(); + return "Content:" + mPkgItem.packageName + ":" + mWidgets.size() + " maxSpanSizeInCells: " + + mMaxSpanSizeInCells; } @Override @@ -42,11 +67,36 @@ public final class WidgetsListContentEntry extends WidgetsListBaseEntry { return RANK_WIDGETS_LIST_CONTENT; } + /** + * Returns a copy of this {@link WidgetsListContentEntry} with updated + * {@param maxSpanSizeInCells}. + * + * @param maxSpanSizeInCells the maximum horizontal span in cells that is allowed for grouping + * more than one widgets in a table row. + */ + public WidgetsListContentEntry withMaxSpanSize(int maxSpanSizeInCells) { + if (mMaxSpanSizeInCells == maxSpanSizeInCells) return this; + return new WidgetsListContentEntry( + mPkgItem, + mTitleSectionName, + mWidgets, + /* maxSpanSizeInCells= */ maxSpanSizeInCells); + } + + /** + * Returns the max horizontal span size in cells that is allowed for grouping more than one + * widget in a table row. + */ + public int getMaxSpanSizeInCells() { + return mMaxSpanSizeInCells; + } + @Override public boolean equals(Object obj) { if (!(obj instanceof WidgetsListContentEntry)) return false; WidgetsListContentEntry otherEntry = (WidgetsListContentEntry) obj; return mWidgets.equals(otherEntry.mWidgets) && mPkgItem.equals(otherEntry.mPkgItem) - && mTitleSectionName.equals(otherEntry.mTitleSectionName); + && mTitleSectionName.equals(otherEntry.mTitleSectionName) + && mMaxSpanSizeInCells == otherEntry.mMaxSpanSizeInCells; } } diff --git a/src/com/android/launcher3/widget/picker/WidgetsDiffReporter.java b/src/com/android/launcher3/widget/picker/WidgetsDiffReporter.java index dfe447a38e..99374f5eef 100644 --- a/src/com/android/launcher3/widget/picker/WidgetsDiffReporter.java +++ b/src/com/android/launcher3/widget/picker/WidgetsDiffReporter.java @@ -115,7 +115,7 @@ public class WidgetsDiffReporter { // or did the widget size and desc, span, etc change? if (!isSamePackageItemInfo(orgRowEntry.mPkgItem, newRowEntry.mPkgItem) || hasHeaderUpdated(orgRowEntry, newRowEntry) - || hasWidgetsListChanged(orgRowEntry, newRowEntry)) { + || hasWidgetsListContentChanged(orgRowEntry, newRowEntry)) { index = currentEntries.indexOf(orgRowEntry); currentEntries.set(index, newRowEntry); mListener.notifyItemChanged(index); @@ -158,17 +158,15 @@ public class WidgetsDiffReporter { /** * Returns {@code true} if both {@code curRow} & {@code newRow} are - * {@link WidgetsListContentEntry}s with a different list of widgets. + * {@link WidgetsListContentEntry}s with a different list or arrangement of widgets. */ - private boolean hasWidgetsListChanged(WidgetsListBaseEntry curRow, + private boolean hasWidgetsListContentChanged(WidgetsListBaseEntry curRow, WidgetsListBaseEntry newRow) { if (!(curRow instanceof WidgetsListContentEntry) || !(newRow instanceof WidgetsListContentEntry)) { return false; } - WidgetsListContentEntry orgRowEntry = (WidgetsListContentEntry) curRow; - WidgetsListContentEntry newRowEntry = (WidgetsListContentEntry) newRow; - return !orgRowEntry.mWidgets.equals(newRowEntry.mWidgets); + return !curRow.equals(newRow); } /** diff --git a/src/com/android/launcher3/widget/picker/WidgetsListAdapter.java b/src/com/android/launcher3/widget/picker/WidgetsListAdapter.java index b668c90031..08a2263eff 100644 --- a/src/com/android/launcher3/widget/picker/WidgetsListAdapter.java +++ b/src/com/android/launcher3/widget/picker/WidgetsListAdapter.java @@ -111,6 +111,7 @@ public class WidgetsListAdapter extends Adapter implements OnHeaderC @Nullable private PackageUserKey mPendingClickHeader; private final int mShortcutPreviewPadding; private final int mSpacingBetweenEntries; + private int mMaxSpanSize = 4; private final WidgetPreviewLoadedCallback mPreviewLoadedCallback = ignored -> updateVisibleEntries(); @@ -249,10 +250,14 @@ public class WidgetsListAdapter extends Adapter implements OnHeaderC .filter(entry -> (mFilter == null || mFilter.test(entry)) && mHeaderAndSelectedContentFilter.test(entry)) .map(entry -> { - // Adjust the original entries to expand headers for the selected content. if (entry instanceof WidgetsListBaseEntry.Header && matchesKey(entry, mWidgetsContentVisiblePackageUserKey)) { + // Adjust the original entries to expand headers for the selected content. return ((WidgetsListBaseEntry.Header) entry).withWidgetListShown(); + } else if (entry instanceof WidgetsListContentEntry) { + // Adjust the original content entries to accommodate for the current + // maxSpanSize. + return ((WidgetsListContentEntry) entry).withMaxSpanSize(mMaxSpanSize); } return entry; }) @@ -491,20 +496,12 @@ public class WidgetsListAdapter extends Adapter implements OnHeaderC } /** - * Sets the max horizontal spans that are allowed for grouping more than one widgets in a table - * row. - * - *

If there is only one widget in a row, that widget horizontal span is allowed to exceed - * {@code maxHorizontalSpans}. - *

Let's say the max horizontal spans is set to 5. Widgets can be grouped in the same row if - * their total horizontal spans added don't exceed 5. - * Example 1: Row 1: 2x2, 2x3, 1x1. Total horizontal spans is 5. This is okay. - * Example 2: Row 1: 2x2, 4x3, 1x1. the total horizontal spans is 7. This is wrong. - * 4x3 and 1x1 should be moved to a new row. - * Example 3: Row 1: 6x4. This is okay because this is the only item in the row. + * Sets the max horizontal span in cells that is allowed for grouping more than one widget in a + * table row. */ public void setMaxHorizontalSpansPerRow(int maxHorizontalSpans) { - mWidgetsListTableViewHolderBinder.setMaxSpansPerRow(maxHorizontalSpans); + mMaxSpanSize = maxHorizontalSpans; + updateVisibleEntries(); } /** diff --git a/src/com/android/launcher3/widget/picker/WidgetsListTableViewHolderBinder.java b/src/com/android/launcher3/widget/picker/WidgetsListTableViewHolderBinder.java index 7b526639f5..57dec1401e 100644 --- a/src/com/android/launcher3/widget/picker/WidgetsListTableViewHolderBinder.java +++ b/src/com/android/launcher3/widget/picker/WidgetsListTableViewHolderBinder.java @@ -49,7 +49,6 @@ public final class WidgetsListTableViewHolderBinder private static final boolean DEBUG = false; private static final String TAG = "WidgetsListRowViewHolderBinder"; - private int mMaxSpansPerRow = 4; private final LayoutInflater mLayoutInflater; private final OnClickListener mIconClickListener; private final OnLongClickListener mIconLongClickListener; @@ -82,10 +81,6 @@ public final class WidgetsListTableViewHolderBinder mApplyBitmapDeferred = applyBitmapDeferred; } - public void setMaxSpansPerRow(int maxSpansPerRow) { - mMaxSpansPerRow = maxSpansPerRow; - } - @Override public WidgetsRowViewHolder newViewHolder(ViewGroup parent) { if (DEBUG) { @@ -113,7 +108,8 @@ public final class WidgetsListTableViewHolderBinder position == mWidgetsListAdapter.getItemCount() - 1 ? LAST : MIDDLE); List> widgetItemsTable = - WidgetsTableUtils.groupWidgetItemsIntoTable(entry.mWidgets, mMaxSpansPerRow); + WidgetsTableUtils.groupWidgetItemsIntoTable( + entry.mWidgets, entry.getMaxSpanSizeInCells()); recycleTableBeforeBinding(table, widgetItemsTable); // Bind the widget items. for (int i = 0; i < widgetItemsTable.size(); i++) { diff --git a/src/com/android/launcher3/widget/util/WidgetsTableUtils.java b/src/com/android/launcher3/widget/util/WidgetsTableUtils.java index 7294a3ab31..54aaf93ac0 100644 --- a/src/com/android/launcher3/widget/util/WidgetsTableUtils.java +++ b/src/com/android/launcher3/widget/util/WidgetsTableUtils.java @@ -56,6 +56,13 @@ public final class WidgetsTableUtils { * 3. The order shortcuts are grouped together in the same row until their total horizontal * spans exceed the {@code maxSpansPerRow} - 1. * 4. If there is only one widget in a row, its width may exceed the {@code maxSpansPerRow}. + * + *

Let's say the {@code maxSpansPerRow} is set to 6. Widgets can be grouped in the same row + * if their total horizontal spans added don't exceed 5. + * Example 1: Row 1: 2x2, 2x3, 1x1. Total horizontal spans is 5. This is okay. + * Example 2: Row 1: 2x2, 4x3, 1x1. the total horizontal spans is 7. This is wrong. 4x3 and 1x1 + * should be moved to a new row. + * Example 3: Row 1: 6x4. This is okay because this is the only item in the row. */ public static List> groupWidgetItemsIntoTable( List widgetItems, final int maxSpansPerRow) {