From b3cad9786905de3c70bafdf8169a10345e5eb7aa Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sat, 3 Oct 2026 02:58:18 +0000 Subject: [PATCH] address review: signal now playing with an event flow, not an int --- .../main/kotlin/com/subtracks/MainActivity.kt | 11 ++++---- .../main/kotlin/com/subtracks/di/AppModule.kt | 2 ++ .../main/kotlin/com/subtracks/ui/AppRoot.kt | 17 +++++------- .../com/subtracks/ui/NowPlayingLauncher.kt | 15 +++++++++++ .../subtracks/ui/NowPlayingLauncherTest.kt | 27 +++++++++++++++++++ 5 files changed, 57 insertions(+), 15 deletions(-) create mode 100644 app/src/main/kotlin/com/subtracks/ui/NowPlayingLauncher.kt create mode 100644 app/src/test/kotlin/com/subtracks/ui/NowPlayingLauncherTest.kt diff --git a/app/src/main/kotlin/com/subtracks/MainActivity.kt b/app/src/main/kotlin/com/subtracks/MainActivity.kt index 52c10d24..408efeb2 100644 --- a/app/src/main/kotlin/com/subtracks/MainActivity.kt +++ b/app/src/main/kotlin/com/subtracks/MainActivity.kt @@ -9,13 +9,14 @@ import androidx.activity.ComponentActivity import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge import androidx.activity.result.contract.ActivityResultContracts -import androidx.compose.runtime.mutableIntStateOf import androidx.core.content.ContextCompat +import com.subtracks.ui.NowPlayingLauncher import com.subtracks.ui.SubtracksRoot import com.subtracks.ui.theme.SubtracksTheme +import org.koin.core.context.GlobalContext class MainActivity : ComponentActivity() { - private val nowPlayingRequest = mutableIntStateOf(0) + private val nowPlayingLauncher: NowPlayingLauncher by lazy { GlobalContext.get().get() } private val requestNotifications = registerForActivityResult(ActivityResultContracts.RequestPermission()) { } @@ -25,11 +26,11 @@ class MainActivity : ComponentActivity() { requestNotificationPermission() enableEdgeToEdge() if (shouldRecordNowPlayingLaunch(intent?.action, restoringState = savedInstanceState != null)) { - nowPlayingRequest.intValue++ + nowPlayingLauncher.request() } setContent { SubtracksTheme { - SubtracksRoot(nowPlayingRequest = nowPlayingRequest.intValue) + SubtracksRoot() } } } @@ -37,7 +38,7 @@ class MainActivity : ComponentActivity() { override fun onNewIntent(intent: Intent) { super.onNewIntent(intent) setIntent(intent) - if (intent.action == ACTION_OPEN_NOW_PLAYING) nowPlayingRequest.intValue++ + if (intent.action == ACTION_OPEN_NOW_PLAYING) nowPlayingLauncher.request() } private fun requestNotificationPermission() { diff --git a/app/src/main/kotlin/com/subtracks/di/AppModule.kt b/app/src/main/kotlin/com/subtracks/di/AppModule.kt index 29c9fced..f2c9d21d 100644 --- a/app/src/main/kotlin/com/subtracks/di/AppModule.kt +++ b/app/src/main/kotlin/com/subtracks/di/AppModule.kt @@ -28,6 +28,7 @@ import com.subtracks.playback.MediaSessionConnection import com.subtracks.playback.PlaybackController import com.subtracks.playback.PlayerConnection import com.subtracks.playback.Scrobbler +import com.subtracks.ui.NowPlayingLauncher import com.subtracks.ui.RootViewModel import com.subtracks.ui.downloads.DownloadsViewModel import com.subtracks.ui.home.HomeListViewModel @@ -108,6 +109,7 @@ fun appModule( it.attach(playback.state, playback.positionMs) } } + single { NowPlayingLauncher() } viewModel { RootViewModel(get()) } viewModel { LibraryViewModel(get(), get(), get(), get(), get(), get()) } viewModel { HomeViewModel(get(), get(), get(), get()) } diff --git a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt index 71553387..61d8c8b3 100644 --- a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt +++ b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt @@ -181,10 +181,7 @@ internal fun focusPops( } @Composable -fun SubtracksRoot( - root: RootViewModel = koinViewModel(), - nowPlayingRequest: Int = 0, -) { +fun SubtracksRoot(root: RootViewModel = koinViewModel()) { when (val hasSource = root.hasSource.collectAsStateWithLifecycle().value) { null -> { LoadingState() @@ -198,14 +195,14 @@ fun SubtracksRoot( val playbackController = koinInject() LaunchedEffect(Unit) { playbackController.connect() } val playbackReady by playbackController.ready.collectAsStateWithLifecycle() - if (playbackReady) MainNavigation(nowPlayingRequest) else LoadingState() + if (playbackReady) MainNavigation() else LoadingState() } } } @OptIn(ExperimentalLayoutApi::class) @Composable -private fun MainNavigation(nowPlayingRequest: Int) { +private fun MainNavigation() { val navController = rememberNavController() val currentRoute = navController @@ -215,13 +212,13 @@ private fun MainNavigation(nowPlayingRequest: Int) { ?.route val tabBarVisible = currentRoute == Routes.LIBRARY val playbackController = koinInject() + val nowPlayingLauncher = koinInject() val libraryRepository = koinInject() val downloadRepository = koinInject() val playback by playbackController.state.collectAsStateWithLifecycle() val downloads by downloadRepository.states().collectAsStateWithLifecycle() var showingQueue by rememberSaveable { mutableStateOf(false) } var nowPlayingOpen by rememberSaveable { mutableStateOf(false) } - var handledNowPlayingRequest by remember { mutableStateOf(0) } var nowPlayingProgress by remember { mutableFloatStateOf(0f) } var miniPlayerTopPx by remember { mutableFloatStateOf(0f) } var nowPlayingFadeOut by remember { mutableStateOf(false) } @@ -306,9 +303,9 @@ private fun MainNavigation(nowPlayingRequest: Int) { nowPlayingProgress = 0f } } - LaunchedEffect(nowPlayingRequest, playerVisible) { - if (playerVisible && nowPlayingRequest > handledNowPlayingRequest) { - handledNowPlayingRequest = nowPlayingRequest + LaunchedEffect(playerVisible) { + if (!playerVisible) return@LaunchedEffect + nowPlayingLauncher.openNowPlaying.collect { if (navController.currentDestination?.route in DETAIL_ROUTES) { navController.popBackStack(Routes.LIBRARY, inclusive = false) } diff --git a/app/src/main/kotlin/com/subtracks/ui/NowPlayingLauncher.kt b/app/src/main/kotlin/com/subtracks/ui/NowPlayingLauncher.kt new file mode 100644 index 00000000..d3777514 --- /dev/null +++ b/app/src/main/kotlin/com/subtracks/ui/NowPlayingLauncher.kt @@ -0,0 +1,15 @@ +package com.subtracks.ui + +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.receiveAsFlow + +class NowPlayingLauncher { + private val requests = Channel(Channel.CONFLATED) + + val openNowPlaying: Flow = requests.receiveAsFlow() + + fun request() { + requests.trySend(Unit) + } +} diff --git a/app/src/test/kotlin/com/subtracks/ui/NowPlayingLauncherTest.kt b/app/src/test/kotlin/com/subtracks/ui/NowPlayingLauncherTest.kt new file mode 100644 index 00000000..fdc64d16 --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/ui/NowPlayingLauncherTest.kt @@ -0,0 +1,27 @@ +package com.subtracks.ui + +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withTimeoutOrNull +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class NowPlayingLauncherTest { + @Test + fun aRequestMadeBeforeCollectionIsDelivered() = + runTest { + val launcher = NowPlayingLauncher() + launcher.request() + assertEquals(Unit, launcher.openNowPlaying.first()) + } + + @Test + fun repeatedRequestsCollapseToTheLatest() = + runTest { + val launcher = NowPlayingLauncher() + repeat(3) { launcher.request() } + assertEquals(Unit, launcher.openNowPlaying.first()) + assertNull(withTimeoutOrNull(100) { launcher.openNowPlaying.first() }) + } +}