From 00776f4d938f7bfe7426ef0f5ac784eb50159670 Mon Sep 17 00:00:00 2001 From: Kimberly Crevecoeur Date: Wed, 26 Aug 2026 08:45:04 -0700 Subject: [PATCH 1/4] address PR comments # Conflicts: # app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt --- .../java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt b/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt index cfd645680..176dbee22 100644 --- a/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt +++ b/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt @@ -54,7 +54,6 @@ import com.google.jetpackcamera.model.CaptureMode import com.google.jetpackcamera.model.ConcurrentCameraMode import com.google.jetpackcamera.model.FlashMode import com.google.jetpackcamera.model.LensFacing -import com.google.jetpackcamera.settings.R as SettingsR import com.google.jetpackcamera.settings.ui.BACK_BUTTON import com.google.jetpackcamera.settings.ui.BTN_SWITCH_SETTING_CONCURRENT_CAMERA_TAG import com.google.jetpackcamera.settings.ui.BTN_SWITCH_SETTING_LENS_FACING_TAG @@ -74,13 +73,14 @@ 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.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 import com.google.jetpackcamera.ui.components.capture.SETTINGS_BUTTON import com.google.jetpackcamera.ui.components.capture.SNACKBAR_NODE_TAG import com.google.jetpackcamera.ui.uistateadapter.capture.R import org.junit.AssumptionViolatedException +import com.google.jetpackcamera.settings.R as SettingsR +import com.google.jetpackcamera.ui.components.capture.R as CaptureR /** * Allows use of testRule.onNodeWithText that uses an integer string resource From a7541961e1cdfab82be6a0225aa16b43a1c4783d Mon Sep 17 00:00:00 2001 From: Kimberly Crevecoeur Date: Wed, 26 Aug 2026 16:05:54 -0700 Subject: [PATCH 2/4] Decouple Quick Settings open/closed state from ViewModel - Remove isQuickSettingsOpen and quickSettingsIsOpen from TrackedCaptureUiState, QuickSettingsUiState, and UI state adapters. - Remove toggleQuickSettings() from QuickSettingsController and its implementations. - Manage quick settings drawer visibility as local UI state in PreviewScreen via rememberSaveable. - Clean up unused quickSettingsIsOpen and toggleQuickSettings tests. --- .../jetpackcamera/utils/ComposeTestRuleExt.kt | 4 +- .../feature/preview/PreviewScreen.kt | 47 +++++++------------ .../feature/preview/PreviewViewModel.kt | 1 - .../feature/preview/PreviewViewModelTest.kt | 27 ----------- .../capture/CaptureScreenComponents.kt | 8 +--- .../quicksettings/QuickSettingsScreen.kt | 17 ++++--- .../impl/QuickSettingsControllerImpl.kt | 13 +---- .../impl/QuickSettingsControllerImplTest.kt | 11 ----- .../quicksettings/QuickSettingsController.kt | 5 -- .../testing/FakeQuickSettingsController.kt | 5 -- .../FakeQuickSettingsControllerTest.kt | 8 ---- .../uistate/capture/TrackedCaptureUiState.kt | 1 - .../capture/compound/QuickSettingsUiState.kt | 4 +- .../capture/compound/CaptureUiStateAdapter.kt | 3 +- .../compound/QuickSettingsUiStateAdapter.kt | 7 +-- .../QuickSettingsUiStateAdapterTest.kt | 6 +-- 16 files changed, 33 insertions(+), 134 deletions(-) diff --git a/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt b/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt index 176dbee22..cfd645680 100644 --- a/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt +++ b/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt @@ -54,6 +54,7 @@ import com.google.jetpackcamera.model.CaptureMode import com.google.jetpackcamera.model.ConcurrentCameraMode import com.google.jetpackcamera.model.FlashMode import com.google.jetpackcamera.model.LensFacing +import com.google.jetpackcamera.settings.R as SettingsR import com.google.jetpackcamera.settings.ui.BACK_BUTTON import com.google.jetpackcamera.settings.ui.BTN_SWITCH_SETTING_CONCURRENT_CAMERA_TAG import com.google.jetpackcamera.settings.ui.BTN_SWITCH_SETTING_LENS_FACING_TAG @@ -73,14 +74,13 @@ 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.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 import com.google.jetpackcamera.ui.components.capture.SETTINGS_BUTTON import com.google.jetpackcamera.ui.components.capture.SNACKBAR_NODE_TAG import com.google.jetpackcamera.ui.uistateadapter.capture.R import org.junit.AssumptionViolatedException -import com.google.jetpackcamera.settings.R as SettingsR -import com.google.jetpackcamera.ui.components.capture.R as CaptureR /** * Allows use of testRule.onNodeWithText that uses an integer string resource diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt index 4cd7e5ecd..f666bcef2 100644 --- a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt +++ b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt @@ -48,6 +48,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 @@ -119,7 +120,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 +300,7 @@ private fun ContentScreen( ) } + var isQuickSettingsOpen by rememberSaveable { mutableStateOf(false) } var initialRecordingSettings by remember { mutableStateOf(null) } LaunchedEffect(videoRecordingState.value) { with(videoRecordingState.value) { @@ -444,36 +445,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 +564,7 @@ private fun ContentScreen( val quickSettingsButtonLambda = remember( isVideoRecordingActive, - quickSettingsState, - quickSettingsController + isQuickSettingsOpen ) { @Composable { modifier: Modifier -> val isQuickSettingsVisible = !isVideoRecordingActive.value @@ -595,19 +580,17 @@ 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 + ) } } } val quickSettingsOverlayLambda = remember( + isQuickSettingsOpen, quickSettingsState, quickSettingsController, onNavigateToSettings @@ -615,10 +598,12 @@ private fun ContentScreen( @Composable { modifier: Modifier -> quickSettingsController?.let { controller -> QuickSettingsBottomSheet( + isOpen = isQuickSettingsOpen, + onDismiss = { isQuickSettingsOpen = false }, modifier = modifier, quickSettingsUiState = quickSettingsState.value, onNavigateToSettings = { - controller.toggleQuickSettings() + isQuickSettingsOpen = false onNavigateToSettings() }, quickSettingsController = controller diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt index 94e90cd61..41ab5b1ef 100644 --- a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewViewModel.kt +++ b/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 ) diff --git a/feature/preview/src/test/java/com/google/jetpackcamera/feature/preview/PreviewViewModelTest.kt b/feature/preview/src/test/java/com/google/jetpackcamera/feature/preview/PreviewViewModelTest.kt index e3e603d1a..5bc08b269 100644 --- a/feature/preview/src/test/java/com/google/jetpackcamera/feature/preview/PreviewViewModelTest.kt +++ b/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() diff --git a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureScreenComponents.kt b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureScreenComponents.kt index 8abb31a73..31ca300fe 100644 --- a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureScreenComponents.kt +++ b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureScreenComponents.kt @@ -113,7 +113,6 @@ import com.google.jetpackcamera.model.CaptureMode import com.google.jetpackcamera.model.StabilizationMode import com.google.jetpackcamera.model.VideoQuality import com.google.jetpackcamera.ui.controller.SnackBarController -import com.google.jetpackcamera.ui.controller.quicksettings.QuickSettingsController import com.google.jetpackcamera.ui.uistate.DisableRationale import com.google.jetpackcamera.ui.uistate.SingleSelectableUiState import com.google.jetpackcamera.ui.uistate.SnackbarData @@ -676,13 +675,11 @@ fun PreviewDisplay( fun CaptureButton( modifier: Modifier = Modifier, captureButtonUiState: CaptureButtonUiState, - isQuickSettingsOpen: Boolean, onIncrementZoom: (Float) -> Unit = {}, onCaptureImage: (ContentResolver) -> Unit = {}, onStartVideoRecording: () -> Unit = {}, onStopVideoRecording: () -> Unit = {}, - onLockVideoRecording: (Boolean) -> Unit = {}, - quickSettingsController: QuickSettingsController? = null + onLockVideoRecording: (Boolean) -> Unit = {} ) { val context = LocalContext.current @@ -695,9 +692,6 @@ fun CaptureButton( ) { onCaptureImage(context.contentResolver) } - if (isQuickSettingsOpen) { - quickSettingsController?.toggleQuickSettings() - } }, onStartRecording = { if (captureButtonUiState is CaptureButtonUiState.Enabled && diff --git a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt index 68b5a9b3f..fb5bf1069 100644 --- a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt +++ b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt @@ -56,19 +56,19 @@ import com.google.jetpackcamera.ui.uistate.capture.compound.QuickSettingsUiState @OptIn(ExperimentalMaterial3Api::class) @Composable fun QuickSettingsBottomSheet( - modifier: Modifier = Modifier, + isOpen: Boolean, + onDismiss: () -> Unit, quickSettingsUiState: QuickSettingsUiState, onNavigateToSettings: () -> Unit, quickSettingsController: QuickSettingsController, + modifier: Modifier = Modifier, showMoreSettingsButton: Boolean = true ) { - if (quickSettingsUiState is QuickSettingsUiState.Available && - quickSettingsUiState.quickSettingsIsOpen - ) { + if (isOpen && quickSettingsUiState is QuickSettingsUiState.Available) { val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) QuickSettingsModalBottomSheet( modifier = modifier, - onDismiss = quickSettingsController::toggleQuickSettings, + onDismiss = onDismiss, sheetState = sheetState ) { QuickSettingsContent( @@ -174,8 +174,6 @@ private fun QuickSettingsContent( * A no-op implementation of [QuickSettingsController] for use in Compose previews and tests. */ class NoOpQuickSettingsController : QuickSettingsController { - override fun toggleQuickSettings() {} - override fun setLensFacing(lensFace: LensFacing) {} override fun setFlash(flashMode: FlashMode) {} @@ -194,6 +192,8 @@ class NoOpQuickSettingsController : QuickSettingsController { fun ExpandedQuickSettingsUiPreview() { MaterialTheme { QuickSettingsBottomSheet( + isOpen = true, + onDismiss = {}, quickSettingsUiState = QuickSettingsUiState.Available( aspectRatioUiState = AspectRatioUiState.Available( selectedAspectRatio = AspectRatio.NINE_SIXTEEN, @@ -227,8 +227,7 @@ fun ExpandedQuickSettingsUiPreview() { SingleSelectableUiState.SelectableUi(LensFacing.FRONT) ) ), - hdrUiState = HdrUiState.Unavailable, - quickSettingsIsOpen = true + hdrUiState = HdrUiState.Unavailable ), onNavigateToSettings = {}, quickSettingsController = NoOpQuickSettingsController() diff --git a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/QuickSettingsControllerImpl.kt b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/QuickSettingsControllerImpl.kt index 2c0502deb..0b532975d 100644 --- a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/QuickSettingsControllerImpl.kt +++ b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/QuickSettingsControllerImpl.kt @@ -23,36 +23,25 @@ import com.google.jetpackcamera.model.FlashMode import com.google.jetpackcamera.model.ImageOutputFormat import com.google.jetpackcamera.model.LensFacing import com.google.jetpackcamera.ui.controller.quicksettings.QuickSettingsController -import com.google.jetpackcamera.ui.uistate.capture.TrackedCaptureUiState import kotlin.coroutines.CoroutineContext import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.cancel -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.update import kotlinx.coroutines.job import kotlinx.coroutines.launch /** - * Implementation of [QuickSettingsController] that interacts with [CameraSystem] and updates - * [trackedCaptureUiState]. + * Implementation of [QuickSettingsController] that interacts with [CameraSystem]. * - * @param trackedCaptureUiState The state flow to update with quick settings information. * @param cameraSystem The camera system to control. * @param coroutineContext The [CoroutineContext] for launching coroutines. */ class QuickSettingsControllerImpl( - private val trackedCaptureUiState: MutableStateFlow, private val cameraSystem: CameraSystem, coroutineContext: CoroutineContext ) : QuickSettingsController { private val job = Job(parent = coroutineContext[Job.Key]) private val scope = CoroutineScope(coroutineContext + job) - override fun toggleQuickSettings() { - trackedCaptureUiState.update { old -> - old.copy(isQuickSettingsOpen = !old.isQuickSettingsOpen) - } - } override fun setLensFacing(lensFace: LensFacing) { scope.launch { diff --git a/ui/controller/impl/src/test/java/com/google/jetpackcamera/ui/controller/impl/QuickSettingsControllerImplTest.kt b/ui/controller/impl/src/test/java/com/google/jetpackcamera/ui/controller/impl/QuickSettingsControllerImplTest.kt index 3acbdef1a..6175485af 100644 --- a/ui/controller/impl/src/test/java/com/google/jetpackcamera/ui/controller/impl/QuickSettingsControllerImplTest.kt +++ b/ui/controller/impl/src/test/java/com/google/jetpackcamera/ui/controller/impl/QuickSettingsControllerImplTest.kt @@ -23,9 +23,7 @@ import com.google.jetpackcamera.model.DynamicRange import com.google.jetpackcamera.model.FlashMode import com.google.jetpackcamera.model.ImageOutputFormat import com.google.jetpackcamera.model.LensFacing -import com.google.jetpackcamera.ui.uistate.capture.TrackedCaptureUiState import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle @@ -42,25 +40,16 @@ internal class QuickSettingsControllerImplTest { private val testDispatcher = StandardTestDispatcher(testScope.testScheduler) private val cameraSystem = FakeCameraSystem() - private val trackedCaptureUiState = MutableStateFlow(TrackedCaptureUiState()) private lateinit var controller: QuickSettingsControllerImpl @Before fun setup() { controller = QuickSettingsControllerImpl( - trackedCaptureUiState = trackedCaptureUiState, cameraSystem = cameraSystem, coroutineContext = testDispatcher ) } - @Test - fun toggleQuickSettings_mutatesUiState() = testScope.runTest { - val initialValue = trackedCaptureUiState.value.isQuickSettingsOpen - controller.toggleQuickSettings() - assertThat(trackedCaptureUiState.value.isQuickSettingsOpen).isEqualTo(!initialValue) - } - @Test fun setLensFacing_mutatesCameraSystem() = testScope.runTest { controller.setLensFacing(LensFacing.FRONT) diff --git a/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/quicksettings/QuickSettingsController.kt b/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/quicksettings/QuickSettingsController.kt index 49e97a2bd..9741a81df 100644 --- a/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/quicksettings/QuickSettingsController.kt +++ b/ui/controller/src/main/java/com/google/jetpackcamera/ui/controller/quicksettings/QuickSettingsController.kt @@ -26,11 +26,6 @@ import com.google.jetpackcamera.model.LensFacing * Interface for controlling quick settings. */ interface QuickSettingsController { - /** - * Toggles the visibility of the quick settings menu. - */ - fun toggleQuickSettings() - /** * Sets the lens facing (e.g., front or back camera). * diff --git a/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeQuickSettingsController.kt b/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeQuickSettingsController.kt index b7e2c5d7a..c92eba7be 100644 --- a/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeQuickSettingsController.kt +++ b/ui/controller/testing/src/main/java/com/google/jetpackcamera/ui/controller/testing/FakeQuickSettingsController.kt @@ -26,7 +26,6 @@ import com.google.jetpackcamera.ui.controller.quicksettings.QuickSettingsControl /** * A fake implementation of [QuickSettingsController] that allows for configuring actions for its methods. * - * @param toggleQuickSettingsAction The action to perform when [toggleQuickSettings] is called. * @param setLensFacingAction The action to perform when [setLensFacing] is called. * @param setFlashAction The action to perform when [setFlash] is called. * @param setAspectRatioAction The action to perform when [setAspectRatio] is called. @@ -35,7 +34,6 @@ import com.google.jetpackcamera.ui.controller.quicksettings.QuickSettingsControl * @param setCaptureModeAction The action to perform when [setCaptureMode] is called. */ class FakeQuickSettingsController( - var toggleQuickSettingsAction: () -> Unit = {}, var setLensFacingAction: (LensFacing) -> Unit = {}, var setFlashAction: (FlashMode) -> Unit = {}, var setAspectRatioAction: (AspectRatio) -> Unit = {}, @@ -43,9 +41,6 @@ class FakeQuickSettingsController( var setImageFormatAction: (ImageOutputFormat) -> Unit = {}, var setCaptureModeAction: (CaptureMode) -> Unit = {} ) : QuickSettingsController { - override fun toggleQuickSettings() { - toggleQuickSettingsAction() - } override fun setLensFacing(lensFace: LensFacing) { setLensFacingAction(lensFace) diff --git a/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeQuickSettingsControllerTest.kt b/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeQuickSettingsControllerTest.kt index 880d6efb5..b36f4da47 100644 --- a/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeQuickSettingsControllerTest.kt +++ b/ui/controller/testing/src/test/java/com/google/jetpackcamera/ui/controller/testing/FakeQuickSettingsControllerTest.kt @@ -28,14 +28,6 @@ import org.junit.runners.JUnit4 @RunWith(JUnit4::class) class FakeQuickSettingsControllerTest { - @Test - fun toggleQuickSettings_invokesAction() { - var called = false - val controller = FakeQuickSettingsController(toggleQuickSettingsAction = { called = true }) - controller.toggleQuickSettings() - assertThat(called).isTrue() - } - @Test fun setLensFacing_invokesAction() { var calledValue: LensFacing? = null diff --git a/ui/uistate/capture/src/main/java/com/google/jetpackcamera/ui/uistate/capture/TrackedCaptureUiState.kt b/ui/uistate/capture/src/main/java/com/google/jetpackcamera/ui/uistate/capture/TrackedCaptureUiState.kt index 42e4ebac7..0e9d908d2 100644 --- a/ui/uistate/capture/src/main/java/com/google/jetpackcamera/ui/uistate/capture/TrackedCaptureUiState.kt +++ b/ui/uistate/capture/src/main/java/com/google/jetpackcamera/ui/uistate/capture/TrackedCaptureUiState.kt @@ -27,7 +27,6 @@ import com.google.jetpackcamera.data.media.MediaDescriptor * directly observes. */ data class TrackedCaptureUiState( - val isQuickSettingsOpen: Boolean = false, val isDebugOverlayOpen: Boolean = false, val isRecordingLocked: Boolean = false, val zoomAnimationTarget: Float? = null, diff --git a/ui/uistate/capture/src/main/java/com/google/jetpackcamera/ui/uistate/capture/compound/QuickSettingsUiState.kt b/ui/uistate/capture/src/main/java/com/google/jetpackcamera/ui/uistate/capture/compound/QuickSettingsUiState.kt index 2c2a61c44..fcaf44d1a 100644 --- a/ui/uistate/capture/src/main/java/com/google/jetpackcamera/ui/uistate/capture/compound/QuickSettingsUiState.kt +++ b/ui/uistate/capture/src/main/java/com/google/jetpackcamera/ui/uistate/capture/compound/QuickSettingsUiState.kt @@ -41,15 +41,13 @@ sealed interface QuickSettingsUiState { * @param flashModeUiState The UI state for the flash mode setting. * @param flipLensUiState The UI state for the flip lens (front/back camera) button. * @param hdrUiState The UI state for the HDR (High Dynamic Range) setting. - * @param quickSettingsIsOpen Indicates whether the quick settings panel is currently open. */ data class Available( val aspectRatioUiState: AspectRatioUiState, val captureModeUiState: CaptureModeUiState, val flashModeUiState: FlashModeUiState, val flipLensUiState: FlipLensUiState, - val hdrUiState: HdrUiState, - val quickSettingsIsOpen: Boolean = false + val hdrUiState: HdrUiState ) : QuickSettingsUiState companion object diff --git a/ui/uistateadapter/capture/src/main/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/CaptureUiStateAdapter.kt b/ui/uistateadapter/capture/src/main/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/CaptureUiStateAdapter.kt index 2afabe86f..b41f1353e 100644 --- a/ui/uistateadapter/capture/src/main/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/CaptureUiStateAdapter.kt +++ b/ui/uistateadapter/capture/src/main/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/CaptureUiStateAdapter.kt @@ -129,8 +129,7 @@ fun captureUiState( flashModeUiState, flipLensUiState, aspectRatioUiState, - hdrUiState, - trackedUiState.isQuickSettingsOpen + hdrUiState ), sessionFirstFrameTimestamp = roundedCameraState.sessionFirstFrameTimestamp, stabilizationUiState = StabilizationUiState.from( diff --git a/ui/uistateadapter/capture/src/main/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/QuickSettingsUiStateAdapter.kt b/ui/uistateadapter/capture/src/main/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/QuickSettingsUiStateAdapter.kt index 008c2289d..243598a01 100644 --- a/ui/uistateadapter/capture/src/main/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/QuickSettingsUiStateAdapter.kt +++ b/ui/uistateadapter/capture/src/main/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/QuickSettingsUiStateAdapter.kt @@ -33,7 +33,6 @@ import com.google.jetpackcamera.ui.uistate.capture.compound.QuickSettingsUiState * @param flipLensUiState The UI state for the flip lens button. * @param aspectRatioUiState The UI state for the aspect ratio setting. * @param hdrUiState The UI state for the HDR setting. - * @param quickSettingsIsOpen Indicates whether the quick settings panel is open. * @return A [QuickSettingsUiState.Available] instance containing the consolidated states. */ fun QuickSettingsUiState.Companion.from( @@ -41,15 +40,13 @@ fun QuickSettingsUiState.Companion.from( flashModeUiState: FlashModeUiState, flipLensUiState: FlipLensUiState, aspectRatioUiState: AspectRatioUiState, - hdrUiState: HdrUiState, - quickSettingsIsOpen: Boolean + hdrUiState: HdrUiState ): QuickSettingsUiState { return QuickSettingsUiState.Available( aspectRatioUiState = aspectRatioUiState, captureModeUiState = captureModeUiState, flashModeUiState = flashModeUiState, flipLensUiState = flipLensUiState, - hdrUiState = hdrUiState, - quickSettingsIsOpen = quickSettingsIsOpen + hdrUiState = hdrUiState ) } diff --git a/ui/uistateadapter/capture/src/test/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/QuickSettingsUiStateAdapterTest.kt b/ui/uistateadapter/capture/src/test/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/QuickSettingsUiStateAdapterTest.kt index 4b1175c96..721e4d4d0 100644 --- a/ui/uistateadapter/capture/src/test/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/QuickSettingsUiStateAdapterTest.kt +++ b/ui/uistateadapter/capture/src/test/java/com/google/jetpackcamera/ui/uistateadapter/capture/compound/QuickSettingsUiStateAdapterTest.kt @@ -36,15 +36,12 @@ internal class QuickSettingsUiStateAdapterTest { val flipLensUiState = FlipLensUiState.Unavailable val aspectRatioUiState = AspectRatioUiState.Unavailable val hdrUiState = HdrUiState.Unavailable - val quickSettingsIsOpen = true - val quickSettingsUiState = QuickSettingsUiState.from( captureModeUiState = captureModeUiState, flashModeUiState = flashModeUiState, flipLensUiState = flipLensUiState, aspectRatioUiState = aspectRatioUiState, - hdrUiState = hdrUiState, - quickSettingsIsOpen = quickSettingsIsOpen + hdrUiState = hdrUiState ) assertThat(quickSettingsUiState).isInstanceOf(QuickSettingsUiState.Available::class.java) @@ -54,6 +51,5 @@ internal class QuickSettingsUiStateAdapterTest { assertThat(availableState.flipLensUiState).isEqualTo(flipLensUiState) assertThat(availableState.aspectRatioUiState).isEqualTo(aspectRatioUiState) assertThat(availableState.hdrUiState).isEqualTo(hdrUiState) - assertThat(availableState.quickSettingsIsOpen).isEqualTo(quickSettingsIsOpen) } } From 4f922ce2cf2547d8973c0353f0dc0a67049aac76 Mon Sep 17 00:00:00 2001 From: Kimberly Crevecoeur Date: Wed, 26 Aug 2026 16:21:56 -0700 Subject: [PATCH 3/4] Refactor QuickSettingsContent as an internal, container-agnostic composable - Make QuickSettingsContent internal and container-independent with modifier and showMoreSettingsButton parameters. - Flatten internal layout structure by removing intermediate QuickSettingsLayout helper. - Add additional top padding above the 'More settings' navigation button. --- .../quicksettings/QuickSettingsScreen.kt | 60 +++++++++---------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt index fb5bf1069..805084239 100644 --- a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt +++ b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt @@ -15,9 +15,8 @@ */ package com.google.jetpackcamera.ui.components.capture.quicksettings -import androidx.annotation.StringRes import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.ColumnScope +import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.MaterialTheme @@ -81,32 +80,23 @@ fun QuickSettingsBottomSheet( } } +/** + * Agnostic content for the Quick Settings panel containing the title header, option rows + * (Flash, Capture Mode, Aspect Ratio, HDR), and an optional navigation button to full settings. + * + * @param quickSettingsUiState The current [QuickSettingsUiState.Available]. + * @param quickSettingsController The [QuickSettingsController] to handle setting changes. + * @param onNavigateToSettings Callback when the user navigates to full settings. + * @param modifier The [Modifier] to apply to the content column. + * @param showMoreSettingsButton Whether to show the "More settings" navigation button. + */ @Composable -private fun QuickSettingsLayout( - @StringRes titleRes: Int, - showMoreSettingsButton: Boolean, - onNavigateToSettings: () -> Unit, - content: @Composable ColumnScope.() -> Unit -) { - Column { - Text( - modifier = Modifier.padding(start = 16.dp, end = 16.dp, bottom = 8.dp), - text = stringResource(id = titleRes), - style = MaterialTheme.typography.titleLarge - ) - content() - } - if (showMoreSettingsButton) { - QuickNavSettings(onNavigateToSettings = onNavigateToSettings) - } -} - -@Composable -private fun QuickSettingsContent( +internal fun QuickSettingsContent( quickSettingsUiState: QuickSettingsUiState.Available, quickSettingsController: QuickSettingsController, onNavigateToSettings: () -> Unit, - showMoreSettingsButton: Boolean + modifier: Modifier = Modifier, + showMoreSettingsButton: Boolean = true ) { val captureMode = (quickSettingsUiState.captureModeUiState as? CaptureModeUiState.Available) ?.selectedCaptureMode ?: CaptureMode.IMAGE_ONLY @@ -117,11 +107,13 @@ private fun QuickSettingsContent( CaptureMode.STANDARD -> R.string.quick_settings_title_photo_and_video_settings } - QuickSettingsLayout( - titleRes = titleRes, - showMoreSettingsButton = showMoreSettingsButton, - onNavigateToSettings = onNavigateToSettings - ) { + Column(modifier = modifier.fillMaxWidth()) { + Text( + modifier = Modifier.padding(start = 16.dp, end = 16.dp, bottom = 8.dp), + text = stringResource(id = titleRes), + style = MaterialTheme.typography.titleLarge + ) + // Flash Mode settings if (quickSettingsUiState.flashModeUiState is FlashModeUiState.Available) { FlashRow( @@ -131,8 +123,7 @@ private fun QuickSettingsContent( } // Capture Mode settings (Standard only) - if (captureMode == CaptureMode.STANDARD && - quickSettingsUiState.captureModeUiState is CaptureModeUiState.Available + if (captureMode == CaptureMode.STANDARD ) { CaptureModeRow( onSetCaptureMode = quickSettingsController::setCaptureMode, @@ -167,6 +158,13 @@ private fun QuickSettingsContent( hdrUiState = quickSettingsUiState.hdrUiState ) } + + if (showMoreSettingsButton) { + QuickNavSettings( + onNavigateToSettings = onNavigateToSettings, + modifier = Modifier.padding(top = 12.dp) + ) + } } } From 8544a5b6ba8da61f7dfdc3ceccfd20edc111736a Mon Sep 17 00:00:00 2001 From: Kimberly Crevecoeur Date: Thu, 27 Aug 2026 13:53:09 -0700 Subject: [PATCH 4/4] Refactor Quick Settings to use BottomSheetScaffold Integrate Material 3 BottomSheetScaffold into CaptureLayout to render Quick Settings in-hierarchy and avoid window-spawning modal overlays. Make the drag handle pill clickable for dismiss, and update test helpers to close the sheet deterministically. --- .gemini/styleguide.md | 1 + .../google/jetpackcamera/NavigationTest.kt | 3 +- .../jetpackcamera/utils/ComposeTestRuleExt.kt | 58 ++++++++++--------- .../feature/preview/PreviewScreen.kt | 51 ++++++++++++++-- .../ui/components/capture/CaptureLayout.kt | 49 +++++++++++++--- .../ui/components/capture/TestTags.kt | 1 + .../quicksettings/QuickSettingsScreen.kt | 32 ++++++++++ 7 files changed, 153 insertions(+), 42 deletions(-) diff --git a/.gemini/styleguide.md b/.gemini/styleguide.md index a6760141c..24f916576 100644 --- a/.gemini/styleguide.md +++ b/.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"). diff --git a/app/src/androidTest/java/com/google/jetpackcamera/NavigationTest.kt b/app/src/androidTest/java/com/google/jetpackcamera/NavigationTest.kt index f4fe00b1e..5ee7610a8 100644 --- a/app/src/androidTest/java/com/google/jetpackcamera/NavigationTest.kt +++ b/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 diff --git a/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt b/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt index cfd645680..ad4470e2c 100644 --- a/app/src/androidTest/java/com/google/jetpackcamera/utils/ComposeTestRuleExt.kt +++ b/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 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 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() } } } diff --git a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt b/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt index f666bcef2..733c91856 100644 --- a/feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt +++ b/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 @@ -97,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 @@ -301,6 +306,38 @@ 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(null) } LaunchedEffect(videoRecordingState.value) { with(videoRecordingState.value) { @@ -590,16 +627,13 @@ private fun ContentScreen( } val quickSettingsOverlayLambda = remember( - isQuickSettingsOpen, quickSettingsState, quickSettingsController, onNavigateToSettings ) { @Composable { modifier: Modifier -> quickSettingsController?.let { controller -> - QuickSettingsBottomSheet( - isOpen = isQuickSettingsOpen, - onDismiss = { isQuickSettingsOpen = false }, + QuickSettingsScaffoldContent( modifier = modifier, quickSettingsUiState = quickSettingsState.value, onNavigateToSettings = { @@ -719,6 +753,8 @@ private fun ContentScreen( LayoutWrapper( modifier = modifier, + scaffoldState = scaffoldState, + onDismissQuickSettings = { isQuickSettingsOpen = false }, hdrIndicator = hdrIndicatorLambda, flashModeIndicator = flashModeIndicatorLambda, videoQualityIndicator = videoQualityIndicatorLambda, @@ -756,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, @@ -784,6 +823,8 @@ private fun LayoutWrapper( ) { PreviewLayout( modifier = modifier, + scaffoldState = scaffoldState, + onDismissQuickSettings = onDismissQuickSettings, viewfinder = viewfinder, captureButton = captureButton, imageWell = imageWell, diff --git a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureLayout.kt b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureLayout.kt index 0409fdc86..13d9826d1 100644 --- a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureLayout.kt +++ b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureLayout.kt @@ -16,6 +16,7 @@ package com.google.jetpackcamera.ui.components.capture import androidx.compose.foundation.background +import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -30,9 +31,17 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.safeDrawingPadding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.statusBarsPadding -import androidx.compose.material3.Scaffold +import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material3.BottomSheetDefaults +import androidx.compose.material3.BottomSheetScaffold +import androidx.compose.material3.BottomSheetScaffoldState +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.SheetValue import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState +import androidx.compose.material3.rememberBottomSheetScaffoldState +import androidx.compose.material3.rememberStandardBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.mutableStateOf @@ -49,6 +58,8 @@ import androidx.compose.ui.unit.dp * The base layout for the camera capture screen. * * @param modifier the modifier for this component + * @param scaffoldState the bottom sheet scaffold state + * @param onDismissQuickSettings callback to dismiss quick settings when clicking the drag handle * @param viewfinder the viewfinder composable * @param captureButton the capture button composable * @param imageWell the image well composable @@ -64,9 +75,17 @@ import androidx.compose.ui.unit.dp * @param screenFlashOverlay the screen flash overlay composable * @param snackBar the snack bar composable for showing messages */ +@OptIn(ExperimentalMaterial3Api::class) @Composable fun PreviewLayout( modifier: Modifier = Modifier, + scaffoldState: BottomSheetScaffoldState = rememberBottomSheetScaffoldState( + bottomSheetState = rememberStandardBottomSheetState( + initialValue = SheetValue.Hidden, + skipHiddenState = false + ) + ), + onDismissQuickSettings: () -> Unit = {}, viewfinder: @Composable (Modifier) -> Unit, captureButton: @Composable (Modifier) -> Unit, imageWell: @Composable (Modifier) -> Unit, @@ -82,15 +101,31 @@ fun PreviewLayout( screenFlashOverlay: @Composable (Modifier) -> Unit, snackBar: @Composable (Modifier, snackbarHostState: SnackbarHostState) -> Unit ) { - val snackbarHostState = remember { SnackbarHostState() } val overlapTargetBounds = remember { mutableStateOf(Rect.Zero) } CompositionLocalProvider(LocalOverlapTargetBounds provides overlapTargetBounds) { - Scaffold( + BottomSheetScaffold( modifier = Modifier.fillMaxSize(), + scaffoldState = scaffoldState, + sheetPeekHeight = 0.dp, + sheetDragHandle = { + BottomSheetDefaults.DragHandle( + modifier = Modifier + .testTag(QUICK_SETTINGS_DRAG_HANDLE) + .clickable( + onClickLabel = "Close quick settings", + onClick = onDismissQuickSettings + ) + ) + }, + sheetShape = RoundedCornerShape(topStart = 28.dp, topEnd = 28.dp), + sheetContainerColor = MaterialTheme.colorScheme.surfaceContainerLow, + sheetContent = { + quickSettingsOverlay(Modifier) + }, snackbarHost = { SnackbarHost( - hostState = snackbarHostState, + hostState = scaffoldState.snackbarHostState, modifier = Modifier.testTag(SNACKBAR_NODE_TAG) ) } @@ -115,13 +150,12 @@ fun PreviewLayout( flipCameraButton = flipCameraButton, quickSettingsToggleButton = quickSettingsButton, captureModeToggleSwitch = captureModeToggle, - bottomSheetQuickSettings = quickSettingsOverlay, zoomControls = zoomLevelDisplay, elapsedTimeDisplay = elapsedTimeDisplay ) } // controls overlay - snackBar(Modifier, snackbarHostState) + snackBar(Modifier, scaffoldState.snackbarHostState) screenFlashOverlay(Modifier) } debugOverlay(Modifier) @@ -138,7 +172,6 @@ private fun VerticalMaterialControls( imageWell: @Composable (Modifier) -> Unit, flipCameraButton: @Composable (Modifier) -> Unit, quickSettingsToggleButton: @Composable (Modifier) -> Unit, - bottomSheetQuickSettings: @Composable (Modifier) -> Unit, captureModeToggleSwitch: @Composable (Modifier) -> Unit, elapsedTimeDisplay: @Composable (Modifier) -> Unit ) { @@ -229,10 +262,10 @@ private fun VerticalMaterialControls( } } } - bottomSheetQuickSettings(Modifier) } } +@OptIn(ExperimentalMaterial3Api::class) @Preview @Composable private fun CaptureLayoutPreview() { diff --git a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/TestTags.kt b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/TestTags.kt index 2ccd5198b..dd664ec6e 100644 --- a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/TestTags.kt +++ b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/TestTags.kt @@ -55,6 +55,7 @@ const val QUICK_SETTINGS_DROP_DOWN = "QuickSettingsDropDown" const val SETTINGS_BUTTON = "SettingsButton" const val QUICK_SETTINGS_BOTTOM_SHEET = "QuickSettingsBottomSheet" +const val QUICK_SETTINGS_DRAG_HANDLE = "QuickSettingsDragHandle" const val QUICK_SETTINGS_RATIO_3_4_BUTTON = "QuickSettingsRatio3:4Button" const val QUICK_SETTINGS_RATIO_9_16_BUTTON = "QuickSettingsRatio9:16Button" diff --git a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt index 805084239..24774d8d8 100644 --- a/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt +++ b/ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/quicksettings/QuickSettingsScreen.kt @@ -17,6 +17,7 @@ package com.google.jetpackcamera.ui.components.capture.quicksettings import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.padding import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.MaterialTheme @@ -24,6 +25,7 @@ import androidx.compose.material3.Text import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp @@ -33,6 +35,7 @@ import com.google.jetpackcamera.model.DynamicRange import com.google.jetpackcamera.model.FlashMode import com.google.jetpackcamera.model.ImageOutputFormat import com.google.jetpackcamera.model.LensFacing +import com.google.jetpackcamera.ui.components.capture.QUICK_SETTINGS_BOTTOM_SHEET import com.google.jetpackcamera.ui.components.capture.R import com.google.jetpackcamera.ui.components.capture.quicksettings.ui.AspectRatioRow import com.google.jetpackcamera.ui.components.capture.quicksettings.ui.CaptureModeRow @@ -49,6 +52,35 @@ import com.google.jetpackcamera.ui.uistate.capture.FlipLensUiState import com.google.jetpackcamera.ui.uistate.capture.HdrUiState import com.google.jetpackcamera.ui.uistate.capture.compound.QuickSettingsUiState +/** + * Agnostic content for the Quick Settings panel wrapped for a BottomSheetScaffold sheet. + */ +@Composable +fun QuickSettingsScaffoldContent( + quickSettingsUiState: QuickSettingsUiState, + onNavigateToSettings: () -> Unit, + quickSettingsController: QuickSettingsController, + modifier: Modifier = Modifier, + showMoreSettingsButton: Boolean = true +) { + if (quickSettingsUiState is QuickSettingsUiState.Available) { + Column( + modifier = modifier + .fillMaxWidth() + .navigationBarsPadding() + .padding(bottom = 24.dp) + .testTag(QUICK_SETTINGS_BOTTOM_SHEET) + ) { + QuickSettingsContent( + quickSettingsUiState = quickSettingsUiState, + quickSettingsController = quickSettingsController, + onNavigateToSettings = onNavigateToSettings, + showMoreSettingsButton = showMoreSettingsButton + ) + } + } +} + /** * The UI bottom sheet component for quick settings. */