Restored Add File buttons

- Disabled remove file button while removing
- Always keep add file buttons enabled

Author: davidvedvick

projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/browsing/files/details/FileDetailsView.kt

@@ -24,7 +24,6 @@ import androidx.compose.foundation.layout.heightIn
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.requiredHeight
 import androidx.compose.foundation.layout.requiredWidth
-import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.layout.systemBars
 import androidx.compose.foundation.layout.width
 import androidx.compose.foundation.lazy.LazyColumn
@@ -113,53 +112,56 @@ private val viewPadding = viewPaddingUnit
 private val maxMenuHeight = topMenuHeight + rowPadding
 
 @Composable
-private fun PlayableFileMenu(
+private fun PlayLastButton(
 	fileDetailsState: FileDetailsState,
-	mediaStylePalette: MediaStylePalette,
-	playableFileDetailsState: PlayableFileDetailsState?,
+	modifier: Modifier = Modifier,
 ) {
-	val modifier = Modifier.requiredWidth(topMenuIconWidth)
-	val isLoading by fileDetailsState.isLoading.subscribeAsState()
-
-	LabelledRefreshButton(
-		onClick = {
-			fileDetailsState.promiseLoadedActiveFile()
-		},
-		modifier = modifier,
-		enabled = !isLoading,
-	)
-
 	val addFileToPlaybackLabel = stringResource(id = R.string.btn_add_file_to_playback)
 	ColumnMenuIcon(
 		onClick = { fileDetailsState.addToNowPlaying() },
-		icon = {
-			Image(
-				painter = painterResource(id = R.drawable.playlist_plus),
-				colorFilter = ColorFilter.tint(mediaStylePalette.secondaryTextColor),
-				contentDescription = addFileToPlaybackLabel,
-				modifier = Modifier.size(topMenuIconSize),
-			)
-		},
+		iconPainter = painterResource(id = R.drawable.playlist_plus),
+		contentDescription = addFileToPlaybackLabel,
 		label = addFileToPlaybackLabel,
 		labelMaxLines = 1,
 		modifier = modifier,
 	)
+}
 
+@Composable
+private fun PlayNextButton(
+	fileDetailsState: FileDetailsState,
+	modifier: Modifier = Modifier,
+) {
 	val playFileNextPlaybackLabel = stringResource(id = R.string.btn_play_file_next)
 	ColumnMenuIcon(
 		onClick = { fileDetailsState.playNext() },
-		icon = {
-			Image(
-				painter = painterResource(id = R.drawable.playlist_inner_plus),
-				colorFilter = ColorFilter.tint(mediaStylePalette.secondaryTextColor),
-				contentDescription = playFileNextPlaybackLabel,
-				modifier = Modifier.size(topMenuIconSize),
-			)
-		},
+		iconPainter = painterResource(id = R.drawable.playlist_inner_plus),
+		contentDescription = playFileNextPlaybackLabel,
 		label = playFileNextPlaybackLabel,
 		labelMaxLines = 1,
 		modifier = modifier,
 	)
+}
+
+@Composable
+private fun PlayableFileMenu(
+	fileDetailsState: FileDetailsState,
+	playableFileDetailsState: PlayableFileDetailsState?,
+) {
+	val modifier = Modifier.requiredWidth(topMenuIconWidth)
+	val isLoading by fileDetailsState.isLoading.subscribeAsState()
+
+	LabelledRefreshButton(
+		onClick = {
+			fileDetailsState.promiseLoadedActiveFile()
+		},
+		modifier = modifier,
+		enabled = !isLoading,
+	)
+
+	PlayLastButton(fileDetailsState, modifier)
+
+	PlayNextButton(fileDetailsState, modifier)
 
 	if (playableFileDetailsState != null) {
 		val playLabel = stringResource(id = R.string.btn_play)
@@ -179,7 +181,7 @@ private fun PlayableFileMenu(
 private fun NowPlayingFileMenu(
 	fileDetailsState: FileDetailsState,
 	nowPlayingFileDetailsState: NowPlayingFileDetailsState,
-	playableFileDetailsState: PlayableFileDetailsState?,
+	playableFileDetailsState: PlayableFileDetailsState,
 ) {
 	val modifier = Modifier.requiredWidth(topMenuIconWidth)
 	val isLoading by fileDetailsState.isLoading.subscribeAsState()
@@ -193,6 +195,7 @@ private fun NowPlayingFileMenu(
 	)
 
 	val isInPosition by nowPlayingFileDetailsState.isInPosition.subscribeAsState()
+	val isRemoving by nowPlayingFileDetailsState.isRemoving.subscribeAsState()
 	val removeFileLabel = stringResource(id = R.string.btn_remove_file)
 	ColumnMenuIcon(
 		onClick = { nowPlayingFileDetailsState.removeFile() },
@@ -201,21 +204,23 @@ private fun NowPlayingFileMenu(
 		label = removeFileLabel,
 		labelMaxLines = 1,
 		modifier = modifier,
-		enabled = !isLoading && isInPosition
+		enabled = !isLoading && !isRemoving && isInPosition
 	)
 
-	if (playableFileDetailsState != null) {
-		val playLabel = stringResource(id = R.string.skip_to)
-		ColumnMenuIcon(
-			onClick = { playableFileDetailsState.play() },
-			iconPainter = painterResource(id = R.drawable.av_next_white),
-			contentDescription = playLabel,
-			label = playLabel,
-			labelMaxLines = 1,
-			modifier = Modifier.requiredWidth(topMenuIconWidth),
-			enabled = isInPosition
-		)
-	}
+	val playLabel = stringResource(id = R.string.skip_to)
+	ColumnMenuIcon(
+		onClick = { playableFileDetailsState.play() },
+		iconPainter = painterResource(id = R.drawable.av_next_white),
+		contentDescription = playLabel,
+		label = playLabel,
+		labelMaxLines = 1,
+		modifier = Modifier.requiredWidth(topMenuIconWidth),
+		enabled = isInPosition
+	)
+
+	PlayLastButton(fileDetailsState, modifier)
+
+	PlayNextButton(fileDetailsState, modifier)
 }
 
 @Composable
@@ -913,7 +918,6 @@ fun FileDetailsView(
 					menuIcons = {
 						PlayableFileMenu(
 							viewModel,
-							coverArtColorState,
 							playableFileDetailsState
 						)
 					}
@@ -925,7 +929,6 @@ fun FileDetailsView(
 					menuIcons = {
 						PlayableFileMenu(
 							viewModel,
-							coverArtColorState,
 							playableFileDetailsState,
 						)
 					}

projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/browsing/files/details/NowPlayingFileDetailsState.kt

@@ -1,8 +1,10 @@
 package com.lasthopesoftware.bluewater.client.browsing.files.details
 
 import com.lasthopesoftware.bluewater.shared.observables.InteractionState
+import com.namehillsoftware.handoff.promises.Promise
 
 interface NowPlayingFileDetailsState {
 	val isInPosition: InteractionState<Boolean>
-	fun removeFile()
+	fun removeFile(): Promise<Unit>
+	val isRemoving: InteractionState<Boolean>
 }

projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/browsing/files/details/NowPlayingFileDetailsViewModel.kt

@@ -42,9 +42,11 @@ class NowPlayingFileDetailsViewModel(
 		AutoCloseableManager().also(::addCloseable)
 	}
 
+	private val mutableIsRemoving = MutableInteractionState(false)
 	private val mutableIsLoading = MutableInteractionState(false)
 	private val mutableIsInPosition = MutableInteractionState(false)
 
+	override val isRemoving = mutableIsRemoving.asInteractionState()
 	override val isInPosition = mutableIsInPosition.asInteractionState()
 	override val isLoading = autoCloseableManager.manage(LiftedInteractionState(
 		Observable.combineLatest(listOf(mutableIsLoading.mapNotNull(), fileDetailsState.isLoading.mapNotNull())) { source -> source.any { loading -> loading as Boolean } },
@@ -66,11 +68,14 @@ class NowPlayingFileDetailsViewModel(
 			.must(this)
 	}
 
-	override fun removeFile() {
-		val libraryId = activeLibraryId ?: return
-		val positionedFile = activePositionedFile ?: return
+	override fun removeFile(): Promise<Unit> {
+		val libraryId = activeLibraryId ?: return Unit.toPromise()
+		val positionedFile = activePositionedFile ?: return Unit.toPromise()
 
-		controlPlayback.removeFromPlaylistAtPosition(libraryId, positionedFile.playlistPosition)
+		mutableIsRemoving.value = true
+		return controlPlayback
+			.removeFromPlaylistAtPosition(libraryId, positionedFile.playlistPosition)
+			.must { _ -> mutableIsRemoving.value = false }
 	}
 
 	override fun play() {

projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/playback/service/ControlPlaybackService.kt

@@ -30,7 +30,7 @@ interface ControlPlaybackService {
 
 	fun addAfterNowPlayingFile(libraryId: LibraryId, serviceFile: ServiceFile)
 
-	fun removeFromPlaylistAtPosition(libraryId: LibraryId, position: Int)
+	fun removeFromPlaylistAtPosition(libraryId: LibraryId, position: Int): Promise<Unit>
 
 	fun setRepeating(libraryId: LibraryId)
 

projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/playback/service/PlaybackService.kt

@@ -238,14 +238,13 @@ import java.util.concurrent.TimeoutException
 			)
 		}
 
-		fun removeFileAtPositionFromPlaylist(context: Context, libraryId: LibraryId, filePosition: Int) {
-			context.safelyStartService(
-				getNewSelfIntent(
-					context,
-					PlaybackEngineAction.RemoveFileAtPosition(libraryId, filePosition)
-				)
-			)
-		}
+		fun removeFileAtPositionFromPlaylist(context: Context, libraryId: LibraryId, filePosition: Int): Promise<Unit> =
+			context.promiseBoundService<PlaybackService>()
+				.eventually { h ->
+					h.service
+						.removeFileAtPosition(libraryId, filePosition)
+						.must { _ -> h.close() }
+				}
 
 		fun moveFile(context: Context, libraryId: LibraryId, filePosition: Int, newPosition: Int) {
 			context.safelyStartService(
@@ -669,25 +668,6 @@ import java.util.concurrent.TimeoutException
 							}
 						}
 				}
-				is PlaybackEngineAction.RemoveFileAtPosition -> {
-					val (libraryId, filePosition) = playbackEngineAction
-
-					restorePlaybackServices(libraryId)
-						.eventually { it.playlistFiles.removeFileAtPosition(filePosition) }
-						.then { _ ->
-							applicationMessageBus.sendMessage(LibraryPlaybackMessage.PlaylistChanged(libraryId))
-						}
-						.eventually {
-							mainLoopHandlerExecutor.preparePromise {
-								Toast.makeText(
-									this,
-									getText(R.string.lbl_song_removed_from_now_playing),
-									Toast.LENGTH_SHORT
-								).show()
-							}
-						}
-						.unitResponse()
-				}
 				is PlaybackEngineAction.MoveFile -> {
 					val (libraryId, filePosition, newPosition) = playbackEngineAction
 
@@ -833,6 +813,23 @@ import java.util.concurrent.TimeoutException
 
 	override fun onBind(intent: Intent): IBinder? = binder
 
+	private fun removeFileAtPosition(libraryId: LibraryId, playlistPosition: Int) =
+		restorePlaybackServices(libraryId)
+			.eventually { it.playlistFiles.removeFileAtPosition(playlistPosition) }
+			.then { _ ->
+				applicationMessageBus.sendMessage(LibraryPlaybackMessage.PlaylistChanged(libraryId))
+			}
+			.eventually {
+				mainLoopHandlerExecutor.preparePromise {
+					Toast.makeText(
+						this,
+						getText(R.string.lbl_song_removed_from_now_playing),
+						Toast.LENGTH_SHORT
+					).show()
+				}
+			}
+			.unitResponse()
+
 	private fun startNewPlaylist(libraryId: LibraryId, playlist: List<ServiceFile>, playlistPosition: Int): Promise<Unit> {
 		activeLibraryId = libraryId
 		val playbackState = promisedPlaybackServices.value
@@ -1092,12 +1089,6 @@ import java.util.concurrent.TimeoutException
 			override val requestCode = 9
 		}
 
-		@Parcelize
-		data class RemoveFileAtPosition(override val libraryId: LibraryId, val position: Int) : PlaybackEngineAction {
-			@IgnoredOnParcel
-			override val requestCode = 10
-		}
-
 		@Parcelize
 		data class MoveFile(override val libraryId: LibraryId, val from: Int, val to: Int) : PlaybackEngineAction {
 			@IgnoredOnParcel

projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/playback/service/PlaybackServiceController.kt

@@ -42,9 +42,8 @@ class PlaybackServiceController(private val context: Context) : ControlPlaybackS
 		PlaybackService.addAfterNowPlayingFile(context, libraryId, serviceFile)
 	}
 
-	override fun removeFromPlaylistAtPosition(libraryId: LibraryId, position: Int) {
+	override fun removeFromPlaylistAtPosition(libraryId: LibraryId, position: Int): Promise<Unit> =
 		PlaybackService.removeFileAtPositionFromPlaylist(context, libraryId, position)
-	}
 
 	override fun setRepeating(libraryId: LibraryId) = PlaybackService.setRepeating(context, libraryId)
 

projectBlueWater/src/test/java/com/lasthopesoftware/bluewater/client/browsing/files/details/GivenAFile/When removing the file from now playing.kt

@@ -34,6 +34,7 @@ class `When removing the file from now playing` {
 				every { removeFromPlaylistAtPosition(any(), any()) } answers {
 					removedLibraryId = firstArg()
 					removedPosition = lastArg()
+					Unit.toPromise()
 				}
 			},
 			mockk {
@@ -62,17 +63,20 @@ class `When removing the file from now playing` {
 	}
 
 	private var isLoadingStates = mutableListOf<Boolean>()
+	private var isRemovingStates = mutableListOf<Boolean>()
 	private var isInPositionStates = mutableListOf<Boolean>()
 
 	@BeforeAll
 	fun act() {
 		viewModel.isLoading.mapNotNull().subscribe(isLoadingStates::add).toCloseable().use {
-			viewModel.isInPosition.mapNotNull().subscribe(isInPositionStates::add).toCloseable().use {
-				viewModel
-					.load(LibraryId(libraryId), PositionedFile(501, ServiceFile(serviceFileId)))
-					.toExpiringFuture()
-					.get()
-				viewModel.removeFile()
+			viewModel.isRemoving.mapNotNull().subscribe(isRemovingStates::add).toCloseable().use {
+				viewModel.isInPosition.mapNotNull().subscribe(isInPositionStates::add).toCloseable().use {
+					viewModel
+						.load(LibraryId(libraryId), PositionedFile(501, ServiceFile(serviceFileId)))
+						.toExpiringFuture()
+						.get()
+					viewModel.removeFile().toExpiringFuture().get()
+				}
 			}
 		}
 	}
@@ -82,6 +86,17 @@ class `When removing the file from now playing` {
 		assertThat(isLoadingStates).isEqualTo(listOf(true))
 	}
 
+	@Test
+	fun `then is removing changes correctly`() {
+		assertThat(isRemovingStates).isEqualTo(
+			listOf(
+				false,
+				true,
+				false,
+			)
+		)
+	}
+
 	@Test
 	fun `then the file is in position changes correctly`() {
 		assertThat(isInPositionStates).isEqualTo(listOf(false))