Refactor ScreenFlash UI state

feature/preview/build.gradle.kts

@@ -116,6 +116,7 @@ dependencies {
     testImplementation(libs.mockito.core)
     testImplementation(libs.kotlinx.coroutines.test)
     testImplementation(libs.robolectric)
+    testImplementation(kotlin("test"))
     debugImplementation(libs.androidx.test.monitor)
     implementation(libs.androidx.junit)
     androidTestImplementation(libs.androidx.junit)

feature/preview/src/androidTest/java/com/google/jetpackcamera/feature/preview/ui/ScreenFlashComponentsKtTest.kt

@@ -29,7 +29,7 @@ import androidx.compose.ui.test.onRoot
 import androidx.compose.ui.unit.height
 import androidx.compose.ui.unit.width
 import androidx.test.ext.junit.runners.AndroidJUnit4
-import com.google.jetpackcamera.feature.preview.ScreenFlash
+import com.google.jetpackcamera.feature.preview.ScreenFlashUiState
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
 import org.junit.Before
@@ -42,8 +42,8 @@ class ScreenFlashComponentsKtTest {
     @get:Rule
     val composeTestRule = createComposeRule()
 
-    private val screenFlashUiState: MutableState<ScreenFlash.ScreenFlashUiState> =
-        mutableStateOf(ScreenFlash.ScreenFlashUiState())
+    private val screenFlashUiState: MutableState<ScreenFlashUiState> =
+        mutableStateOf(ScreenFlashUiState.NotApplied())
 
     @Before
     fun setUp() {
@@ -63,24 +63,24 @@ class ScreenFlashComponentsKtTest {
 
     @Test
     fun screenFlashOverlay_existsAfterStateIsEnabled() = runTest {
-        screenFlashUiState.value = ScreenFlash.ScreenFlashUiState(enabled = true)
+        screenFlashUiState.value = ScreenFlashUiState.Applied(onComplete = {})
 
         composeTestRule.awaitIdle()
         composeTestRule.onNode(hasTestTag("ScreenFlashOverlay")).assertExists()
     }
 
     @Test
     fun screenFlashOverlay_doesNotExistWhenDisabledAfterEnabled() = runTest {
-        screenFlashUiState.value = ScreenFlash.ScreenFlashUiState(enabled = true)
-        screenFlashUiState.value = ScreenFlash.ScreenFlashUiState(enabled = false)
+        screenFlashUiState.value = ScreenFlashUiState.Applied(onComplete = {})
+        screenFlashUiState.value = ScreenFlashUiState.NotApplied()
 
         composeTestRule.awaitIdle()
         composeTestRule.onNode(hasTestTag("ScreenFlashOverlay")).assertDoesNotExist()
     }
 
     @Test
     fun screenFlashOverlay_sizeFillsMaxSize() = runTest {
-        screenFlashUiState.value = ScreenFlash.ScreenFlashUiState(enabled = true)
+        screenFlashUiState.value = ScreenFlashUiState.Applied(onComplete = {})
 
         composeTestRule.awaitIdle()
         val rootBounds = composeTestRule.onRoot().getBoundsInRoot()
@@ -92,7 +92,7 @@ class ScreenFlashComponentsKtTest {
 
     @Test
     fun screenFlashOverlay_fullWhiteWhenEnabled() = runTest {
-        screenFlashUiState.value = ScreenFlash.ScreenFlashUiState(enabled = true)
+        screenFlashUiState.value = ScreenFlashUiState.Applied(onComplete = {})
 
         composeTestRule.awaitIdle()
         val overlayScreenShot =

feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt

@@ -71,7 +71,7 @@ fun PreviewScreen(
 
     val previewUiState: PreviewUiState by viewModel.previewUiState.collectAsState()
 
-    val screenFlashUiState: ScreenFlash.ScreenFlashUiState
+    val screenFlashUiState: ScreenFlashUiState
         by viewModel.screenFlash.screenFlashUiState.collectAsState()
 
     val surfaceRequest: SurfaceRequest?
@@ -113,7 +113,7 @@ fun PreviewScreen(
 private fun ContentScreen(
     previewUiState: PreviewUiState,
     previewMode: PreviewMode,
-    screenFlashUiState: ScreenFlash.ScreenFlashUiState,
+    screenFlashUiState: ScreenFlashUiState,
     surfaceRequest: SurfaceRequest?,
     onNavigateToSettings: () -> Unit = {},
     onClearUiScreenBrightness: (Float) -> Unit = {},
@@ -224,7 +224,7 @@ private fun ContentScreenPreview() {
         ContentScreen(
             previewUiState = PreviewUiState(),
             previewMode = PreviewMode.StandardMode {},
-            screenFlashUiState = ScreenFlash.ScreenFlashUiState(),
+            screenFlashUiState = ScreenFlashUiState.NotApplied(),
             surfaceRequest = null
         )
     }
@@ -239,7 +239,7 @@ private fun ContentScreen_WhileRecording() {
                 videoRecordingState = VideoRecordingState.ACTIVE
             ),
             previewMode = PreviewMode.StandardMode {},
-            screenFlashUiState = ScreenFlash.ScreenFlashUiState(),
+            screenFlashUiState = ScreenFlashUiState.NotApplied(),
             surfaceRequest = null
         )
     }

feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/ScreenFlash.kt

@@ -17,9 +17,10 @@ package com.google.jetpackcamera.feature.preview
 
 import com.google.jetpackcamera.domain.camera.CameraUseCase
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
-import kotlinx.coroutines.launch
+import kotlinx.coroutines.flow.map
+import kotlinx.coroutines.flow.stateIn
 
 private const val TAG = "ScreenFlash"
 
@@ -29,59 +30,37 @@ private const val TAG = "ScreenFlash"
 // TODO: Add this to ViewModelScoped so that it can be injected automatically. However, the current
 //  ViewModel and Hilt APIs probably don't support injecting the viewModelScope.
 class ScreenFlash(
-    private val cameraUseCase: CameraUseCase,
-    private val scope: CoroutineScope
+    cameraUseCase: CameraUseCase,
+    scope: CoroutineScope
 ) {
-    data class ScreenFlashUiState(
-        val enabled: Boolean = false,
-        val onChangeComplete: () -> Unit = {},
-        // restored during CLEAR_UI event
-        val screenBrightnessToRestore: Float? = null
-    )
+    private var screenBrightnessToRestore: Float? = null
 
-    private val _screenFlashUiState: MutableStateFlow<ScreenFlashUiState> =
-        MutableStateFlow(ScreenFlashUiState())
-    val screenFlashUiState: StateFlow<ScreenFlashUiState> = _screenFlashUiState
-
-    init {
-        scope.launch {
-            cameraUseCase.getScreenFlashEvents().collect { event ->
-                _screenFlashUiState.emit(
-                    when (event.type) {
-                        CameraUseCase.ScreenFlashEvent.Type.APPLY_UI ->
-                            screenFlashUiState.value.copy(
-                                enabled = true,
-                                onChangeComplete = event.onComplete
-                            )
-
-                        CameraUseCase.ScreenFlashEvent.Type.CLEAR_UI ->
-                            screenFlashUiState.value.copy(
-                                enabled = false,
-                                onChangeComplete = {
-                                    event.onComplete()
-                                    // reset ui state on CLEAR_UI event completion
-                                    scope.launch {
-                                        _screenFlashUiState.emit(
-                                            ScreenFlashUiState()
-                                        )
-                                    }
-                                }
-                            )
-                    }
-                )
+    val screenFlashUiState: StateFlow<ScreenFlashUiState> =
+        cameraUseCase.getScreenFlashEvents().map { event ->
+            when (event.type) {
+                CameraUseCase.ScreenFlashEvent.Type.APPLY_UI ->
+                    ScreenFlashUiState.Applied(onComplete = event.onComplete)
+                CameraUseCase.ScreenFlashEvent.Type.CLEAR_UI -> {
+                    ScreenFlashUiState.NotApplied(
+                        screenBrightnessToRestore = screenBrightnessToRestore
+                    )
+                }
             }
-        }
-    }
+        }.stateIn(
+            scope = scope,
+            started = SharingStarted.WhileSubscribed(5_000),
+            initialValue = ScreenFlashUiState.NotApplied()
+        )
 
     /**
-     * Sets the screenBrightness value to the value right before APPLY_UI event for the next
-     * CLEAR_UI event, will be set to unknown (null) again after CLEAR_UI event is completed.
+     * Set the screen brightness to restore to after a screen flash has been applied.
      */
     fun setClearUiScreenBrightness(brightness: Float) {
-        scope.launch {
-            _screenFlashUiState.emit(
-                screenFlashUiState.value.copy(screenBrightnessToRestore = brightness)
-            )
-        }
+        screenBrightnessToRestore = brightness
     }
 }
+
+sealed interface ScreenFlashUiState {
+    data class Applied(val onComplete: () -> Unit) : ScreenFlashUiState
+    data class NotApplied(val screenBrightnessToRestore: Float? = null) : ScreenFlashUiState
+}

feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/ui/ScreenFlashComponents.kt

@@ -32,42 +32,47 @@ import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.platform.testTag
-import com.google.jetpackcamera.feature.preview.ScreenFlash
+import com.google.jetpackcamera.feature.preview.ScreenFlashUiState
 
 private const val TAG = "ScreenFlashComponents"
 
 @Composable
 fun ScreenFlashScreen(
-    screenFlashUiState: ScreenFlash.ScreenFlashUiState,
+    screenFlashUiState: ScreenFlashUiState,
     onInitialBrightnessCalculated: (Float) -> Unit
 ) {
     ScreenFlashOverlay(screenFlashUiState)
 
-    if (screenFlashUiState.enabled) {
-        BrightnessMaximization(onInitialBrightnessCalculated = onInitialBrightnessCalculated)
-    } else {
-        screenFlashUiState.screenBrightnessToRestore?.let {
+    when (screenFlashUiState) {
+        is ScreenFlashUiState.Applied -> {
+            BrightnessMaximization(onInitialBrightnessCalculated = onInitialBrightnessCalculated)
+        }
+        is ScreenFlashUiState.NotApplied -> {
             // non-null brightness value means there is a value to restore
-            BrightnessRestoration(
-                brightness = it
-            )
+            screenFlashUiState.screenBrightnessToRestore?.let {
+                BrightnessRestoration(brightness = it)
+            }
         }
     }
 }
 
 @Composable
-fun ScreenFlashOverlay(screenFlashUiState: ScreenFlash.ScreenFlashUiState) {
+fun ScreenFlashOverlay(screenFlashUiState: ScreenFlashUiState) {
     // Update overlay transparency gradually
     val alpha by animateFloatAsState(
-        targetValue = if (screenFlashUiState.enabled) 1f else 0f,
+        targetValue = if (screenFlashUiState is ScreenFlashUiState.Applied) 1f else 0f,
         label = "screenFlashAlphaAnimation",
         animationSpec = tween(),
-        finishedListener = { screenFlashUiState.onChangeComplete() }
+        finishedListener = {
+            if (screenFlashUiState is ScreenFlashUiState.Applied) {
+                screenFlashUiState.onComplete()
+            }
+        }
     )
     Box(
         modifier = Modifier
             .run {
-                if (screenFlashUiState.enabled) {
+                if (screenFlashUiState is ScreenFlashUiState.Applied) {
                     this.testTag(SCREEN_FLASH_OVERLAY)
                 } else {
                     this

feature/preview/src/test/java/com/google/jetpackcamera/feature/preview/ScreenFlashTest.kt

@@ -22,6 +22,7 @@ import com.google.jetpackcamera.domain.camera.test.FakeCameraUseCase
 import com.google.jetpackcamera.feature.preview.rules.MainDispatcherRule
 import com.google.jetpackcamera.settings.model.FlashMode
 import com.google.jetpackcamera.settings.model.LensFacing
+import kotlin.test.assertIs
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.toList
 import kotlinx.coroutines.launch
@@ -49,19 +50,21 @@ class ScreenFlashTest {
     @Before
     fun setup() = runTest(testDispatcher) {
         screenFlash = ScreenFlash(cameraUseCase, testScope)
+        advanceUntilIdle() // Ensure that the subject under test has finished initializing.
     }
 
     @Test
-    fun initialScreenFlashUiState_disabledByDefault() {
-        assertThat(screenFlash.screenFlashUiState.value.enabled).isFalse()
+    fun initialScreenFlashUiState_notAppliedByDefault() {
+        assertIs<ScreenFlashUiState.NotApplied>(screenFlash.screenFlashUiState.value)
     }
 
     @Test
     fun captureScreenFlashImage_screenFlashUiStateChangedInCorrectSequence() = runCameraTest {
-        val states = mutableListOf<ScreenFlash.ScreenFlashUiState>()
+        val states = mutableListOf<ScreenFlashUiState>()
         backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) {
             screenFlash.screenFlashUiState.toList(states)
         }
+        advanceUntilIdle()
 
         // FlashMode.ON in front facing camera automatically enables screen flash
         cameraUseCase.setLensFacing(lensFacing = LensFacing.FRONT)
@@ -70,7 +73,7 @@ class ScreenFlashTest {
         cameraUseCase.takePicture(contentResolver, null)
 
         advanceUntilIdle()
-        assertThat(states.map { it.enabled }).containsExactlyElementsIn(
+        assertThat(states.map { it is ScreenFlashUiState.Applied }).containsExactlyElementsIn(
             listOf(
                 false,
                 true,
@@ -82,37 +85,26 @@ class ScreenFlashTest {
     @Test
     fun emitClearUiEvent_screenFlashUiStateContainsClearUiScreenBrightness() = runCameraTest {
         screenFlash.setClearUiScreenBrightness(5.0f)
-        cameraUseCase.emitScreenFlashEvent(
-            CameraUseCase.ScreenFlashEvent(CameraUseCase.ScreenFlashEvent.Type.CLEAR_UI) { }
-        )
 
-        advanceUntilIdle()
-        assertThat(screenFlash.screenFlashUiState.value.screenBrightnessToRestore)
-            .isWithin(FLOAT_TOLERANCE)
-            .of(5.0f)
-    }
+        backgroundScope.launch {
+            screenFlash.screenFlashUiState.collect { state ->
+                assertIs<ScreenFlashUiState.NotApplied>(state)
+                assertThat(state.screenBrightnessToRestore)
+                    .isWithin(FLOAT_TOLERANCE)
+                    .of(5.0f)
+            }
+        }
 
-    @Test
-    fun invokeOnChangeCompleteAfterClearUiEvent_screenFlashUiStateReset() = runCameraTest {
-        screenFlash.setClearUiScreenBrightness(5.0f)
         cameraUseCase.emitScreenFlashEvent(
             CameraUseCase.ScreenFlashEvent(CameraUseCase.ScreenFlashEvent.Type.CLEAR_UI) { }
         )
-
-        advanceUntilIdle()
-        screenFlash.screenFlashUiState.value.onChangeComplete()
-
-        advanceUntilIdle()
-        assertThat(ScreenFlash.ScreenFlashUiState())
-            .isEqualTo(screenFlash.screenFlashUiState.value)
     }
 
     private fun runCameraTest(testBody: suspend TestScope.() -> Unit) = runTest(testDispatcher) {
         backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) {
             cameraUseCase.initialize()
             cameraUseCase.runCamera()
         }
-
         testBody()
     }