From c2f1d0cc70ef6083fd47e055a421fd74f620ea33 Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Thu, 5 Oct 2023 17:16:40 +0000 Subject: "Bottom padding overrides" -> 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/4..5, in smaller incremental steps to "show the work." As originally described in that CL: "... moves ownership/usage of the 'container bottom padding override supplier' over to the UI helper; this PagerAdapter dependency is really only used in empty-state logic, so it can be handed over to the extracted component *instead of* retaining it as an ivar in the PagerAdapter." As presented in this CL, the "incremental steps" per-snapshot are: 1. Move `getActiveEmptyStateView()` from `ChooserActivity` to `MultiProfilePagerAdapter`, and expose it in favor of the broader `getEmptyStateView()` method that `ChooserActivity` previously used to implement the lower-level logic. 2. Remove `MultiProfilePagerAdapter.setupContainerPadding()` parameters since they're just derived from a combination of a single hard-coded (resource) symbol, and a concern that already belonged to the pager-adapter (`getActiveEmptyStateView()`). Switch tests to using one of our real layouts so that we can find an empty-state container with the expected View ID. Also update method visibility to show `setupContainerPadding()` isn't intended to be overridden in the more-modern "generic pager" design, and update Javadoc accordingly. 3. Inline `ChooserMultiProfilePagerAdapter.setupContainerPadding()` as part of its `setEmptyStateBottomOffset()`, since the latter method is never called anywhere else (and both concerns already belonged to the specialized pager-adapter). 4. Refactor the internal `setupContainerPadding()` to operate on profile descriptors instead of container Views. This is based on the observation that both callers of `setupContainerPadding()` were passing containers that they had to query from the descriptor's empty-state view anyways, and the lookup used the same View ID in both cases. This change encapsulates those shared concerns at a higher level of abstraction. 5. Move the implementation logic of `setupContainerPadding()` into the descriptor inner-class; given the override-supplier, the rest of the operation can be implemented just using the info available to the descriptor. 6. Move ownership of the override-supplier into the descriptor (to be shared among all descriptor instances). The outer pager-adapter had no remaining need to reference the supplier, and we can easily confirm that this results in `setupContainerPadding()` calls for each descriptor instance using the same supplier instance they would've used before. 7. Moves implementation logic (including [shared] ownership of the override-supplier) from the descriptor down to the empty-state helper which now encapsulates the entire `setupContainerPadding()` operation with zero args. This may require *slightly* more of a leap-of-faith than the other "steps," but note the extensive test coverage: the existing `MultiProfilePagerAdapterTest` covers this exact functionality as integrated into a broader config; parallel unit tests are newly-added in `EmptyStateUiHelper`; and even our "broad-scope" integration tests exercise empty-state logic to _some_ degree. 8. Inlines some client usages to simplify scaffolding that we don't really need after the earlier refactoring changes. Bug: 302311217 Test: IntentResolverUnitTests Change-Id: Icc460ede5b8a0314d5c807dd884e6e2b7044bee9 --- .../v2/MultiProfilePagerAdapterTest.kt | 49 ++++++++++++---------- .../v2/emptystate/EmptyStateUiHelperTest.kt | 36 +++++++++++++++- 2 files changed, 61 insertions(+), 24 deletions(-) (limited to 'java/tests') diff --git a/java/tests/src/com/android/intentresolver/v2/MultiProfilePagerAdapterTest.kt b/java/tests/src/com/android/intentresolver/v2/MultiProfilePagerAdapterTest.kt index f1af9790..f5dc0935 100644 --- a/java/tests/src/com/android/intentresolver/v2/MultiProfilePagerAdapterTest.kt +++ b/java/tests/src/com/android/intentresolver/v2/MultiProfilePagerAdapterTest.kt @@ -26,7 +26,6 @@ import com.android.intentresolver.MultiProfilePagerAdapter.PROFILE_PERSONAL import com.android.intentresolver.MultiProfilePagerAdapter.PROFILE_WORK import com.android.intentresolver.R import com.android.intentresolver.ResolverListAdapter -import com.android.intentresolver.any import com.android.intentresolver.emptystate.EmptyStateProvider import com.android.intentresolver.mock import com.android.intentresolver.whenever @@ -35,8 +34,6 @@ import com.google.common.truth.Truth.assertThat import java.util.Optional import java.util.function.Supplier import org.junit.Test -import org.mockito.Mockito.never -import org.mockito.Mockito.verify class MultiProfilePagerAdapterTest { private val PERSONAL_USER_HANDLE = UserHandle.of(10) @@ -158,20 +155,15 @@ class MultiProfilePagerAdapterTest { @Test fun testBottomPaddingDelegate_default() { - val container = - mock { - whenever(getPaddingLeft()).thenReturn(1) - whenever(getPaddingTop()).thenReturn(2) - whenever(getPaddingRight()).thenReturn(3) - whenever(getPaddingBottom()).thenReturn(4) - } + val personalListAdapter = + mock { whenever(getUserHandle()).thenReturn(PERSONAL_USER_HANDLE) } val pagerAdapter = MultiProfilePagerAdapter( { listAdapter: ResolverListAdapter -> listAdapter }, { listView: ListView, bindAdapter: ResolverListAdapter -> listView.setAdapter(bindAdapter) }, - ImmutableList.of(), + ImmutableList.of(personalListAdapter), object : EmptyStateProvider {}, { false }, PROFILE_PERSONAL, @@ -180,26 +172,29 @@ class MultiProfilePagerAdapterTest { inflater, { Optional.empty() } ) - pagerAdapter.setupContainerPadding(container) - verify(container, never()).setPadding(any(), any(), any(), any()) + val container = + pagerAdapter + .getActiveEmptyStateView() + .requireViewById(com.android.internal.R.id.resolver_empty_state_container) + container.setPadding(1, 2, 3, 4) + pagerAdapter.setupContainerPadding() + assertThat(container.paddingLeft).isEqualTo(1) + assertThat(container.paddingTop).isEqualTo(2) + assertThat(container.paddingRight).isEqualTo(3) + assertThat(container.paddingBottom).isEqualTo(4) } @Test fun testBottomPaddingDelegate_override() { - val container = - mock { - whenever(getPaddingLeft()).thenReturn(1) - whenever(getPaddingTop()).thenReturn(2) - whenever(getPaddingRight()).thenReturn(3) - whenever(getPaddingBottom()).thenReturn(4) - } + val personalListAdapter = + mock { whenever(getUserHandle()).thenReturn(PERSONAL_USER_HANDLE) } val pagerAdapter = MultiProfilePagerAdapter( { listAdapter: ResolverListAdapter -> listAdapter }, { listView: ListView, bindAdapter: ResolverListAdapter -> listView.setAdapter(bindAdapter) }, - ImmutableList.of(), + ImmutableList.of(personalListAdapter), object : EmptyStateProvider {}, { false }, PROFILE_PERSONAL, @@ -208,8 +203,16 @@ class MultiProfilePagerAdapterTest { inflater, { Optional.of(42) } ) - pagerAdapter.setupContainerPadding(container) - verify(container).setPadding(1, 2, 3, 42) + val container = + pagerAdapter + .getActiveEmptyStateView() + .requireViewById(com.android.internal.R.id.resolver_empty_state_container) + container.setPadding(1, 2, 3, 4) + pagerAdapter.setupContainerPadding() + assertThat(container.paddingLeft).isEqualTo(1) + assertThat(container.paddingTop).isEqualTo(2) + assertThat(container.paddingRight).isEqualTo(3) + assertThat(container.paddingBottom).isEqualTo(42) } @Test 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 12943cd7..27ed7e38 100644 --- a/java/tests/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelperTest.kt +++ b/java/tests/src/com/android/intentresolver/v2/emptystate/EmptyStateUiHelperTest.kt @@ -22,12 +22,20 @@ import android.view.ViewGroup import android.widget.FrameLayout import androidx.test.platform.app.InstrumentationRegistry import com.google.common.truth.Truth.assertThat +import java.util.Optional +import java.util.function.Supplier import org.junit.Before import org.junit.Test class EmptyStateUiHelperTest { private val context = InstrumentationRegistry.getInstrumentation().getContext() + var shouldOverrideContainerPadding = false + val containerPaddingSupplier = + Supplier> { + Optional.ofNullable(if (shouldOverrideContainerPadding) 42 else null) + } + lateinit var rootContainer: ViewGroup lateinit var emptyStateTitleView: View lateinit var emptyStateSubtitleView: View @@ -60,7 +68,7 @@ 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) + emptyStateUiHelper = EmptyStateUiHelper(rootContainer, containerPaddingSupplier) } @Test @@ -109,4 +117,30 @@ class EmptyStateUiHelperTest { assertThat(emptyStateRootView.visibility).isEqualTo(View.GONE) } + + @Test + fun testBottomPaddingDelegate_default() { + shouldOverrideContainerPadding = false + emptyStateContainerView.setPadding(1, 2, 3, 4) + + emptyStateUiHelper.setupContainerPadding() + + assertThat(emptyStateContainerView.paddingLeft).isEqualTo(1) + assertThat(emptyStateContainerView.paddingTop).isEqualTo(2) + assertThat(emptyStateContainerView.paddingRight).isEqualTo(3) + assertThat(emptyStateContainerView.paddingBottom).isEqualTo(4) + } + + @Test + fun testBottomPaddingDelegate_override() { + shouldOverrideContainerPadding = true // Set bottom padding to 42. + emptyStateContainerView.setPadding(1, 2, 3, 4) + + emptyStateUiHelper.setupContainerPadding() + + assertThat(emptyStateContainerView.paddingLeft).isEqualTo(1) + assertThat(emptyStateContainerView.paddingTop).isEqualTo(2) + assertThat(emptyStateContainerView.paddingRight).isEqualTo(3) + assertThat(emptyStateContainerView.paddingBottom).isEqualTo(42) + } } -- cgit v1.2.3-59-g8ed1b