From 677a65d143ddf42de7bcab28100421bf5bbcd593 Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Thu, 5 Oct 2023 19:43:40 +0000 Subject: `showEmptyState()` -> EmptyStateUiHelper As part of ongoing work to pull low-level empty state responsibilities out of the pager adapter, this CL generally replicates the change prototyped in ag/24516421/5..7, in smaller incremental steps to "show the work." As originally described in that CL: "... moves most of the low-level logic of `showEmptyState()` into the UI helper. (...) also has the UI helper take responsibility for setting the visibility of the main "list view" in sync (opposite of) the empty state visibility." As presented in this CL, the "incremental steps" per-snapshot are: 1. Extract most of the implementation directly to the new method at `EmptyStateUiHelper.showEmptyState()`. The general functionality is covered by existing integration tests (e.g., commenting-out the new method body causes `UnbundledChooserActivityWorkProfileTest` to fail). New `EmptyStateUiHelper` unit tests cover finer points of the empty-state "button" conditions, and I've added a TODO comment at one place legacy behavior seemingly may not align with the original developer intent. 2. Also make the UI helper responsible for propagating empty-state visibility changes back to the main list view (hiding the main list when we show an empty state, and restoring it when the empty state is hidden). 3. Look up all the sub-views during `EmptyStateUiHelper` construction so we don't have to keep repeating their View IDs throughout. 4. Tighten visibility on `EmptyStateUiHelper.resetViewVisibilities()` now that it's a private `showEmptyState()` implementation detail (updated to package-protected/visible-for-testing). Also move the method to the end of the class (after all the public methods). Bug: 302311217 Test: IntentResolverUnitTests Change-Id: Iac0cf3d62e2c3bf22afa6a2796ae4e731b706c02 --- .../v2/MultiProfilePagerAdapter.java | 51 ++------- .../v2/emptystate/EmptyStateUiHelper.java | 118 +++++++++++++++------ .../v2/emptystate/EmptyStateUiHelperTest.kt | 88 ++++++++++++++- 3 files changed, 181 insertions(+), 76 deletions(-) (limited to 'java') diff --git a/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java index ad9614b9..391cce7a 100644 --- a/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java +++ b/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java @@ -21,8 +21,6 @@ import android.os.Trace; import android.os.UserHandle; import android.view.View; import android.view.ViewGroup; -import android.widget.Button; -import android.widget.TextView; import androidx.viewpager.widget.PagerAdapter; import androidx.viewpager.widget.ViewPager; @@ -414,6 +412,8 @@ public class MultiProfilePagerAdapter< * The intention is to prevent the user from having to turn * the work profile on if there will not be any apps resolved * anyway. + * + * TODO: move this comment to the place where we configure our composite provider. */ public void showEmptyResolverListEmptyState(ListAdapterT listAdapter) { final EmptyState emptyState = mEmptyStateProvider.getEmptyState(listAdapter); @@ -449,48 +449,13 @@ public class MultiProfilePagerAdapter< } } - protected void showEmptyState( + private void showEmptyState( ListAdapterT activeListAdapter, EmptyState emptyState, View.OnClickListener buttonOnClick) { ProfileDescriptor descriptor = getItem( userHandleToPageIndex(activeListAdapter.getUserHandle())); - descriptor.mRootView.findViewById( - com.android.internal.R.id.resolver_list).setVisibility(View.GONE); - descriptor.mEmptyStateUi.resetViewVisibilities(); - descriptor.setupContainerPadding(); - - ViewGroup emptyStateView = descriptor.getEmptyStateView(); - - - TextView titleView = emptyStateView.findViewById( - com.android.internal.R.id.resolver_empty_state_title); - String title = emptyState.getTitle(); - if (title != null) { - titleView.setVisibility(View.VISIBLE); - titleView.setText(title); - } else { - titleView.setVisibility(View.GONE); - } - - TextView subtitleView = emptyStateView.findViewById( - com.android.internal.R.id.resolver_empty_state_subtitle); - String subtitle = emptyState.getSubtitle(); - if (subtitle != null) { - subtitleView.setVisibility(View.VISIBLE); - subtitleView.setText(subtitle); - } else { - subtitleView.setVisibility(View.GONE); - } - - View defaultEmptyText = emptyStateView.findViewById(com.android.internal.R.id.empty); - defaultEmptyText.setVisibility(emptyState.useDefaultEmptyView() ? View.VISIBLE : View.GONE); - - Button button = emptyStateView.findViewById( - com.android.internal.R.id.resolver_empty_state_button); - button.setVisibility(buttonOnClick != null ? View.VISIBLE : View.GONE); - button.setOnClickListener(buttonOnClick); - + descriptor.mEmptyStateUi.showEmptyState(emptyState, buttonOnClick); activeListAdapter.markTabLoaded(); } @@ -505,8 +470,6 @@ public class MultiProfilePagerAdapter< public void showListView(ListAdapterT activeListAdapter) { ProfileDescriptor descriptor = getItem( userHandleToPageIndex(activeListAdapter.getUserHandle())); - descriptor.mRootView.findViewById( - com.android.internal.R.id.resolver_list).setVisibility(View.VISIBLE); descriptor.mEmptyStateUi.hide(); } @@ -538,8 +501,10 @@ public class MultiProfilePagerAdapter< mAdapter = adapter; mEmptyStateView = rootView.findViewById(com.android.internal.R.id.resolver_empty_state); mView = (PageViewT) rootView.findViewById(com.android.internal.R.id.resolver_list); - mEmptyStateUi = - new EmptyStateUiHelper(rootView, containerBottomPaddingOverrideSupplier); + mEmptyStateUi = new EmptyStateUiHelper( + rootView, + com.android.internal.R.id.resolver_list, + containerBottomPaddingOverrideSupplier); } protected ViewGroup getEmptyStateView() { diff --git a/java/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelper.java b/java/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelper.java index fc852f5c..2f1e1b59 100644 --- a/java/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelper.java +++ b/java/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelper.java @@ -17,6 +17,11 @@ package com.android.intentresolver.v2.emptystate; import android.view.View; import android.view.ViewGroup; +import android.widget.Button; +import android.widget.TextView; + +import com.android.intentresolver.emptystate.EmptyState; +import com.android.internal.annotations.VisibleForTesting; import java.util.Optional; import java.util.function.Supplier; @@ -26,58 +31,111 @@ import java.util.function.Supplier; * some empty-state status. */ public class EmptyStateUiHelper { - private final View mEmptyStateView; private final Supplier> mContainerBottomPaddingOverrideSupplier; + private final View mEmptyStateView; + private final View mListView; + private final View mEmptyStateContainerView; + private final TextView mEmptyStateTitleView; + private final TextView mEmptyStateSubtitleView; + private final Button mEmptyStateButtonView; + private final View mEmptyStateProgressView; + private final View mEmptyStateEmptyView; public EmptyStateUiHelper( ViewGroup rootView, + int listViewResourceId, Supplier> containerBottomPaddingOverrideSupplier) { + mContainerBottomPaddingOverrideSupplier = containerBottomPaddingOverrideSupplier; mEmptyStateView = rootView.requireViewById(com.android.internal.R.id.resolver_empty_state); - mContainerBottomPaddingOverrideSupplier = containerBottomPaddingOverrideSupplier; + mListView = rootView.requireViewById(listViewResourceId); + mEmptyStateContainerView = mEmptyStateView.requireViewById( + com.android.internal.R.id.resolver_empty_state_container); + mEmptyStateTitleView = mEmptyStateView.requireViewById( + com.android.internal.R.id.resolver_empty_state_title); + mEmptyStateSubtitleView = mEmptyStateView.requireViewById( + com.android.internal.R.id.resolver_empty_state_subtitle); + mEmptyStateButtonView = mEmptyStateView.requireViewById( + com.android.internal.R.id.resolver_empty_state_button); + mEmptyStateProgressView = mEmptyStateView.requireViewById( + com.android.internal.R.id.resolver_empty_state_progress); + mEmptyStateEmptyView = mEmptyStateView.requireViewById(com.android.internal.R.id.empty); + } + + /** + * Display the described empty state. + * @param emptyState the data describing the cause of this empty-state condition. + * @param buttonOnClick handler for a button that the user might be able to use to circumvent + * the empty-state condition. If null, no button will be displayed. + */ + public void showEmptyState(EmptyState emptyState, View.OnClickListener buttonOnClick) { + resetViewVisibilities(); + setupContainerPadding(); + + String title = emptyState.getTitle(); + if (title != null) { + mEmptyStateTitleView.setVisibility(View.VISIBLE); + mEmptyStateTitleView.setText(title); + } else { + mEmptyStateTitleView.setVisibility(View.GONE); + } + + String subtitle = emptyState.getSubtitle(); + if (subtitle != null) { + mEmptyStateSubtitleView.setVisibility(View.VISIBLE); + mEmptyStateSubtitleView.setText(subtitle); + } else { + mEmptyStateSubtitleView.setVisibility(View.GONE); + } + + mEmptyStateEmptyView.setVisibility( + emptyState.useDefaultEmptyView() ? View.VISIBLE : View.GONE); + // TODO: The EmptyState API says that if `useDefaultEmptyView()` is true, we'll ignore the + // state's specified title/subtitle; where (if anywhere) is that implemented? + + mEmptyStateButtonView.setVisibility(buttonOnClick != null ? View.VISIBLE : View.GONE); + mEmptyStateButtonView.setOnClickListener(buttonOnClick); + + // Don't show the main list view when we're showing an empty state. + mListView.setVisibility(View.GONE); } /** Sets up the padding of the view containing the empty state screens. */ public void setupContainerPadding() { - View container = mEmptyStateView.requireViewById( - com.android.internal.R.id.resolver_empty_state_container); Optional bottomPaddingOverride = mContainerBottomPaddingOverrideSupplier.get(); bottomPaddingOverride.ifPresent(paddingBottom -> - container.setPadding( - container.getPaddingLeft(), - container.getPaddingTop(), - container.getPaddingRight(), + mEmptyStateContainerView.setPadding( + mEmptyStateContainerView.getPaddingLeft(), + mEmptyStateContainerView.getPaddingTop(), + mEmptyStateContainerView.getPaddingRight(), paddingBottom)); } - public void resetViewVisibilities() { - mEmptyStateView.requireViewById(com.android.internal.R.id.resolver_empty_state_title) - .setVisibility(View.VISIBLE); - mEmptyStateView.requireViewById(com.android.internal.R.id.resolver_empty_state_subtitle) - .setVisibility(View.VISIBLE); - mEmptyStateView.requireViewById(com.android.internal.R.id.resolver_empty_state_button) - .setVisibility(View.INVISIBLE); - mEmptyStateView.requireViewById(com.android.internal.R.id.resolver_empty_state_progress) - .setVisibility(View.GONE); - mEmptyStateView.requireViewById(com.android.internal.R.id.empty) - .setVisibility(View.GONE); - mEmptyStateView.setVisibility(View.VISIBLE); - } - public void showSpinner() { - mEmptyStateView.requireViewById(com.android.internal.R.id.resolver_empty_state_title) - .setVisibility(View.INVISIBLE); + mEmptyStateTitleView.setVisibility(View.INVISIBLE); // TODO: subtitle? - mEmptyStateView.requireViewById(com.android.internal.R.id.resolver_empty_state_button) - .setVisibility(View.INVISIBLE); - mEmptyStateView.requireViewById(com.android.internal.R.id.resolver_empty_state_progress) - .setVisibility(View.VISIBLE); - mEmptyStateView.requireViewById(com.android.internal.R.id.empty) - .setVisibility(View.GONE); + mEmptyStateButtonView.setVisibility(View.INVISIBLE); + mEmptyStateProgressView.setVisibility(View.VISIBLE); + mEmptyStateEmptyView.setVisibility(View.GONE); } public void hide() { mEmptyStateView.setVisibility(View.GONE); + mListView.setVisibility(View.VISIBLE); + } + + // TODO: this is exposed for testing so we can thoroughly prepare initial conditions that let us + // observe the resulting change. In reality it's only invoked as part of `showEmptyState()` and + // we could consider setting up narrower "realistic" preconditions to make assertions about the + // higher-level operation. + @VisibleForTesting + void resetViewVisibilities() { + mEmptyStateTitleView.setVisibility(View.VISIBLE); + mEmptyStateSubtitleView.setVisibility(View.VISIBLE); + mEmptyStateButtonView.setVisibility(View.INVISIBLE); + mEmptyStateProgressView.setVisibility(View.GONE); + mEmptyStateEmptyView.setVisibility(View.GONE); + mEmptyStateView.setVisibility(View.VISIBLE); } } diff --git a/java/tests/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelperTest.kt b/java/tests/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelperTest.kt index 27ed7e38..696dd1fd 100644 --- a/java/tests/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelperTest.kt +++ b/java/tests/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelperTest.kt @@ -20,12 +20,18 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.widget.FrameLayout +import android.widget.TextView import androidx.test.platform.app.InstrumentationRegistry +import com.android.intentresolver.any +import com.android.intentresolver.emptystate.EmptyState +import com.android.intentresolver.mock import com.google.common.truth.Truth.assertThat import java.util.Optional import java.util.function.Supplier import org.junit.Before import org.junit.Test +import org.mockito.Mockito.never +import org.mockito.Mockito.verify class EmptyStateUiHelperTest { private val context = InstrumentationRegistry.getInstrumentation().getContext() @@ -37,8 +43,9 @@ class EmptyStateUiHelperTest { } lateinit var rootContainer: ViewGroup - lateinit var emptyStateTitleView: View - lateinit var emptyStateSubtitleView: View + lateinit var mainListView: View // Visible when no empty state is showing. + lateinit var emptyStateTitleView: TextView + lateinit var emptyStateSubtitleView: TextView lateinit var emptyStateButtonView: View lateinit var emptyStateProgressView: View lateinit var emptyStateDefaultTextView: View @@ -55,6 +62,7 @@ class EmptyStateUiHelperTest { rootContainer, true ) + mainListView = rootContainer.requireViewById(com.android.internal.R.id.resolver_list) emptyStateRootView = rootContainer.requireViewById(com.android.internal.R.id.resolver_empty_state) emptyStateTitleView = @@ -68,7 +76,12 @@ class EmptyStateUiHelperTest { emptyStateDefaultTextView = rootContainer.requireViewById(com.android.internal.R.id.empty) emptyStateContainerView = rootContainer.requireViewById(com.android.internal.R.id.resolver_empty_state_container) - emptyStateUiHelper = EmptyStateUiHelper(rootContainer, containerPaddingSupplier) + emptyStateUiHelper = + EmptyStateUiHelper( + rootContainer, + com.android.internal.R.id.resolver_list, + containerPaddingSupplier + ) } @Test @@ -112,10 +125,12 @@ class EmptyStateUiHelperTest { @Test fun testHide() { emptyStateRootView.visibility = View.VISIBLE + mainListView.visibility = View.GONE emptyStateUiHelper.hide() assertThat(emptyStateRootView.visibility).isEqualTo(View.GONE) + assertThat(mainListView.visibility).isEqualTo(View.VISIBLE) } @Test @@ -143,4 +158,71 @@ class EmptyStateUiHelperTest { assertThat(emptyStateContainerView.paddingRight).isEqualTo(3) assertThat(emptyStateContainerView.paddingBottom).isEqualTo(42) } + + @Test + fun testShowEmptyState_noOnClickHandler() { + mainListView.visibility = View.VISIBLE + + // Note: an `EmptyState.ClickListener` isn't invoked directly by the UI helper; it has to be + // built into the "on-click handler" that's injected to implement the button-press. We won't + // display the button without a click "handler," even if it *does* have a `ClickListener`. + val clickListener = mock() + + val emptyState = + object : EmptyState { + override fun getTitle() = "Test title" + override fun getSubtitle() = "Test subtitle" + + override fun getButtonClickListener() = clickListener + } + emptyStateUiHelper.showEmptyState(emptyState, null) + + assertThat(mainListView.visibility).isEqualTo(View.GONE) + assertThat(emptyStateRootView.visibility).isEqualTo(View.VISIBLE) + assertThat(emptyStateTitleView.visibility).isEqualTo(View.VISIBLE) + assertThat(emptyStateSubtitleView.visibility).isEqualTo(View.VISIBLE) + assertThat(emptyStateButtonView.visibility).isEqualTo(View.GONE) + assertThat(emptyStateProgressView.visibility).isEqualTo(View.GONE) + assertThat(emptyStateDefaultTextView.visibility).isEqualTo(View.GONE) + + assertThat(emptyStateTitleView.text).isEqualTo("Test title") + assertThat(emptyStateSubtitleView.text).isEqualTo("Test subtitle") + + verify(clickListener, never()).onClick(any()) + } + + @Test + fun testShowEmptyState_withOnClickHandlerAndClickListener() { + mainListView.visibility = View.VISIBLE + + val clickListener = mock() + val onClickHandler = mock() + + val emptyState = + object : EmptyState { + override fun getTitle() = "Test title" + override fun getSubtitle() = "Test subtitle" + + override fun getButtonClickListener() = clickListener + } + emptyStateUiHelper.showEmptyState(emptyState, onClickHandler) + + assertThat(mainListView.visibility).isEqualTo(View.GONE) + assertThat(emptyStateRootView.visibility).isEqualTo(View.VISIBLE) + assertThat(emptyStateTitleView.visibility).isEqualTo(View.VISIBLE) + assertThat(emptyStateSubtitleView.visibility).isEqualTo(View.VISIBLE) + assertThat(emptyStateButtonView.visibility).isEqualTo(View.VISIBLE) // Now shown. + assertThat(emptyStateProgressView.visibility).isEqualTo(View.GONE) + assertThat(emptyStateDefaultTextView.visibility).isEqualTo(View.GONE) + + assertThat(emptyStateTitleView.text).isEqualTo("Test title") + assertThat(emptyStateSubtitleView.text).isEqualTo("Test subtitle") + + emptyStateButtonView.performClick() + + verify(onClickHandler).onClick(emptyStateButtonView) + // The test didn't explicitly configure its `OnClickListener` to relay the click event on + // to the `EmptyState.ClickListener`, so it still won't have fired here. + verify(clickListener, never()).onClick(any()) + } } -- cgit v1.2.3-59-g8ed1b