address PR comments

google · Sep 20, 2023 · 51fe392 · 51fe392
1 parent 5549b0f
commit 51fe392
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 36 deletions.
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
@@ -20,7 +20,6 @@ import android.os.Handler
 import android.os.Looper
 import android.util.Log
 import androidx.camera.core.Preview.SurfaceProvider
-import androidx.compose.foundation.gestures.rememberTransformableState
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.IntrinsicSize
@@ -82,12 +81,6 @@ fun PreviewScreen(
 
     val zoomHandler = Handler(Looper.getMainLooper())
 
-    val transformableState = rememberTransformableState(
-        onTransformation = { zoomChange, _, _ ->
-            zoomScale = viewModel.setZoomScale(zoomChange)
-            zoomScaleShow = true
-            zoomHandler.postDelayed({ zoomScaleShow = false }, ZOOM_SCALE_SHOW_TIMEOUT_MS)
-        })
 
     LaunchedEffect(lifecycleOwner) {
         val surfaceProvider = deferredSurfaceProvider.await()
@@ -107,8 +100,12 @@ fun PreviewScreen(
         PreviewDisplay(
             onFlipCamera = viewModel::flipCamera,
             onTapToFocus = viewModel::tapToFocus,
-            transformableState = transformableState,
-            previewUiState = previewUiState,
+            onZoomChange = { zoomChange: Float ->
+                viewModel.setZoomScale(zoomChange)
+                zoomScaleShow = true
+                zoomHandler.postDelayed({ zoomScaleShow = false }, ZOOM_SCALE_SHOW_TIMEOUT_MS)
+            },
+            aspectRatio = previewUiState.currentCameraSettings.aspect_ratio,
             deferredSurfaceProvider = deferredSurfaceProvider
         )
         // overlay
@@ -184,7 +181,7 @@ fun PreviewScreen(
                         onClick = { viewModel.captureImage() },
                         onLongPress = { viewModel.startVideoRecording() },
                         onRelease = { viewModel.stopVideoRecording() },
-                        state = previewUiState.videoRecordingState
+                        videoRecordingState = previewUiState.videoRecordingState
                     )
                     /* spacer is a placeholder to maintain the proportionate location of this row of
                      UI elements. if you want to  add another element, replace it with ONE element.

diff --git a/...view/src/main/java/com/google/jetpackcamera/feature/preview/ui/PreviewScreenComponents.kt b/...view/src/main/java/com/google/jetpackcamera/feature/preview/ui/PreviewScreenComponents.kt
@@ -25,8 +25,8 @@ import androidx.compose.animation.core.tween
 import androidx.compose.foundation.Canvas
 import androidx.compose.foundation.background
 import androidx.compose.foundation.border
-import androidx.compose.foundation.gestures.TransformableState
 import androidx.compose.foundation.gestures.detectTapGestures
+import androidx.compose.foundation.gestures.rememberTransformableState
 import androidx.compose.foundation.gestures.transformable
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.BoxWithConstraints
@@ -52,9 +52,9 @@ import androidx.compose.ui.input.pointer.pointerInput
 import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.unit.dp
 import androidx.compose.ui.unit.sp
-import com.google.jetpackcamera.feature.preview.PreviewUiState
 import com.google.jetpackcamera.feature.preview.R
 import com.google.jetpackcamera.feature.preview.VideoRecordingState
+import com.google.jetpackcamera.settings.model.AspectRatio
 import com.google.jetpackcamera.viewfinder.CameraPreview
 import kotlinx.coroutines.CompletableDeferred
 
@@ -66,11 +66,16 @@ private const val TAG = "PreviewScreen"
 fun PreviewDisplay(
     onTapToFocus: (Display, Int, Int, Float, Float) -> Unit,
     onFlipCamera: () -> Unit,
-
-    transformableState: TransformableState,
-    previewUiState: PreviewUiState,
+    onZoomChange: (Float) -> Unit,
+    aspectRatio: AspectRatio,
     deferredSurfaceProvider: CompletableDeferred<Preview.SurfaceProvider>
 ) {
+
+    val transformableState = rememberTransformableState(
+        onTransformation = { zoomChange, _, _ ->
+            onZoomChange(zoomChange)
+
+        })
     val onSurfaceProviderReady: (Preview.SurfaceProvider) -> Unit = {
         Log.d(TAG, "onSurfaceProviderReady")
         deferredSurfaceProvider.complete(it)
@@ -109,8 +114,7 @@ fun PreviewDisplay(
         contentAlignment = Alignment.Center
     ) {
         val maxAspectRatio: Float = maxWidth / maxHeight
-        val aspectRatio: Float =
-            previewUiState.currentCameraSettings.aspect_ratio.ratio.toFloat()
+        val aspectRatio: Float = aspectRatio.ratio.toFloat()
         val shouldUseMaxWidth = maxAspectRatio <= aspectRatio
         val width = if (shouldUseMaxWidth) maxWidth else maxHeight * aspectRatio
         val height = if (!shouldUseMaxWidth) maxHeight else maxWidth / aspectRatio
@@ -196,7 +200,7 @@ fun CaptureButton(
     onClick: () -> Unit,
     onLongPress: () -> Unit,
     onRelease: () -> Unit,
-    state: VideoRecordingState
+    videoRecordingState: VideoRecordingState
 ) {
     Box(
         modifier = modifier
@@ -217,7 +221,7 @@ fun CaptureButton(
         Canvas(modifier = Modifier.size(110.dp), onDraw = {
             drawCircle(
                 color =
-                when (state) {
+                when (videoRecordingState) {
                     VideoRecordingState.INACTIVE -> Color.Transparent
                     VideoRecordingState.ACTIVE -> Color.Red
                 }

diff --git a/...tings/src/main/java/com/google/jetpackcamera/feature/quicksettings/QuickSettingsScreen.kt b/...tings/src/main/java/com/google/jetpackcamera/feature/quicksettings/QuickSettingsScreen.kt
@@ -82,10 +82,10 @@ fun QuickSettingsScreen(
                 .clickable {
                     // if a setting is expanded, click on the background to close it.
                     // if no other settings are expanded, then close the popup
-                    if (shouldShowQuickSetting == IsExpandedQuickSetting.NONE)
-                        toggleIsOpen()
-                    else
-                        shouldShowQuickSetting = IsExpandedQuickSetting.NONE
+                    when (shouldShowQuickSetting) {
+                        IsExpandedQuickSetting.NONE -> toggleIsOpen()
+                        else -> shouldShowQuickSetting = IsExpandedQuickSetting.NONE
+                    }
                 },
             verticalArrangement = Arrangement.Center,
             horizontalAlignment = Alignment.CenterHorizontally

diff --git a/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/SettingsComponents.kt b/feature/settings/src/main/java/com/google/jetpackcamera/settings/ui/SettingsComponents.kt
@@ -86,7 +86,10 @@ fun SectionHeader(title: String) {
 }
 
 @Composable
-fun DefaultCameraFacing(cameraAppSettings: CameraAppSettings, onClick: () -> Unit) {
+fun DefaultCameraFacing(
+    cameraAppSettings: CameraAppSettings,
+    onClick: () -> Unit
+) {
     SwitchSettingUI(
         title = stringResource(id = R.string.default_facing_camera_title),
         description = null,
@@ -98,7 +101,10 @@ fun DefaultCameraFacing(cameraAppSettings: CameraAppSettings, onClick: () -> Uni
 }
 
 @Composable
-fun DarkModeSetting(currentDarkModeStatus: DarkModeStatus, setDarkMode: (DarkModeStatus) -> Unit) {
+fun DarkModeSetting(
+    currentDarkModeStatus: DarkModeStatus,
+    setDarkMode: (DarkModeStatus) -> Unit
+) {
     BasicPopupSetting(
         title = stringResource(id = R.string.dark_mode_title),
         leadingIcon = null,
@@ -127,8 +133,13 @@ fun DarkModeSetting(currentDarkModeStatus: DarkModeStatus, setDarkMode: (DarkMod
 }
 
 @Composable
-fun FlashModeSetting(currentFlashMode: FlashModeStatus, setFlashMode: (FlashModeStatus) -> Unit) {
+fun FlashModeSetting(
+    modifier: Modifier = Modifier,
+    currentFlashMode: FlashModeStatus,
+    setFlashMode: (FlashModeStatus) -> Unit
+) {
     BasicPopupSetting(
+        modifier = modifier,
         title = stringResource(id = R.string.flash_mode_title),
         leadingIcon = null,
         description = when (currentFlashMode) {
@@ -165,18 +176,19 @@ fun FlashModeSetting(currentFlashMode: FlashModeStatus, setFlashMode: (FlashMode
 
 @Composable
 fun BasicPopupSetting(
+    modifier: Modifier = Modifier,
     title: String,
     description: String?,
+    enabled: Boolean = true,
     leadingIcon: @Composable (() -> Unit)?,
     popupContents: @Composable () -> Unit
 ) {
     val popupStatus = remember { mutableStateOf(false) }
     SettingUI(
-        modifier = Modifier.clickable() { popupStatus.value = true },
+        modifier = modifier.clickable(enabled = enabled) { popupStatus.value = true },
         title = title,
         description = description,
         leadingIcon = leadingIcon,
-        //onClick = { popupStatus.value = true },
         trailingContent = null
     )
     if (popupStatus.value) {
@@ -201,6 +213,7 @@ fun BasicPopupSetting(
  */
 @Composable
 fun SwitchSettingUI(
+    modifier: Modifier = Modifier,
     title: String,
     description: String?,
     leadingIcon: @Composable (() -> Unit)?,
@@ -209,19 +222,17 @@ fun SwitchSettingUI(
     enabled: Boolean
 ) {
     SettingUI(
-        modifier = Modifier.toggleable(
+        modifier = modifier.toggleable(
+            enabled = enabled,
             role = Role.Switch,
             value = settingValue,
             onValueChange = { _ -> onClick() }
         ),
-        //enabled = enabled,
         title = title,
         description = description,
         leadingIcon = leadingIcon,
-        //onClick = onClick,
         trailingContent = {
             Switch(
-                //modifier = Modifier,
                 enabled = enabled,
                 checked = settingValue,
                 onCheckedChange = {
@@ -243,11 +254,10 @@ fun SettingUI(
     description: String? = null,
     leadingIcon: @Composable (() -> Unit)?,
     trailingContent: @Composable (() -> Unit)?,
-    //onClick: () -> Unit,
-    //enabled: Boolean = true
-) {
+
+    ) {
     ListItem(
-        modifier = modifier, //Modifier.clickable(enabled = enabled) { onClick() },
+        modifier = modifier,
         headlineText = { Text(title) },
         supportingText = when (description) {
             null -> null