diff options
author | Joshua Trask <joshtrask@google.com> | 2023-12-05 19:55:41 +0000 |
---|---|---|
committer | Joshua Trask <joshtrask@google.com> | 2023-12-06 19:28:45 +0000 |
commit | 6f0eeb452ae640a1adfa87aacefb50c083441b46 (patch) | |
tree | 2fa6c4d33ad6022f2a380ad3dd6d117c0373eb04 | |
parent | 702b0bcb8cde313c97cdd461e6c96b8075cf0b6d (diff) | |
download | IntentResolver-6f0eeb452ae640a1adfa87aacefb50c083441b46.tar.gz |
Move per-profile operations to pager-adapter APIs
These are methods previously implemented in the activities, which
operate generically on all "profile tabs" (or the "active" or
"inactive" tabs) -- as opposed to special-case behaviors for
personal/work/clone/private/etc. The pager-adapter API exposed
low-level access to its data model, allowing clients to implement
their own algorithms that implicitly inherited our historic "2-tab"
assumption. By pulling these methods into the pager-adapter (which
was generally supposed to own this data model anyways), we limit
the scope of the remaining cleanup to relax the "2-tab" assumption.
These changes are as prototyped in ag/25335069 and described in
go/chooser-ntab-refactoring (starting with "snapshot #7"; note
according to go/chooser-ntab-refactoring, this "arc" begins with
"snapshot #6" of ag/25335069, but that particular change builds on
other changes reproduced in ag/25560839, so here we skip that one
change to get a "clean start" for parallel review).
Snapshot #1 (from ag/25335069 "snapshot #7"):
Generalize `ChooserActivity.shouldShowExtraRow()` to consider the
presence of empty-state screens in "any" inactive adapter, while
pushing the logic for that determination into the pager-adapter.
The logic isn't yet updated to evaluate multiple inactive profiles,
but it's at least consolidated into the `MultiProfilePagerAdapter`
component to be fixed later. Note the refactoring omits the old
`shouldShowTabs()` condition; this is implied since the new
"any-inactive" logic returns false if there are no inactive tabs.
There's seemingly no coverage for this "extra-row" functionality;
I'm open to suggestions but maybe we can wait for more requirements
from the private-space work. For now the equivalence should still
be clear from code-reading (but I'd reluctantly be willing to revert
the removal of the `shouldShowTabs()` in consideration of the scant
testing).
Snapshot #2 (from ag/25335069 "snapshot #8"):
Instead of letting `ResolverActivity` use low-level pager-adapter
methods to access the "active" and "inactive" tabs to rebuild,
have the activity request a "tab rebuild" where the pager-adapter
is responsible for deciding the applicable tabs according to its
internal data model. The equivalence of this change should be clear
from code-reading, but in the interest of completeness I confirmed
that we have some existing test coverage exercising this flow -- an
early return (true or false) from the new `rebuildTabs()` causes a
test failure, as does an inversion of the 'partial-rebuild' condtion
(i.e., starting with `includePartialRebuildOfInactiveTabs = !...").
Snapshot #3 (from ag/25335069 "snapshot #9"):
Move a concrete-`ResolverActivity`-specific operation into its
subclass pager-adapter since it generically manipulates "inactive
tabs." This test method is exercised by the legacy
`ResolverActivityTest` but we don't seem to make any assertions about
its behavior (do we need to add more test coverage now? It may be hard
to set up, and we expect significant architectural changes in the near
future anyways...)
Snapshot #4 (from ag/25335069 "snapshot #17"):
Move a `ChooserActivity`-specific operation into its subclass
pager-adapter since it generically manipulates *all* tabs. As in the
previous snapshot, this method is only nominally covered in tests, but
we don't make specific assertions about its behavior.
Bug: 310211468
Test: `IntentResolver-tests-{unit,activity,integration}`. See notes ^
Change-Id: Ie4a0e99a59872ec16f8491bfa6ffb548e37db1da
5 files changed, 59 insertions, 22 deletions
diff --git a/java/src/com/android/intentresolver/v2/ChooserActivity.java b/java/src/com/android/intentresolver/v2/ChooserActivity.java index 95eedf4..19032b3 100644 --- a/java/src/com/android/intentresolver/v2/ChooserActivity.java +++ b/java/src/com/android/intentresolver/v2/ChooserActivity.java @@ -992,11 +992,7 @@ public class ChooserActivity extends Hilt_ChooserActivity implements @Override protected void applyFooterView(int height) { - int count = mChooserMultiProfilePagerAdapter.getItemCount(); - - for (int i = 0; i < count; i++) { - mChooserMultiProfilePagerAdapter.getAdapterForIndex(i).setFooterHeight(height); - } + mChooserMultiProfilePagerAdapter.setFooterHeightInEveryAdapter(height); } private void logDirectShareTargetReceived(UserHandle forUser) { @@ -1499,14 +1495,13 @@ public class ChooserActivity extends Hilt_ChooserActivity implements /** * If we have a tabbed view and are showing 1 row in the current profile and an empty - * state screen in the other profile, to prevent cropping of the empty state screen we show + * state screen in another profile, to prevent cropping of the empty state screen we show * a second row in the current profile. */ private boolean shouldShowExtraRow(int rowsToShow) { - return shouldShowTabs() - && rowsToShow == 1 - && mChooserMultiProfilePagerAdapter.shouldShowEmptyStateScreen( - mChooserMultiProfilePagerAdapter.getInactiveListAdapter()); + return rowsToShow == 1 + && mChooserMultiProfilePagerAdapter + .shouldShowEmptyStateScreenInAnyInactiveAdapter(); } /** diff --git a/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java index 87b3b20..7ea78d1 100644 --- a/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java +++ b/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java @@ -161,6 +161,13 @@ public class ChooserMultiProfilePagerAdapter extends MultiProfilePagerAdapter< return super.rebuildTab(listAdapter, doPostProcessing); } + /** Apply the specified {@code height} as the footer in each tab's adapter. */ + public void setFooterHeightInEveryAdapter(int height) { + for (int i = 0; i < getItemCount(); ++i) { + getAdapterForIndex(i).setFooterHeight(height); + } + } + private static class BottomPaddingOverrideSupplier implements Supplier<Optional<Integer>> { private final Context mContext; private int mBottomOffset; diff --git a/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java index a600d4a..212bf3b 100644 --- a/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java +++ b/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java @@ -354,6 +354,28 @@ public class MultiProfilePagerAdapter< } /** + * Fully-rebuild the active tab and, if specified, partially-rebuild any other inactive tabs. + */ + public boolean rebuildTabs(boolean includePartialRebuildOfInactiveTabs) { + // TODO: we may be able to determine `includePartialRebuildOfInactiveTabs` ourselves as + // a function of our own instance state. OTOH the purpose of this "partial rebuild" is to + // be able to evaluate the intermediate state of one particular profile tab (i.e. work + // profile) that may not generalize well when we have other "inactive tabs." I.e., either we + // rebuild *all* the inactive tabs just to evaluate some auto-launch conditions that only + // depend on personal and/or work tabs, or we have to explicitly specify the ones we care + // about. It's not the pager-adapter's business to know "which ones we care about," so maybe + // they should be rebuilt lazily when-and-if it comes up (e.g. during the evaluation of + // autolaunch conditions). + boolean rebuildCompleted = rebuildActiveTab(true) || getActiveListAdapter().isTabLoaded(); + if (includePartialRebuildOfInactiveTabs) { + boolean rebuildInactiveCompleted = + rebuildInactiveTab(false) || getInactiveListAdapter().isTabLoaded(); + rebuildCompleted = rebuildCompleted && rebuildInactiveCompleted; + } + return rebuildCompleted; + } + + /** * Rebuilds the tab that is currently visible to the user. * <p>Returns {@code true} if rebuild has completed. */ @@ -368,7 +390,7 @@ public class MultiProfilePagerAdapter< * Rebuilds the tab that is not currently visible to the user, if such one exists. * <p>Returns {@code true} if rebuild has completed. */ - public final boolean rebuildInactiveTab(boolean doPostProcessing) { + private boolean rebuildInactiveTab(boolean doPostProcessing) { Trace.beginSection("MultiProfilePagerAdapter#rebuildInactiveTab"); if (getItemCount() == 1) { Trace.endSection(); @@ -477,6 +499,18 @@ public class MultiProfilePagerAdapter< descriptor.mEmptyStateUi.hide(); } + /** + * @return whether any "inactive" tab's adapter would show an empty-state screen in our current + * application state. + */ + public final boolean shouldShowEmptyStateScreenInAnyInactiveAdapter() { + if (getCount() < 2) { + return false; + } + // TODO: check against *any* inactive adapter; for now we only have one. + return shouldShowEmptyStateScreen(getInactiveListAdapter()); + } + public boolean shouldShowEmptyStateScreen(ListAdapterT listAdapter) { int count = listAdapter.getUnfilteredCount(); return (count == 0 && listAdapter.getPlaceholderCount() == 0) diff --git a/java/src/com/android/intentresolver/v2/ResolverActivity.java b/java/src/com/android/intentresolver/v2/ResolverActivity.java index a7f2047..2c1497f 100644 --- a/java/src/com/android/intentresolver/v2/ResolverActivity.java +++ b/java/src/com/android/intentresolver/v2/ResolverActivity.java @@ -1582,13 +1582,7 @@ public class ResolverActivity extends FragmentActivity implements Trace.beginSection("configureContentView"); // We partially rebuild the inactive adapter to determine if we should auto launch // isTabLoaded will be true here if the empty state screen is shown instead of the list. - boolean rebuildCompleted = mMultiProfilePagerAdapter.rebuildActiveTab(true) - || mMultiProfilePagerAdapter.getActiveListAdapter().isTabLoaded(); - if (shouldShowTabs()) { - boolean rebuildInactiveCompleted = mMultiProfilePagerAdapter.rebuildInactiveTab(false) - || mMultiProfilePagerAdapter.getInactiveListAdapter().isTabLoaded(); - rebuildCompleted = rebuildCompleted && rebuildInactiveCompleted; - } + boolean rebuildCompleted = mMultiProfilePagerAdapter.rebuildTabs(shouldShowTabs()); if (shouldUseMiniResolver()) { configureMiniResolverContent(targetDataLoader); @@ -1962,10 +1956,8 @@ public class ResolverActivity extends FragmentActivity implements return; } mLastSelected = ListView.INVALID_POSITION; - ListView inactiveListView = (ListView) mMultiProfilePagerAdapter.getInactiveAdapterView(); - if (inactiveListView.getCheckedItemCount() > 0) { - inactiveListView.setItemChecked(inactiveListView.getCheckedItemPosition(), false); - } + ((ResolverMultiProfilePagerAdapter) mMultiProfilePagerAdapter) + .clearCheckedItemsInInactiveProfiles(); } private String getPersonalTabAccessibilityLabel() { diff --git a/java/src/com/android/intentresolver/v2/ResolverMultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/v2/ResolverMultiProfilePagerAdapter.java index dadc9c0..d96fd15 100644 --- a/java/src/com/android/intentresolver/v2/ResolverMultiProfilePagerAdapter.java +++ b/java/src/com/android/intentresolver/v2/ResolverMultiProfilePagerAdapter.java @@ -107,6 +107,15 @@ public class ResolverMultiProfilePagerAdapter extends mBottomPaddingOverrideSupplier.setUseLayoutWithDefault(useLayoutWithDefault); } + /** Un-check any item(s) that may be checked in any of our inactive adapter(s). */ + public void clearCheckedItemsInInactiveProfiles() { + // TODO: apply to all inactive adapters; for now we just have the one. + ListView inactiveListView = getInactiveAdapterView(); + if (inactiveListView.getCheckedItemCount() > 0) { + inactiveListView.setItemChecked(inactiveListView.getCheckedItemPosition(), false); + } + } + private static class BottomPaddingOverrideSupplier implements Supplier<Optional<Integer>> { private boolean mUseLayoutWithDefault; |