From f701928157260ea998a3091f3efeb5b42a26e2a6 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 28 Sep 2022 20:32:32 +0000 Subject: [PATCH 1/3] [Media TTT] Add the background back to the receiver chip. Fixes: 245979191 Test: manual: See video commented on bug Test: media.taptotransfer tests Change-Id: I13c60b3b950ac5081766664582490e5b46283c5e --- .../res/layout/media_ttt_chip_receiver.xml | 1 + packages/SystemUI/res/values/dimens.xml | 5 +-- .../taptotransfer/common/MediaTttUtils.kt | 24 ---------- .../MediaTttChipControllerReceiver.kt | 18 ++++---- .../sender/MediaTttChipControllerSender.kt | 9 ++-- .../taptotransfer/common/MediaTttUtilsTest.kt | 44 ------------------- .../MediaTttChipControllerReceiverTest.kt | 36 ++++++--------- 7 files changed, 29 insertions(+), 108 deletions(-) diff --git a/packages/SystemUI/res/layout/media_ttt_chip_receiver.xml b/packages/SystemUI/res/layout/media_ttt_chip_receiver.xml index e079fd3c5e8f0..21d12c2784532 100644 --- a/packages/SystemUI/res/layout/media_ttt_chip_receiver.xml +++ b/packages/SystemUI/res/layout/media_ttt_chip_receiver.xml @@ -29,6 +29,7 @@ 100dp 95dp - - 70dp + + 12dp 20dp diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttUtils.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttUtils.kt index 792ae7ca60491..c3de94f28aea1 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttUtils.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/common/MediaTttUtils.kt @@ -19,7 +19,6 @@ package com.android.systemui.media.taptotransfer.common import android.content.Context import android.content.pm.PackageManager import android.graphics.drawable.Drawable -import com.android.internal.widget.CachingIconView import com.android.settingslib.Utils import com.android.systemui.R @@ -76,29 +75,6 @@ class MediaTttUtils { isAppIcon = false ) } - - /** - * Sets an icon to be displayed by the given view. - * - * @param iconSize the size in pixels that the icon should be. If null, the size of - * [appIconView] will not be adjusted. - */ - fun setIcon( - appIconView: CachingIconView, - icon: Drawable, - iconContentDescription: CharSequence, - iconSize: Int? = null, - ) { - iconSize?.let { size -> - val lp = appIconView.layoutParams - lp.width = size - lp.height = size - appIconView.layoutParams = lp - } - - appIconView.contentDescription = iconContentDescription - appIconView.setImageDrawable(icon) - } } } diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiver.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiver.kt index dfd9e22c14b14..8fc5519cc73e7 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiver.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiver.kt @@ -30,6 +30,7 @@ import android.view.View import android.view.ViewGroup import android.view.WindowManager import android.view.accessibility.AccessibilityManager +import com.android.internal.widget.CachingIconView import com.android.settingslib.Utils import com.android.systemui.R import com.android.systemui.dagger.SysUISingleton @@ -146,20 +147,17 @@ class MediaTttChipControllerReceiver @Inject constructor( ) val iconDrawable = newInfo.appIconDrawableOverride ?: iconInfo.drawable val iconContentDescription = newInfo.appNameOverride ?: iconInfo.contentDescription - val iconSize = context.resources.getDimensionPixelSize( + val iconPadding = if (iconInfo.isAppIcon) { - R.dimen.media_ttt_icon_size_receiver + 0 } else { - R.dimen.media_ttt_generic_icon_size_receiver + context.resources.getDimensionPixelSize(R.dimen.media_ttt_generic_icon_padding) } - ) - MediaTttUtils.setIcon( - currentView.requireViewById(R.id.app_icon), - iconDrawable, - iconContentDescription, - iconSize, - ) + val iconView = currentView.requireViewById(R.id.app_icon) + iconView.setPadding(iconPadding, iconPadding, iconPadding, iconPadding) + iconView.setImageDrawable(iconDrawable) + iconView.contentDescription = iconContentDescription } override fun animateViewIn(view: ViewGroup) { diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt index 007eb8f8deee1..a487b1d295bc6 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt @@ -29,6 +29,7 @@ import android.view.WindowManager import android.view.accessibility.AccessibilityManager import android.widget.TextView import com.android.internal.statusbar.IUndoMediaTransferCallback +import com.android.internal.widget.CachingIconView import com.android.systemui.Gefingerpoken import com.android.systemui.R import com.android.systemui.animation.Interpolators @@ -145,11 +146,9 @@ class MediaTttChipControllerSender @Inject constructor( val iconInfo = MediaTttUtils.getIconInfoFromPackageName( context, newInfo.routeInfo.clientPackageName, logger ) - MediaTttUtils.setIcon( - currentView.requireViewById(R.id.app_icon), - iconInfo.drawable, - iconInfo.contentDescription - ) + val iconView = currentView.requireViewById(R.id.app_icon) + iconView.setImageDrawable(iconInfo.drawable) + iconView.contentDescription = iconInfo.contentDescription // Text val otherDeviceName = newInfo.routeInfo.name.toString() diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/common/MediaTttUtilsTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/common/MediaTttUtilsTest.kt index 37f6434ea0699..7c83cb74bb773 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/common/MediaTttUtilsTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/common/MediaTttUtilsTest.kt @@ -19,9 +19,7 @@ package com.android.systemui.media.taptotransfer.common import android.content.pm.ApplicationInfo import android.content.pm.PackageManager import android.graphics.drawable.Drawable -import android.widget.FrameLayout import androidx.test.filters.SmallTest -import com.android.internal.widget.CachingIconView import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.util.mockito.any @@ -90,48 +88,6 @@ class MediaTttUtilsTest : SysuiTestCase() { assertThat(iconInfo.drawable).isEqualTo(appIconFromPackageName) assertThat(iconInfo.contentDescription).isEqualTo(APP_NAME) } - - @Test - fun setIcon_viewHasIconAndContentDescription() { - val view = CachingIconView(context) - val icon = context.getDrawable(R.drawable.ic_celebration)!! - val contentDescription = "Happy birthday!" - - MediaTttUtils.setIcon(view, icon, contentDescription) - - assertThat(view.drawable).isEqualTo(icon) - assertThat(view.contentDescription).isEqualTo(contentDescription) - } - - @Test - fun setIcon_iconSizeNull_viewSizeDoesNotChange() { - val view = CachingIconView(context) - val size = 456 - view.layoutParams = FrameLayout.LayoutParams(size, size) - - MediaTttUtils.setIcon(view, context.getDrawable(R.drawable.ic_cake)!!, "desc") - - assertThat(view.layoutParams.width).isEqualTo(size) - assertThat(view.layoutParams.height).isEqualTo(size) - } - - @Test - fun setIcon_iconSizeProvided_viewSizeUpdates() { - val view = CachingIconView(context) - val size = 456 - view.layoutParams = FrameLayout.LayoutParams(size, size) - - val newSize = 40 - MediaTttUtils.setIcon( - view, - context.getDrawable(R.drawable.ic_cake)!!, - "desc", - iconSize = newSize - ) - - assertThat(view.layoutParams.width).isEqualTo(newSize) - assertThat(view.layoutParams.height).isEqualTo(newSize) - } } private const val PACKAGE_NAME = "com.android.systemui" diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiverTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiverTest.kt index d41ad48676b41..775dc11f6edd8 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiverTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/receiver/MediaTttChipControllerReceiverTest.kt @@ -212,35 +212,27 @@ class MediaTttChipControllerReceiverTest : SysuiTestCase() { } @Test - fun updateView_isAppIcon_usesAppIconSize() { + fun updateView_isAppIcon_usesAppIconPadding() { controllerReceiver.displayView(getChipReceiverInfo(packageName = PACKAGE_NAME)) + val chipView = getChipView() - - chipView.measure( - View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED), - View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED) - ) - - val expectedSize = - context.resources.getDimensionPixelSize(R.dimen.media_ttt_icon_size_receiver) - assertThat(chipView.getAppIconView().measuredWidth).isEqualTo(expectedSize) - assertThat(chipView.getAppIconView().measuredHeight).isEqualTo(expectedSize) + assertThat(chipView.getAppIconView().paddingLeft).isEqualTo(0) + assertThat(chipView.getAppIconView().paddingRight).isEqualTo(0) + assertThat(chipView.getAppIconView().paddingTop).isEqualTo(0) + assertThat(chipView.getAppIconView().paddingBottom).isEqualTo(0) } @Test - fun updateView_notAppIcon_usesGenericIconSize() { + fun updateView_notAppIcon_usesGenericIconPadding() { controllerReceiver.displayView(getChipReceiverInfo(packageName = null)) + val chipView = getChipView() - - chipView.measure( - View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED), - View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED) - ) - - val expectedSize = - context.resources.getDimensionPixelSize(R.dimen.media_ttt_generic_icon_size_receiver) - assertThat(chipView.getAppIconView().measuredWidth).isEqualTo(expectedSize) - assertThat(chipView.getAppIconView().measuredHeight).isEqualTo(expectedSize) + val expectedPadding = + context.resources.getDimensionPixelSize(R.dimen.media_ttt_generic_icon_padding) + assertThat(chipView.getAppIconView().paddingLeft).isEqualTo(expectedPadding) + assertThat(chipView.getAppIconView().paddingRight).isEqualTo(expectedPadding) + assertThat(chipView.getAppIconView().paddingTop).isEqualTo(expectedPadding) + assertThat(chipView.getAppIconView().paddingBottom).isEqualTo(expectedPadding) } @Test From f6c00ca9340f7d88f2270cce4d16064a3daf55d5 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Thu, 29 Sep 2022 03:10:09 +0000 Subject: [PATCH 2/3] [Chipbar] Define #shouldIgnoreViewRemoval as a specific API on the TemporaryViewDisplayController, instead of having subclasses just override #removeView. Bug: 245610654 Test: manual: Send a TRANSFER_TRIGGERED event then FAR_FROM_RECEIVER event and verify the chip is still displayed (i.e. the removal request is ignored) Test: MediaTttChipControllerSenderTest Test: TemporaryViewDisplayControllerTest Change-Id: I2cb907c5fb563c80c8a5720ae7f70a70e115bd2e --- .../sender/MediaTttChipControllerSender.kt | 6 ++--- .../TemporaryViewDisplayController.kt | 13 +++++++++- .../TemporaryViewDisplayControllerTest.kt | 26 +++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt index a487b1d295bc6..71389f55d360b 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt @@ -195,7 +195,7 @@ class MediaTttChipControllerSender @Inject constructor( ) } - override fun removeView(removalReason: String) { + override fun shouldIgnoreViewRemoval(removalReason: String): Boolean { // Don't remove the chip if we're in progress or succeeded, since the user should still be // able to see the status of the transfer. (But do remove it if it's finally timed out.) val transferStatus = info?.state?.transferStatus @@ -207,9 +207,9 @@ class MediaTttChipControllerSender @Inject constructor( logger.logRemovalBypass( removalReason, bypassReason = "transferStatus=${transferStatus.name}" ) - return + return true } - super.removeView(removalReason) + return false } private fun Boolean.visibleIfTrue(): Int { diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt index a52e2aff52c18..dc9a683c83c2a 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt @@ -167,7 +167,11 @@ abstract class TemporaryViewDisplayController Date: Thu, 29 Sep 2022 01:40:43 +0000 Subject: [PATCH 3/3] [Media TTT] Animate the sender chip out. This also updates ViewHierarchyAnimator to have different behavior depending on whether the view in question has siblings or not. Bug: 203800644 Test: manual: See video attached to bug. Test: media.taptotransfer tests Test: ViewHierarchyAnimatorTest Change-Id: I61bfb975de5a46e0ab490a5eec468ec141937e75 --- .../animation/ViewHierarchyAnimator.kt | 40 ++++++--- .../sender/MediaTttChipControllerSender.kt | 14 ++- .../TemporaryViewDisplayController.kt | 20 ++++- .../animation/ViewHierarchyAnimatorTest.kt | 85 ++++++++++++++++++- .../MediaTttChipControllerSenderTest.kt | 38 ++++++++- 5 files changed, 179 insertions(+), 18 deletions(-) diff --git a/packages/SystemUI/animation/src/com/android/systemui/animation/ViewHierarchyAnimator.kt b/packages/SystemUI/animation/src/com/android/systemui/animation/ViewHierarchyAnimator.kt index dc2c63561d79c..1b7e26b0aea09 100644 --- a/packages/SystemUI/animation/src/com/android/systemui/animation/ViewHierarchyAnimator.kt +++ b/packages/SystemUI/animation/src/com/android/systemui/animation/ViewHierarchyAnimator.kt @@ -361,13 +361,17 @@ class ViewHierarchyAnimator { * * The end state of the animation is controlled by [destination]. This value can be any of * the four corners, any of the four edges, or the center of the view. + * + * @param onAnimationEnd an optional runnable that will be run once the animation finishes + * successfully. Will not be run if the animation is cancelled. */ @JvmOverloads fun animateRemoval( rootView: View, destination: Hotspot = Hotspot.CENTER, interpolator: Interpolator = DEFAULT_REMOVAL_INTERPOLATOR, - duration: Long = DEFAULT_DURATION + duration: Long = DEFAULT_DURATION, + onAnimationEnd: Runnable? = null, ): Boolean { if ( !occupiesSpace( @@ -391,13 +395,28 @@ class ViewHierarchyAnimator { addListener(child, listener, recursive = false) } - // Remove the view so that a layout update is triggered for the siblings and they - // animate to their next position while the view's removal is also animating. - parent.removeView(rootView) - // By adding the view to the overlay, we can animate it while it isn't part of the view - // hierarchy. It is correctly positioned because we have its previous bounds, and we set - // them manually during the animation. - parent.overlay.add(rootView) + val viewHasSiblings = parent.childCount > 1 + if (viewHasSiblings) { + // Remove the view so that a layout update is triggered for the siblings and they + // animate to their next position while the view's removal is also animating. + parent.removeView(rootView) + // By adding the view to the overlay, we can animate it while it isn't part of the + // view hierarchy. It is correctly positioned because we have its previous bounds, + // and we set them manually during the animation. + parent.overlay.add(rootView) + } + // If this view has no siblings, the parent view may shrink to (0,0) size and mess + // up the animation if we immediately remove the view. So instead, we just leave the + // view in the real hierarchy until the animation finishes. + + val endRunnable = Runnable { + if (viewHasSiblings) { + parent.overlay.remove(rootView) + } else { + parent.removeView(rootView) + } + onAnimationEnd?.run() + } val startValues = mapOf( @@ -430,7 +449,8 @@ class ViewHierarchyAnimator { endValues, interpolator, duration, - ephemeral = true + ephemeral = true, + endRunnable, ) if (rootView is ViewGroup) { @@ -463,7 +483,6 @@ class ViewHierarchyAnimator { .alpha(0f) .setInterpolator(Interpolators.ALPHA_OUT) .setDuration(duration / 2) - .withEndAction { parent.overlay.remove(rootView) } .start() } } @@ -477,7 +496,6 @@ class ViewHierarchyAnimator { .setInterpolator(Interpolators.ALPHA_OUT) .setDuration(duration / 2) .setStartDelay(duration / 2) - .withEndAction { parent.overlay.remove(rootView) } .start() } diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt index 71389f55d360b..11c55285c00b3 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttChipControllerSender.kt @@ -54,7 +54,7 @@ import javax.inject.Inject * chip is shown when a user is transferring media to/from this device and a receiver device. */ @SysUISingleton -class MediaTttChipControllerSender @Inject constructor( +open class MediaTttChipControllerSender @Inject constructor( commandQueue: CommandQueue, context: Context, @MediaTttSenderLogger logger: MediaTttLogger, @@ -195,6 +195,18 @@ class MediaTttChipControllerSender @Inject constructor( ) } + override fun animateViewOut(view: ViewGroup, onAnimationEnd: Runnable) { + ViewHierarchyAnimator.animateRemoval( + view.requireViewById(R.id.media_ttt_sender_chip_inner), + ViewHierarchyAnimator.Hotspot.TOP, + Interpolators.EMPHASIZED_ACCELERATE, + ANIMATION_DURATION, + onAnimationEnd, + ) + // TODO(b/203800644): Add includeMargins as an option to ViewHierarchyAnimator so that the + // animateChipOut matches the animateChipIn. + } + override fun shouldIgnoreViewRemoval(removalReason: String): Boolean { // Don't remove the chip if we're in progress or succeeded, since the user should still be // able to see the status of the transfer. (But do remove it if it's finally timed out.) diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt index dc9a683c83c2a..91e20ee309762 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt @@ -171,11 +171,15 @@ abstract class TemporaryViewDisplayController, + falsingCollector: Lazy, + ) : MediaTttChipControllerSender( + commandQueue, + context, + logger, + windowManager, + mainExecutor, + accessibilityManager, + configurationController, + powerManager, + uiEventLogger, + falsingManager, + falsingCollector, + ) { + override fun animateViewOut(view: ViewGroup, onAnimationEnd: Runnable) { + // Just bypass the animation in tests + onAnimationEnd.run() + } + } } private const val APP_NAME = "Fake app name"