From fd3b1ce2985ae2f0117e9e793cab8e2aa12b4169 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Fri, 2 Oct 2026 12:52:06 +0000 Subject: [PATCH] address review: keep notification taps working after recreation --- .../main/kotlin/com/subtracks/MainActivity.kt | 9 ++++- .../main/kotlin/com/subtracks/ui/AppRoot.kt | 13 +++---- .../com/subtracks/NowPlayingLaunchTest.kt | 37 +++++++++++++++++++ .../com/subtracks/ui/NowPlayingRequestTest.kt | 33 ----------------- 4 files changed, 50 insertions(+), 42 deletions(-) create mode 100644 app/src/test/kotlin/com/subtracks/NowPlayingLaunchTest.kt delete mode 100644 app/src/test/kotlin/com/subtracks/ui/NowPlayingRequestTest.kt diff --git a/app/src/main/kotlin/com/subtracks/MainActivity.kt b/app/src/main/kotlin/com/subtracks/MainActivity.kt index 3492a808..52c10d24 100644 --- a/app/src/main/kotlin/com/subtracks/MainActivity.kt +++ b/app/src/main/kotlin/com/subtracks/MainActivity.kt @@ -24,7 +24,9 @@ class MainActivity : ComponentActivity() { super.onCreate(savedInstanceState) requestNotificationPermission() enableEdgeToEdge() - if (intent?.action == ACTION_OPEN_NOW_PLAYING) nowPlayingRequest.intValue++ + if (shouldRecordNowPlayingLaunch(intent?.action, restoringState = savedInstanceState != null)) { + nowPlayingRequest.intValue++ + } setContent { SubtracksTheme { SubtracksRoot(nowPlayingRequest = nowPlayingRequest.intValue) @@ -50,3 +52,8 @@ class MainActivity : ComponentActivity() { const val ACTION_OPEN_NOW_PLAYING = "com.subtracks.action.OPEN_NOW_PLAYING" } } + +internal fun shouldRecordNowPlayingLaunch( + action: String?, + restoringState: Boolean, +): Boolean = action == MainActivity.ACTION_OPEN_NOW_PLAYING && !restoringState diff --git a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt index fa252080..71553387 100644 --- a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt +++ b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt @@ -166,12 +166,6 @@ internal fun resetOnSourceSwitch( if (route in DETAIL_ROUTES) popDetail() } -internal fun shouldOpenNowPlaying( - request: Int, - handled: Int, - playerVisible: Boolean, -): Boolean = playerVisible && request > handled - internal data class BackStackKey( val route: String?, val argument: String?, @@ -227,7 +221,7 @@ private fun MainNavigation(nowPlayingRequest: Int) { val downloads by downloadRepository.states().collectAsStateWithLifecycle() var showingQueue by rememberSaveable { mutableStateOf(false) } var nowPlayingOpen by rememberSaveable { mutableStateOf(false) } - var handledNowPlayingRequest by rememberSaveable { mutableStateOf(0) } + var handledNowPlayingRequest by remember { mutableStateOf(0) } var nowPlayingProgress by remember { mutableFloatStateOf(0f) } var miniPlayerTopPx by remember { mutableFloatStateOf(0f) } var nowPlayingFadeOut by remember { mutableStateOf(false) } @@ -313,8 +307,11 @@ private fun MainNavigation(nowPlayingRequest: Int) { } } LaunchedEffect(nowPlayingRequest, playerVisible) { - if (shouldOpenNowPlaying(nowPlayingRequest, handledNowPlayingRequest, playerVisible)) { + if (playerVisible && nowPlayingRequest > handledNowPlayingRequest) { handledNowPlayingRequest = nowPlayingRequest + if (navController.currentDestination?.route in DETAIL_ROUTES) { + navController.popBackStack(Routes.LIBRARY, inclusive = false) + } settleNowPlaying(true) } } diff --git a/app/src/test/kotlin/com/subtracks/NowPlayingLaunchTest.kt b/app/src/test/kotlin/com/subtracks/NowPlayingLaunchTest.kt new file mode 100644 index 00000000..90a4c9fa --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/NowPlayingLaunchTest.kt @@ -0,0 +1,37 @@ +package com.subtracks + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class NowPlayingLaunchTest { + @Test + fun aFreshLaunchFromTheNotificationIsRecorded() { + assertTrue( + shouldRecordNowPlayingLaunch( + action = MainActivity.ACTION_OPEN_NOW_PLAYING, + restoringState = false, + ), + ) + } + + @Test + fun recreatingTheActivityDoesNotReRecordTheLaunch() { + assertFalse( + shouldRecordNowPlayingLaunch( + action = MainActivity.ACTION_OPEN_NOW_PLAYING, + restoringState = true, + ), + ) + } + + @Test + fun aNormalLaunchIsNotRecorded() { + assertFalse(shouldRecordNowPlayingLaunch(action = "android.intent.action.MAIN", restoringState = false)) + } + + @Test + fun aLaunchWithoutAnActionIsNotRecorded() { + assertFalse(shouldRecordNowPlayingLaunch(action = null, restoringState = false)) + } +} diff --git a/app/src/test/kotlin/com/subtracks/ui/NowPlayingRequestTest.kt b/app/src/test/kotlin/com/subtracks/ui/NowPlayingRequestTest.kt deleted file mode 100644 index 4812e788..00000000 --- a/app/src/test/kotlin/com/subtracks/ui/NowPlayingRequestTest.kt +++ /dev/null @@ -1,33 +0,0 @@ -package com.subtracks.ui - -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Test - -class NowPlayingRequestTest { - @Test - fun aNotificationWithNothingPlayingDoesNotOpen() { - assertFalse(shouldOpenNowPlaying(request = 1, handled = 0, playerVisible = false)) - } - - @Test - fun aNotificationWithSomethingPlayingOpens() { - assertTrue(shouldOpenNowPlaying(request = 1, handled = 0, playerVisible = true)) - } - - @Test - fun anAlreadyHandledRequestDoesNotReopen() { - assertFalse(shouldOpenNowPlaying(request = 1, handled = 1, playerVisible = true)) - } - - @Test - fun aNewerRequestOpensAgain() { - assertTrue(shouldOpenNowPlaying(request = 2, handled = 1, playerVisible = true)) - } - - @Test - fun anOldRequestWaitsUntilSomethingIsPlaying() { - assertFalse(shouldOpenNowPlaying(request = 1, handled = 0, playerVisible = false)) - assertTrue(shouldOpenNowPlaying(request = 1, handled = 0, playerVisible = true)) - } -}