From 5eb4f21fee0f7245c38d1594fa6fdb1908ab875e Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 8 Mar 2023 16:01:52 +0000 Subject: [PATCH 1/4] [Chipbar] Re-name the dimens to be chipbar instead of media_ttt. Bug: 245610654 Test: Triggered each of the media ttt chips and verified they still appeared correctly Change-Id: Ia2441b383c33ddbd182636f4bcfb36b67a4c77e7 --- ..._background.xml => chipbar_background.xml} | 0 ....xml => chipbar_end_button_background.xml} | 0 packages/SystemUI/res/layout/chipbar.xml | 41 +++++++++---------- packages/SystemUI/res/values/dimens.xml | 17 ++++---- 4 files changed, 29 insertions(+), 29 deletions(-) rename packages/SystemUI/res/drawable/{media_ttt_chip_background.xml => chipbar_background.xml} (100%) rename packages/SystemUI/res/drawable/{media_ttt_undo_background.xml => chipbar_end_button_background.xml} (100%) diff --git a/packages/SystemUI/res/drawable/media_ttt_chip_background.xml b/packages/SystemUI/res/drawable/chipbar_background.xml similarity index 100% rename from packages/SystemUI/res/drawable/media_ttt_chip_background.xml rename to packages/SystemUI/res/drawable/chipbar_background.xml diff --git a/packages/SystemUI/res/drawable/media_ttt_undo_background.xml b/packages/SystemUI/res/drawable/chipbar_end_button_background.xml similarity index 100% rename from packages/SystemUI/res/drawable/media_ttt_undo_background.xml rename to packages/SystemUI/res/drawable/chipbar_end_button_background.xml diff --git a/packages/SystemUI/res/layout/chipbar.xml b/packages/SystemUI/res/layout/chipbar.xml index 0ff944c2becf5..9d26657a45352 100644 --- a/packages/SystemUI/res/layout/chipbar.xml +++ b/packages/SystemUI/res/layout/chipbar.xml @@ -29,8 +29,8 @@ android:orientation="horizontal" android:layout_width="wrap_content" android:layout_height="wrap_content" - android:padding="@dimen/media_ttt_chip_outer_padding" - android:background="@drawable/media_ttt_chip_background" + android:padding="@dimen/chipbar_outer_padding" + android:background="@drawable/chipbar_background" android:layout_marginTop="20dp" android:layout_marginStart="@dimen/notification_side_paddings" android:layout_marginEnd="@dimen/notification_side_paddings" @@ -43,8 +43,8 @@ @@ -54,7 +54,7 @@ android:layout_width="0dp" android:layout_height="wrap_content" android:layout_weight="1" - android:textSize="@dimen/media_ttt_text_size" + android:textSize="@dimen/chipbar_text_size" android:textColor="?android:attr/textColorPrimary" android:alpha="0.0" /> @@ -62,9 +62,9 @@ - diff --git a/packages/SystemUI/res/values/dimens.xml b/packages/SystemUI/res/values/dimens.xml index 3b8f1a7d1e4d9..607c13dadb2c6 100644 --- a/packages/SystemUI/res/values/dimens.xml +++ b/packages/SystemUI/res/values/dimens.xml @@ -1111,14 +1111,15 @@ 22dp 12dp - - 16dp - 16sp - 24dp - 20dp - 8dp - -8dp - 12dp + + + 16dp + 16sp + 24dp + 20dp + 8dp + -8dp + 12dp 112dp From e7d87a4e256e9c9c12e5af8b88b8a00ea9d86140 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 8 Mar 2023 15:45:28 +0000 Subject: [PATCH 2/4] [Chipbar] Update the colors to make the chipbar stand out more. Bug: 267821684 Test: Trigger chipbar -> see new colors (screenshots in bug) Test: Change theme while chipbar is displayed -> verify chipbar is re-displayed in the new theme color Change-Id: I86ff54f931d63f5d8d44e85a8b1f1a4e7c238171 --- packages/SystemUI/res/drawable/chipbar_background.xml | 2 +- .../SystemUI/res/drawable/chipbar_end_button_background.xml | 2 +- packages/SystemUI/res/layout/chipbar.xml | 6 +++--- packages/SystemUI/res/values/colors.xml | 1 + .../temporarydisplay/TemporaryViewDisplayController.kt | 4 ++++ 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/SystemUI/res/drawable/chipbar_background.xml b/packages/SystemUI/res/drawable/chipbar_background.xml index 3abf4d72c5386..57221776a32ff 100644 --- a/packages/SystemUI/res/drawable/chipbar_background.xml +++ b/packages/SystemUI/res/drawable/chipbar_background.xml @@ -17,6 +17,6 @@ - + diff --git a/packages/SystemUI/res/drawable/chipbar_end_button_background.xml b/packages/SystemUI/res/drawable/chipbar_end_button_background.xml index 3e2e4f055b14e..80c7207a35b61 100644 --- a/packages/SystemUI/res/drawable/chipbar_end_button_background.xml +++ b/packages/SystemUI/res/drawable/chipbar_end_button_background.xml @@ -20,7 +20,7 @@ android:color="?android:textColorPrimary"> - + diff --git a/packages/SystemUI/res/layout/chipbar.xml b/packages/SystemUI/res/layout/chipbar.xml index 9d26657a45352..a3171784903f8 100644 --- a/packages/SystemUI/res/layout/chipbar.xml +++ b/packages/SystemUI/res/layout/chipbar.xml @@ -55,7 +55,7 @@ android:layout_height="wrap_content" android:layout_weight="1" android:textSize="@dimen/chipbar_text_size" - android:textColor="?android:attr/textColorPrimary" + android:textColor="@android:color/system_accent2_900" android:alpha="0.0" /> @@ -66,7 +66,7 @@ android:layout_height="@dimen/chipbar_end_icon_size" android:layout_marginStart="@dimen/chipbar_end_item_start_margin" android:src="@drawable/ic_progress_activity" - android:tint="?androidprv:attr/colorAccentPrimaryVariant" + android:tint="@android:color/system_accent2_700" android:alpha="0.0" /> @@ -76,7 +76,7 @@ android:layout_height="@dimen/chipbar_end_icon_size" android:layout_marginStart="@dimen/chipbar_end_item_start_margin" android:src="@drawable/ic_warning" - android:tint="@color/GM2_red_500" + android:tint="@color/GM2_red_600" android:alpha="0.0" /> diff --git a/packages/SystemUI/res/values/colors.xml b/packages/SystemUI/res/values/colors.xml index ca4217f64b600..285017c014b11 100644 --- a/packages/SystemUI/res/values/colors.xml +++ b/packages/SystemUI/res/values/colors.xml @@ -161,6 +161,7 @@ #F28B82 #EA4335 + #B3261E #C5221F #8AB4F8 diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt index 1065d3349930d..c6cb79a6a8802 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt @@ -294,6 +294,10 @@ abstract class TemporaryViewDisplayController Date: Wed, 8 Mar 2023 17:55:46 +0000 Subject: [PATCH 3/4] [Media TTT] Align the timeout with heads up notifications. This reduces the timeout to 5s by default, and 10s for "in progress" actions like loading. The 5s timeout is the same as the heads up notification timeout, since the UIs are in similar places doing similar things. Fixes: 271302820 Test: verify loading chip disappears after 5s Test: verify success chip disappears after 10s Test: atest MediaTttSenderCoordinatorTest Change-Id: I6bf500c55102f2339cd112fc373ba85a88f93d61 --- .../taptotransfer/sender/ChipStateSender.kt | 32 ++++--- .../sender/MediaTttSenderCoordinator.kt | 11 ++- .../sender/MediaTttSenderCoordinatorTest.kt | 87 +++++++++++++++++++ 3 files changed, 119 insertions(+), 11 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/ChipStateSender.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/ChipStateSender.kt index 537dbb93826e5..d3efae427cb02 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/ChipStateSender.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/ChipStateSender.kt @@ -23,7 +23,6 @@ import androidx.annotation.StringRes import com.android.internal.logging.UiEventLogger import com.android.systemui.R import com.android.systemui.common.shared.model.Text -import com.android.systemui.temporarydisplay.DEFAULT_TIMEOUT_MILLIS /** * A class enumerating all the possible states of the media tap-to-transfer chip on the sender @@ -34,8 +33,8 @@ import com.android.systemui.temporarydisplay.DEFAULT_TIMEOUT_MILLIS * state should not have the chip be displayed. * @property transferStatus the transfer status that the chip state represents. * @property endItem the item that should be displayed in the end section of the chip. - * @property timeout the amount of time this chip should display on the screen before it times out - * and disappears. + * @property timeoutLength how long the chip should display on the screen before it times out and + * disappears. */ enum class ChipStateSender( @StatusBarManager.MediaTransferSenderState val stateInt: Int, @@ -43,7 +42,7 @@ enum class ChipStateSender( @StringRes val stringResId: Int?, val transferStatus: TransferStatus, val endItem: SenderEndItem?, - val timeout: Int = DEFAULT_TIMEOUT_MILLIS, + val timeoutLength: TimeoutLength = TimeoutLength.DEFAULT, ) { /** * A state representing that the two devices are close but not close enough to *start* a cast to @@ -56,6 +55,9 @@ enum class ChipStateSender( R.string.media_move_closer_to_start_cast, transferStatus = TransferStatus.NOT_STARTED, endItem = null, + // Give this view more time in case the loading view takes a bit to come in. (We don't want + // this view to disappear and then the loading view to appear quickly afterwards.) + timeoutLength = TimeoutLength.LONG, ) { override fun isValidNextState(nextState: ChipStateSender): Boolean { return nextState == FAR_FROM_RECEIVER || @@ -75,6 +77,7 @@ enum class ChipStateSender( R.string.media_move_closer_to_end_cast, transferStatus = TransferStatus.NOT_STARTED, endItem = null, + timeoutLength = TimeoutLength.LONG, ) { override fun isValidNextState(nextState: ChipStateSender): Boolean { return nextState == FAR_FROM_RECEIVER || @@ -92,7 +95,9 @@ enum class ChipStateSender( R.string.media_transfer_playing_different_device, transferStatus = TransferStatus.IN_PROGRESS, endItem = SenderEndItem.Loading, - timeout = TRANSFER_TRIGGERED_TIMEOUT_MILLIS + // Give this view more time in case the succeeded/failed view takes a bit to come in. (We + // don't want this view to disappear and then the next view to appear quickly afterwards.) + timeoutLength = TimeoutLength.LONG, ) { override fun isValidNextState(nextState: ChipStateSender): Boolean { return nextState == FAR_FROM_RECEIVER || @@ -111,7 +116,7 @@ enum class ChipStateSender( R.string.media_transfer_playing_this_device, transferStatus = TransferStatus.IN_PROGRESS, endItem = SenderEndItem.Loading, - timeout = TRANSFER_TRIGGERED_TIMEOUT_MILLIS + timeoutLength = TimeoutLength.LONG, ) { override fun isValidNextState(nextState: ChipStateSender): Boolean { return nextState == FAR_FROM_RECEIVER || @@ -325,9 +330,16 @@ sealed class SenderEndItem { ) : SenderEndItem() } -// Give the Transfer*Triggered states a longer timeout since those states represent an active -// process and we should keep the user informed about it as long as possible (but don't allow it to -// continue indefinitely). -private const val TRANSFER_TRIGGERED_TIMEOUT_MILLIS = 30000 +/** Represents how long the chip should be visible before it times out. */ +enum class TimeoutLength { + /** A default timeout used for temporary displays at the top of the screen. */ + DEFAULT, + /** + * A longer timeout. Should be used when the status is pending (e.g. loading), so that the user + * remains informed about the process for longer and so that the UI has more time to resolve the + * pending state before disappearing. + */ + LONG, +} private const val TAG = "ChipStateSender" diff --git a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinator.kt b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinator.kt index 6bb6906a0dfc9..c7c72a9cd1203 100644 --- a/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinator.kt +++ b/packages/SystemUI/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinator.kt @@ -56,6 +56,9 @@ constructor( private val uiEventLogger: MediaTttSenderUiEventLogger, ) : CoreStartable, Dumpable { + // Since the media transfer display is similar to a heads-up notification, use the same timeout. + private val defaultTimeout = context.resources.getInteger(R.integer.heads_up_notification_decay) + // A map to store current chip state per id. private var stateMap: MutableMap = mutableMapOf() @@ -165,6 +168,12 @@ constructor( logger.logPackageNotFound(packageName) } + val timeout = + when (chipStateSender.timeoutLength) { + TimeoutLength.DEFAULT -> defaultTimeout + TimeoutLength.LONG -> 2 * defaultTimeout + } + return ChipbarInfo( // Display the app's icon as the start icon startIcon = icon.toTintedIcon(), @@ -191,7 +200,7 @@ constructor( allowSwipeToDismiss = true, windowTitle = MediaTttUtils.WINDOW_TITLE_SENDER, wakeReason = MediaTttUtils.WAKE_REASON_SENDER, - timeoutMs = chipStateSender.timeout, + timeoutMs = timeout, id = routeInfo.id, priority = ViewPriority.NORMAL, ) diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt index db890f6e739be..ca2b1da34766f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/media/taptotransfer/sender/MediaTttSenderCoordinatorTest.kt @@ -107,6 +107,7 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { private lateinit var fakeExecutor: FakeExecutor private lateinit var uiEventLoggerFake: UiEventLoggerFake private lateinit var uiEventLogger: MediaTttSenderUiEventLogger + private val defaultTimeout = context.resources.getInteger(R.integer.heads_up_notification_decay) @Before fun setUp() { @@ -1356,6 +1357,92 @@ class MediaTttSenderCoordinatorTest : SysuiTestCase() { assertThat(chipbarView.getLoadingIcon().visibility).isEqualTo(View.VISIBLE) } + @Test + fun almostClose_hasLongTimeout_eventuallyTimesOut() { + whenever(accessibilityManager.getRecommendedTimeoutMillis(any(), any())).thenAnswer { + it.arguments[0] + } + + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_ALMOST_CLOSE_TO_START_CAST, + routeInfo, + null, + ) + + // WHEN the default timeout has passed + fakeClock.advanceTime(defaultTimeout + 1L) + + // THEN the view is still on-screen because it has a long timeout + verify(windowManager, never()).removeView(any()) + + // WHEN a very long amount of time has passed + fakeClock.advanceTime(5L * defaultTimeout) + + // THEN the view does time out + verify(windowManager).removeView(any()) + } + + @Test + fun loading_hasLongTimeout_eventuallyTimesOut() { + whenever(accessibilityManager.getRecommendedTimeoutMillis(any(), any())).thenAnswer { + it.arguments[0] + } + + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_THIS_DEVICE_TRIGGERED, + routeInfo, + null, + ) + + // WHEN the default timeout has passed + fakeClock.advanceTime(defaultTimeout + 1L) + + // THEN the view is still on-screen because it has a long timeout + verify(windowManager, never()).removeView(any()) + + // WHEN a very long amount of time has passed + fakeClock.advanceTime(5L * defaultTimeout) + + // THEN the view does time out + verify(windowManager).removeView(any()) + } + + @Test + fun succeeded_hasDefaultTimeout() { + whenever(accessibilityManager.getRecommendedTimeoutMillis(any(), any())).thenAnswer { + it.arguments[0] + } + + displayReceiverTriggered() + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_RECEIVER_SUCCEEDED, + routeInfo, + null, + ) + + fakeClock.advanceTime(defaultTimeout + 1L) + + verify(windowManager).removeView(any()) + } + + @Test + fun failed_hasDefaultTimeout() { + whenever(accessibilityManager.getRecommendedTimeoutMillis(any(), any())).thenAnswer { + it.arguments[0] + } + + displayThisDeviceTriggered() + commandQueueCallback.updateMediaTapToTransferSenderDisplay( + StatusBarManager.MEDIA_TRANSFER_SENDER_STATE_TRANSFER_TO_THIS_DEVICE_FAILED, + routeInfo, + null, + ) + + fakeClock.advanceTime(defaultTimeout + 1L) + + verify(windowManager).removeView(any()) + } + private fun getChipbarView(): ViewGroup { val viewCaptor = ArgumentCaptor.forClass(View::class.java) verify(windowManager).addView(viewCaptor.capture(), any()) From 6595af030870f3215ea7ed07700bd0c204188a02 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 8 Mar 2023 21:07:06 +0000 Subject: [PATCH 4/4] [Chipbar] Add logs at the exact time WindowManager calls are made. Bug: 271411294 Test: Verified logs occurred at the correct times when the chipbar appeared and disappered Change-Id: I3c1f0ebb72407ff4b2a073ea927f5a0ab0e71250 --- .../TemporaryViewDisplayController.kt | 7 +++ .../temporarydisplay/TemporaryViewLogger.kt | 43 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt index c6cb79a6a8802..59122aff7f216 100644 --- a/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt +++ b/packages/SystemUI/src/com/android/systemui/temporarydisplay/TemporaryViewDisplayController.kt @@ -274,6 +274,7 @@ abstract class TemporaryViewDisplayController( { "Removal of view with id=$str2 is ignored because $str1" } ) } + + fun logViewAddedToWindowManager(info: T, view: View) { + buffer.log( + tag, + LogLevel.DEBUG, + { + str1 = info.id + str2 = info.windowTitle + str3 = view.javaClass.name + int1 = view.getIdForLogging() + }, + { + "Adding view to window manager. " + + "id=$str1 window=$str2 view=$str3(id=${Integer.toHexString(int1)})" + } + ) + } + + fun logViewRemovedFromWindowManager(info: T, view: View, isReinflation: Boolean = false) { + buffer.log( + tag, + LogLevel.DEBUG, + { + str1 = info.id + str2 = info.windowTitle + str3 = view.javaClass.name + int1 = view.getIdForLogging() + bool1 = isReinflation + }, + { + "Removing view from window manager${if (bool1) " due to reinflation" else ""}. " + + "id=$str1 window=$str2 view=$str3(id=${Integer.toHexString(int1)})" + } + ) + } + + companion object { + private fun View.getIdForLogging(): Int { + // The identityHashCode is guaranteed to be constant for the lifetime of the object. + return System.identityHashCode(this) + } + } }