bottom sheet refactor

.gemini/styleguide.md

@@ -41,6 +41,7 @@ When reviewing a pull request, focus on the following key areas:
     * Verify that Compose and CameraX APIs are used correctly and effectively.
     * Suggest more idiomatic or updated API usages where applicable.
     * Ensure state management in Compose is handled correctly (e.g., using `remember`, `derivedStateOf`, etc.).
+    * **Avoid Window-Spawning Overlays (`ModalBottomSheet`):** Do NOT use `ModalBottomSheet` on the camera capture screen. Modal bottom sheets spawn a separate Android `DialogWindow` above the main window, which can disrupt hardware-accelerated zero-copy rendering over the CameraX `SurfaceView` and create window lifecycle/gesture conflicts. Instead, use in-hierarchy containers like `BottomSheetScaffold` with `sheetPeekHeight = 0.dp`.
 
 5.  **Testing Coverage**
     * **When Tests are Missing:** If a PR introduces a significant feature or modifies logic without corresponding tests, flag this omission. Suggest a name for a new test class (e.g., `NewFeatureViewModelTest`) and outline what it should verify (e.g., "This test should check that the UI state updates correctly when the user performs X action").

app/src/androidTest/java/com/google/jetpackcamera/NavigationTest.kt

@@ -15,6 +15,7 @@
  */
 package com.google.jetpackcamera
 
+import androidx.compose.ui.test.assertIsNotDisplayed
 import androidx.compose.ui.test.isEnabled
 import androidx.compose.ui.test.junit4.createEmptyComposeRule
 import androidx.compose.ui.test.onNodeWithTag
@@ -117,7 +118,7 @@ class NavigationTest {
         composeTestRule.onNodeWithTag(CAPTURE_BUTTON).assertExists()
 
         // Assert bottom sheet is not open
-        composeTestRule.onNodeWithTag(QUICK_SETTINGS_BOTTOM_SHEET).assertDoesNotExist()
+        composeTestRule.onNodeWithTag(QUICK_SETTINGS_BOTTOM_SHEET).assertIsNotDisplayed()
     }
 
     @Test

app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt

@@ -46,10 +46,8 @@ import androidx.compose.ui.test.performScrollTo
 import androidx.compose.ui.test.performTouchInput
 import androidx.compose.ui.test.printToString
 import androidx.test.core.app.ApplicationProvider
-import androidx.test.espresso.action.ViewActions.swipeDown
 import com.google.common.truth.Truth.assertThat
 import com.google.errorprone.annotations.CanIgnoreReturnValue
-import com.google.jetpackcamera.core.common.ignoreResult
 import com.google.jetpackcamera.model.CaptureMode
 import com.google.jetpackcamera.model.ConcurrentCameraMode
 import com.google.jetpackcamera.model.FlashMode
@@ -74,6 +72,7 @@ import com.google.jetpackcamera.ui.components.capture.CAPTURE_MODE_TOGGLE_BUTTON
 import com.google.jetpackcamera.ui.components.capture.ELAPSED_TIME_TAG
 import com.google.jetpackcamera.ui.components.capture.FLIP_CAMERA_BUTTON
 import com.google.jetpackcamera.ui.components.capture.QUICK_SETTINGS_BOTTOM_SHEET
+import com.google.jetpackcamera.ui.components.capture.QUICK_SETTINGS_DRAG_HANDLE
 import com.google.jetpackcamera.ui.components.capture.R as CaptureR
 import com.google.jetpackcamera.ui.components.capture.ROW_QUICK_SETTINGS_ASPECT_RATIO
 import com.google.jetpackcamera.ui.components.capture.ROW_QUICK_SETTINGS_CAPTURE_MODE
@@ -194,7 +193,16 @@ fun ComposeTestRule.waitForNodeWithTagToDisappear(
     timeoutMillis: Long = DEFAULT_TIMEOUT_MILLIS
 ) {
     waitUntil(timeoutMillis = timeoutMillis) {
-        onNodeWithTag(tag).isNotDisplayed()
+        val nodes = onAllNodesWithTag(tag).fetchSemanticsNodes()
+        if (nodes.isEmpty()) {
+            true
+        } else {
+            try {
+                onNodeWithTag(tag).isNotDisplayed()
+            } catch (_: AssertionError) {
+                true
+            }
+        }
     }
 }
 
@@ -646,6 +654,24 @@ inline fun <T> SettingsScreenScope.visitSettingDialog(
 //
 // ////////////////////////////
 
+/**
+ * Closes the quick settings bottom sheet by clicking the drag handle pill.
+ */
+fun ComposeTestRule.closeQuickSettings(timeoutMillis: Long = DEFAULT_TIMEOUT_MILLIS) {
+    val dragHandleNodes = onAllNodesWithTag(QUICK_SETTINGS_DRAG_HANDLE).fetchSemanticsNodes()
+    if (dragHandleNodes.isNotEmpty()) {
+        onNodeWithTag(QUICK_SETTINGS_DRAG_HANDLE).performClick()
+    } else {
+        val openToggle =
+            onNodeWithContentDescription(CaptureR.string.quick_settings_toggle_open_description)
+        if (openToggle.isDisplayed()) {
+            openToggle.performClick()
+        }
+    }
+
+    waitForNodeWithTagToDisappear(QUICK_SETTINGS_BOTTOM_SHEET, timeoutMillis)
+}
+
 /**
  * Navigates to quick settings if not already there and perform action from provided block.
  * This will return from quick settings if not already there, or remain on quick settings if there.
@@ -680,31 +706,7 @@ inline fun <T> ComposeTestRule.visitQuickSettings(
         return block()
     } finally {
         if (needReturnFromQuickSettings) {
-            val bottomSheetNode = onNodeWithTag(QUICK_SETTINGS_BOTTOM_SHEET)
-            // Check if the bottom sheet content exists and is visible
-
-            if (bottomSheetNode.isDisplayed()) {
-                // It's visible, so perform the swipe down
-                bottomSheetNode.performTouchInput {
-                    down(center)
-                    swipeDown().ignoreResult()
-                    up()
-                }
-
-                // Assert that the sheet is no longer visible (e.g., the text disappears)
-                waitUntil(timeoutMillis = DEFAULT_TIMEOUT_MILLIS) {
-                    onNodeWithTag(QUICK_SETTINGS_BOTTOM_SHEET).isNotDisplayed()
-                }
-            } else {
-                Log.d(
-                    "ComposeTestRuleExt",
-                    "Bottom sheet with tag $QUICK_SETTINGS_BOTTOM_SHEET is not visible. Skipping quick settings closure."
-                )
-            }
-
-            waitUntil(timeoutMillis = DEFAULT_TIMEOUT_MILLIS) {
-                onNodeWithTag(QUICK_SETTINGS_BOTTOM_SHEET).isNotDisplayed()
-            }
+            closeQuickSettings()
         }
     }
 }

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

@@ -19,6 +19,7 @@ import android.Manifest
 import android.os.Build
 import android.util.Log
 import android.util.Range
+import androidx.activity.compose.BackHandler
 import androidx.camera.core.SurfaceRequest
 import androidx.compose.animation.AnimatedVisibility
 import androidx.compose.animation.EnterTransition
@@ -33,12 +34,16 @@ import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.size
+import androidx.compose.material3.BottomSheetScaffoldState
 import androidx.compose.material3.CircularProgressIndicator
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.material3.MaterialTheme
+import androidx.compose.material3.SheetValue
 import androidx.compose.material3.SnackbarHostState
 import androidx.compose.material3.Text
 import androidx.compose.material3.darkColorScheme
+import androidx.compose.material3.rememberBottomSheetScaffoldState
+import androidx.compose.material3.rememberStandardBottomSheetState
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.collectAsState
@@ -48,6 +53,7 @@ import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.rememberUpdatedState
+import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.runtime.setValue
 import androidx.compose.runtime.snapshotFlow
 import androidx.compose.ui.Alignment
@@ -96,7 +102,7 @@ import com.google.jetpackcamera.ui.components.capture.VideoQualityIcon
 import com.google.jetpackcamera.ui.components.capture.ZoomButtonRow
 import com.google.jetpackcamera.ui.components.capture.ZoomStateManager
 import com.google.jetpackcamera.ui.components.capture.debouncedOrientationFlow
-import com.google.jetpackcamera.ui.components.capture.quicksettings.QuickSettingsBottomSheet
+import com.google.jetpackcamera.ui.components.capture.quicksettings.QuickSettingsScaffoldContent
 import com.google.jetpackcamera.ui.components.capture.quicksettings.ui.FlashModeIndicator
 import com.google.jetpackcamera.ui.components.capture.quicksettings.ui.HdrIndicator
 import com.google.jetpackcamera.ui.components.capture.quicksettings.ui.ToggleQuickSettingsButton
@@ -119,7 +125,6 @@ import com.google.jetpackcamera.ui.uistate.capture.ImageWellUiState
 import com.google.jetpackcamera.ui.uistate.capture.ZoomControlUiState
 import com.google.jetpackcamera.ui.uistate.capture.ZoomUiState
 import com.google.jetpackcamera.ui.uistate.capture.compound.CaptureUiState
-import com.google.jetpackcamera.ui.uistate.capture.compound.QuickSettingsUiState
 import kotlinx.coroutines.flow.transformWhile
 import kotlinx.coroutines.launch
 
@@ -300,6 +305,39 @@ private fun ContentScreen(
         )
     }
 
+    var isQuickSettingsOpen by rememberSaveable { mutableStateOf(false) }
+    val scaffoldState = rememberBottomSheetScaffoldState(
+        bottomSheetState = rememberStandardBottomSheetState(
+            initialValue = SheetValue.Hidden,
+            skipHiddenState = false
+        )
+    )
+
+    // Programmatic sync: When `isQuickSettingsOpen` changes via button click or dismiss action,
+    // drive the bottom sheet animation to expand or hide accordingly.
+    LaunchedEffect(isQuickSettingsOpen) {
+        if (isQuickSettingsOpen) {
+            scaffoldState.bottomSheetState.expand()
+        } else {
+            scaffoldState.bottomSheetState.hide()
+        }
+    }
+
+    // Gesture sync: When the user manually swipes down to dismiss the bottom sheet,
+    // synchronize the state holder so `isQuickSettingsOpen` resets to false when hidden.
+    LaunchedEffect(scaffoldState.bottomSheetState.isVisible) {
+        if (!scaffoldState.bottomSheetState.isVisible && isQuickSettingsOpen) {
+            isQuickSettingsOpen = false
+        }
+    }
+
+    // Intercept back navigation only while Quick Settings is actively open.
+    // Note: Checking `isQuickSettingsOpen` instead of `bottomSheetState.isVisible` ensures back handling
+    // is immediately relinquished back to the Activity the instant the sheet begins closing.
+    BackHandler(enabled = isQuickSettingsOpen) {
+        isQuickSettingsOpen = false
+    }
+
     var initialRecordingSettings by remember { mutableStateOf<InitialRecordingSettings?>(null) }
     LaunchedEffect(videoRecordingState.value) {
         with(videoRecordingState.value) {
@@ -444,36 +482,21 @@ private fun ContentScreen(
     }
     val captureButtonLambda = remember(
         captureButtonState,
-        quickSettingsState,
-        quickSettingsController,
         captureController
     ) {
         @Composable { modifier: Modifier ->
-            val quickSettingsUiState = quickSettingsState.value
-            fun runCaptureAction(action: () -> Unit) {
-                if ((quickSettingsUiState as? QuickSettingsUiState.Available)
-                        ?.quickSettingsIsOpen == true
-                ) {
-                    quickSettingsController?.toggleQuickSettings()
-                }
-                action()
-            }
             CaptureButton(
                 captureButtonUiState = captureButtonState.value,
-                isQuickSettingsOpen = (quickSettingsUiState as? QuickSettingsUiState.Available)
-                    ?.quickSettingsIsOpen ?: false,
                 onCaptureImage = {
-                    runCaptureAction {
-                        captureController?.captureImage(it)
-                    }
+                    isQuickSettingsOpen = false
+                    captureController?.captureImage(it)
                 },
                 onIncrementZoom = { targetZoom ->
                     scope.launch { zoomStateManager.incrementZoom(targetZoom, LensToZoom.PRIMARY) }
                 },
                 onStartVideoRecording = {
-                    runCaptureAction {
-                        captureController?.startVideoRecording()
-                    }
+                    isQuickSettingsOpen = false
+                    captureController?.startVideoRecording()
                 },
                 onStopVideoRecording = { captureController?.stopVideoRecording() },
                 onLockVideoRecording = { isLocked ->
@@ -578,8 +601,7 @@ private fun ContentScreen(
 
     val quickSettingsButtonLambda = remember(
         isVideoRecordingActive,
-        quickSettingsState,
-        quickSettingsController
+        isQuickSettingsOpen
     ) {
         @Composable { modifier: Modifier ->
             val isQuickSettingsVisible = !isVideoRecordingActive.value
@@ -595,14 +617,11 @@ private fun ContentScreen(
                     )
                 }
             ) {
-                quickSettingsController?.let { controller ->
-                    ToggleQuickSettingsButton(
-                        isOpen = (quickSettingsState.value as? QuickSettingsUiState.Available)
-                            ?.quickSettingsIsOpen == true,
-                        onClick = controller::toggleQuickSettings,
-                        modifier = modifier
-                    )
-                }
+                ToggleQuickSettingsButton(
+                    isOpen = isQuickSettingsOpen,
+                    onClick = { isQuickSettingsOpen = !isQuickSettingsOpen },
+                    modifier = modifier
+                )
             }
         }
     }
@@ -614,11 +633,11 @@ private fun ContentScreen(
     ) {
         @Composable { modifier: Modifier ->
             quickSettingsController?.let { controller ->
-                QuickSettingsBottomSheet(
+                QuickSettingsScaffoldContent(
                     modifier = modifier,
                     quickSettingsUiState = quickSettingsState.value,
                     onNavigateToSettings = {
-                        controller.toggleQuickSettings()
+                        isQuickSettingsOpen = false
                         onNavigateToSettings()
                     },
                     quickSettingsController = controller
@@ -734,6 +753,8 @@ private fun ContentScreen(
 
     LayoutWrapper(
         modifier = modifier,
+        scaffoldState = scaffoldState,
+        onDismissQuickSettings = { isQuickSettingsOpen = false },
         hdrIndicator = hdrIndicatorLambda,
         flashModeIndicator = flashModeIndicatorLambda,
         videoQualityIndicator = videoQualityIndicatorLambda,
@@ -771,9 +792,12 @@ private fun LoadingScreen(modifier: Modifier = Modifier) {
     }
 }
 
+@OptIn(ExperimentalMaterial3Api::class)
 @Composable
 private fun LayoutWrapper(
     modifier: Modifier = Modifier,
+    scaffoldState: BottomSheetScaffoldState,
+    onDismissQuickSettings: () -> Unit = {},
     viewfinder: @Composable (modifier: Modifier) -> Unit,
     captureButton: @Composable (modifier: Modifier) -> Unit,
     flipCameraButton: @Composable (modifier: Modifier) -> Unit,
@@ -799,6 +823,8 @@ private fun LayoutWrapper(
 ) {
     PreviewLayout(
         modifier = modifier,
+        scaffoldState = scaffoldState,
+        onDismissQuickSettings = onDismissQuickSettings,
         viewfinder = viewfinder,
         captureButton = captureButton,
         imageWell = imageWell,

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

@@ -166,7 +166,6 @@ class PreviewViewModel @Inject constructor(
      * Controller for managing the quick settings UI panel and state.
      */
     val quickSettingsController: QuickSettingsController = QuickSettingsControllerImpl(
-        trackedCaptureUiState = trackedCaptureUiState,
         cameraSystem = cameraSystemRepository.cameraSystem,
         coroutineContext = viewModelScope.coroutineContext
     )

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

@@ -32,7 +32,6 @@ import com.google.jetpackcamera.settings.testing.FakeSettingsRepository
 import com.google.jetpackcamera.ui.uistate.capture.FlashModeUiState
 import com.google.jetpackcamera.ui.uistate.capture.FlipLensUiState
 import com.google.jetpackcamera.ui.uistate.capture.compound.CaptureUiState
-import com.google.jetpackcamera.ui.uistate.capture.compound.QuickSettingsUiState
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.StandardTestDispatcher
@@ -170,32 +169,6 @@ class PreviewViewModelTest {
         assertThat(cameraSystem.isLensFacingFront).isTrue()
     }
 
-    @Test
-    fun toggleQuickSettings() = runTest(StandardTestDispatcher()) {
-        startCameraUntilRunning()
-        // Initial state should be closed
-        assertIsReady(previewViewModel.captureUiState.value).also {
-            val quickSettings = it.quickSettingsUiState as QuickSettingsUiState.Available
-            assertThat(quickSettings.quickSettingsIsOpen).isFalse()
-        }
-
-        // Toggle to open
-        previewViewModel.quickSettingsController.toggleQuickSettings()
-        advanceUntilIdle()
-        assertIsReady(previewViewModel.captureUiState.value).also {
-            val quickSettings = it.quickSettingsUiState as QuickSettingsUiState.Available
-            assertThat(quickSettings.quickSettingsIsOpen).isTrue()
-        }
-
-        // Toggle back to closed
-        previewViewModel.quickSettingsController.toggleQuickSettings()
-        advanceUntilIdle()
-        assertIsReady(previewViewModel.captureUiState.value).also {
-            val quickSettings = it.quickSettingsUiState as QuickSettingsUiState.Available
-            assertThat(quickSettings.quickSettingsIsOpen).isFalse()
-        }
-    }
-
     private fun TestScope.startCameraUntilRunning() {
         previewViewModel.cameraController.startCamera()
         advanceUntilIdle()