Skip to content

Commit

Permalink
Fix Compose issues (mostly modifier order)
Browse files Browse the repository at this point in the history
  • Loading branch information
JolandaVerhoef committed Mar 29, 2024
1 parent bb84056 commit 46a3b36
Show file tree
Hide file tree
Showing 13 changed files with 185 additions and 145 deletions.
3 changes: 2 additions & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
[*.{kt,kts}]
ktlint_code_style = android_studio
ktlint_code_style = android_studio
ktlint_standard_function-naming = disabled
2 changes: 1 addition & 1 deletion .idea/kotlinc.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 9 additions & 6 deletions app/src/main/java/com/google/jetpackcamera/ui/JcaApp.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,21 @@ import com.google.jetpackcamera.ui.Routes.SETTINGS_ROUTE
fun JcaApp(
onPreviewViewModel: (PreviewViewModel) -> Unit,
/*TODO(b/306236646): remove after still capture*/
previewMode: PreviewMode
previewMode: PreviewMode,
modifier: Modifier = Modifier
) {
val permissionState =
rememberPermissionState(permission = Manifest.permission.CAMERA)

if (permissionState.status.isGranted) {
JetpackCameraNavHost(
onPreviewViewModel = onPreviewViewModel,
previewMode = previewMode
previewMode = previewMode,
modifier = modifier
)
} else {
CameraPermission(
modifier = Modifier.fillMaxSize(),
modifier = modifier.fillMaxSize(),
cameraPermissionState = permissionState
)
}
Expand All @@ -61,10 +63,11 @@ fun JcaApp(
@Composable
private fun JetpackCameraNavHost(
onPreviewViewModel: (PreviewViewModel) -> Unit,
navController: NavHostController = rememberNavController(),
previewMode: PreviewMode
previewMode: PreviewMode,
modifier: Modifier = Modifier,
navController: NavHostController = rememberNavController()
) {
NavHost(navController = navController, startDestination = PREVIEW_ROUTE) {
NavHost(navController = navController, startDestination = PREVIEW_ROUTE, modifier = modifier) {
composable(PREVIEW_ROUTE) {
PreviewScreen(
onPreviewViewModel = onPreviewViewModel,
Expand Down
24 changes: 12 additions & 12 deletions app/src/main/java/com/google/jetpackcamera/ui/PermissionsUi.kt
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import com.google.jetpackcamera.R

@OptIn(ExperimentalPermissionsApi::class)
@Composable
fun CameraPermission(modifier: Modifier = Modifier, cameraPermissionState: PermissionState) {
fun CameraPermission(cameraPermissionState: PermissionState, modifier: Modifier = Modifier) {
PermissionTemplate(
modifier = modifier,
permissionState = cameraPermissionState,
Expand All @@ -64,14 +64,14 @@ fun CameraPermission(modifier: Modifier = Modifier, cameraPermissionState: Permi
@OptIn(ExperimentalPermissionsApi::class)
@Composable
fun PermissionTemplate(
modifier: Modifier = Modifier,
permissionState: PermissionState,
onSkipPermission: (() -> Unit)? = null,
painter: Painter,
iconAccessibilityText: String,
title: String,
bodyText: String,
requestButtonText: String
requestButtonText: String,
modifier: Modifier = Modifier,
onSkipPermission: (() -> Unit)? = null
) {
Column(
modifier = modifier.background(MaterialTheme.colorScheme.primary),
Expand Down Expand Up @@ -110,7 +110,7 @@ fun PermissionTemplate(
}

@Composable
fun PermissionImage(modifier: Modifier = Modifier, painter: Painter, accessibilityText: String) {
fun PermissionImage(painter: Painter, accessibilityText: String, modifier: Modifier = Modifier) {
Box(modifier = modifier) {
Icon(
modifier = Modifier
Expand All @@ -126,9 +126,9 @@ fun PermissionImage(modifier: Modifier = Modifier, painter: Painter, accessibili
@OptIn(ExperimentalPermissionsApi::class)
@Composable
fun PermissionButtonSection(
modifier: Modifier = Modifier,
permissionState: PermissionState,
requestButtonText: String,
modifier: Modifier = Modifier,
onSkipPermission: (() -> Unit)?
) {
Box(modifier = modifier) {
Expand Down Expand Up @@ -159,9 +159,9 @@ fun PermissionButtonSection(
@OptIn(ExperimentalPermissionsApi::class)
@Composable
fun PermissionButton(
modifier: Modifier = Modifier,
permissionState: PermissionState,
requestButtonText: String
requestButtonText: String,
modifier: Modifier = Modifier
) {
Button(
modifier = modifier,
Expand All @@ -181,13 +181,13 @@ fun PermissionButton(
}

@Composable
fun PermissionText(modifier: Modifier = Modifier, title: String, bodyText: String) {
fun PermissionText(title: String, bodyText: String, modifier: Modifier = Modifier) {
Box(
modifier = modifier
.height(IntrinsicSize.Min)
) {
Column(
modifier = modifier
modifier = Modifier
.fillMaxSize()
.align(Alignment.Center)
) {
Expand All @@ -213,7 +213,7 @@ fun PermissionText(modifier: Modifier = Modifier, title: String, bodyText: Strin
}

@Composable
fun PermissionTitleText(modifier: Modifier = Modifier, text: String, color: Color) {
fun PermissionTitleText(text: String, color: Color, modifier: Modifier = Modifier) {
Text(
modifier = modifier,
color = color,
Expand All @@ -224,7 +224,7 @@ fun PermissionTitleText(modifier: Modifier = Modifier, text: String, color: Colo
}

@Composable
fun PermissionBodyText(modifier: Modifier = Modifier, text: String, color: Color) {
fun PermissionBodyText(text: String, color: Color, modifier: Modifier = Modifier) {
Text(
modifier = modifier,
color = color,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import android.view.Display
import androidx.camera.core.SurfaceRequest
import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.size
Expand Down Expand Up @@ -63,8 +64,9 @@ private const val TAG = "PreviewScreen"
fun PreviewScreen(
onPreviewViewModel: (PreviewViewModel) -> Unit,
onNavigateToSettings: () -> Unit,
viewModel: PreviewViewModel = hiltViewModel(),
previewMode: PreviewMode
previewMode: PreviewMode,
modifier: Modifier = Modifier,
viewModel: PreviewViewModel = hiltViewModel()
) {
Log.d(TAG, "PreviewScreen")
onPreviewViewModel(viewModel)
Expand All @@ -85,8 +87,9 @@ fun PreviewScreen(
}

when (previewUiState.cameraState) {
CameraState.NOT_READY -> LoadingScreen()
CameraState.NOT_READY -> LoadingScreen(modifier)
CameraState.READY -> ContentScreen(
modifier = modifier,
previewUiState = previewUiState,
previewMode = previewMode,
screenFlashUiState = screenFlashUiState,
Expand Down Expand Up @@ -115,6 +118,7 @@ private fun ContentScreen(
previewMode: PreviewMode,
screenFlashUiState: ScreenFlash.ScreenFlashUiState,
surfaceRequest: SurfaceRequest?,
modifier: Modifier = Modifier,
onNavigateToSettings: () -> Unit = {},
onClearUiScreenBrightness: (Float) -> Unit = {},
onSetLensFacing: (newLensFacing: LensFacing) -> Unit = {},
Expand Down Expand Up @@ -147,66 +151,68 @@ private fun ContentScreen(

val scope = rememberCoroutineScope()
val blinkState = remember { BlinkState(coroutineScope = scope) }
// display camera feed. this stays behind everything else
PreviewDisplay(
onFlipCamera = onFlipCamera,
onTapToFocus = onTapToFocus,
onZoomChange = onChangeZoomScale,
aspectRatio = previewUiState.currentCameraSettings.aspectRatio,
surfaceRequest = surfaceRequest,
blinkState = blinkState
)

QuickSettingsScreenOverlay(
modifier = Modifier,
isOpen = previewUiState.quickSettingsIsOpen,
toggleIsOpen = onToggleQuickSettings,
currentCameraSettings = previewUiState.currentCameraSettings,
onLensFaceClick = onSetLensFacing,
onFlashModeClick = onChangeFlash,
onAspectRatioClick = onChangeAspectRatio,
onCaptureModeClick = onChangeCaptureMode
// onTimerClick = {}/*TODO*/
)
// relative-grid style overlay on top of preview display
CameraControlsOverlay(
previewUiState = previewUiState,
onNavigateToSettings = onNavigateToSettings,
previewMode = previewMode,
onFlipCamera = onFlipCamera,
onChangeFlash = onChangeFlash,
onToggleQuickSettings = onToggleQuickSettings,
onCaptureImage = onCaptureImage,
onCaptureImageWithUri = onCaptureImageWithUri,
onStartVideoRecording = onStartVideoRecording,
onStopVideoRecording = onStopVideoRecording,
blinkState = blinkState
)

// displays toast when there is a message to show
if (previewUiState.toastMessageToShow != null) {
TestableToast(
modifier = Modifier.testTag(previewUiState.toastMessageToShow.testTag),
toastMessage = previewUiState.toastMessageToShow,
onToastShown = onToastShown
Box(modifier.fillMaxSize()) {
// display camera feed. this stays behind everything else
PreviewDisplay(
onFlipCamera = onFlipCamera,
onTapToFocus = onTapToFocus,
onZoomChange = onChangeZoomScale,
aspectRatio = previewUiState.currentCameraSettings.aspectRatio,
surfaceRequest = surfaceRequest,
blinkState = blinkState
)
}

// Screen flash overlay that stays on top of everything but invisible normally. This should
// not be enabled based on whether screen flash is enabled because a previous image capture
// may still be running after flash mode change and clear actions (e.g. brightness restore)
// may need to be handled later. Compose smart recomposition should be able to optimize this
// if the relevant states are no longer changing.
ScreenFlashScreen(
screenFlashUiState = screenFlashUiState,
onInitialBrightnessCalculated = onClearUiScreenBrightness
)
QuickSettingsScreenOverlay(
modifier = Modifier,
isOpen = previewUiState.quickSettingsIsOpen,
toggleIsOpen = onToggleQuickSettings,
currentCameraSettings = previewUiState.currentCameraSettings,
onLensFaceClick = onSetLensFacing,
onFlashModeClick = onChangeFlash,
onAspectRatioClick = onChangeAspectRatio,
onCaptureModeClick = onChangeCaptureMode
// onTimerClick = {}/*TODO*/
)
// relative-grid style overlay on top of preview display
CameraControlsOverlay(
previewUiState = previewUiState,
onNavigateToSettings = onNavigateToSettings,
previewMode = previewMode,
onFlipCamera = onFlipCamera,
onChangeFlash = onChangeFlash,
onToggleQuickSettings = onToggleQuickSettings,
onCaptureImage = onCaptureImage,
onCaptureImageWithUri = onCaptureImageWithUri,
onStartVideoRecording = onStartVideoRecording,
onStopVideoRecording = onStopVideoRecording,
blinkState = blinkState
)

// displays toast when there is a message to show
if (previewUiState.toastMessageToShow != null) {
TestableToast(
modifier = Modifier.testTag(previewUiState.toastMessageToShow.testTag),
toastMessage = previewUiState.toastMessageToShow,
onToastShown = onToastShown
)
}

// Screen flash overlay that stays on top of everything but invisible normally. This should
// not be enabled based on whether screen flash is enabled because a previous image capture
// may still be running after flash mode change and clear actions (e.g. brightness restore)
// may need to be handled later. Compose smart recomposition should be able to optimize this
// if the relevant states are no longer changing.
ScreenFlashScreen(
screenFlashUiState = screenFlashUiState,
onInitialBrightnessCalculated = onClearUiScreenBrightness
)
}
}

@Composable
private fun LoadingScreen() {
private fun LoadingScreen(modifier: Modifier = Modifier) {
Column(
modifier = Modifier
modifier = modifier
.fillMaxSize()
.background(Color.Black),
verticalArrangement = Arrangement.Center,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,11 @@ class ZoomLevelDisplayState(showInitially: Boolean = false) {
@Composable
fun CameraControlsOverlay(
previewUiState: PreviewUiState,
zoomLevelDisplayState: ZoomLevelDisplayState = remember { ZoomLevelDisplayState() },
onNavigateToSettings: () -> Unit,
previewMode: PreviewMode,
blinkState: BlinkState,
modifier: Modifier = Modifier,
zoomLevelDisplayState: ZoomLevelDisplayState = remember { ZoomLevelDisplayState() },
onNavigateToSettings: () -> Unit = {},
onFlipCamera: () -> Unit = {},
onChangeFlash: (FlashMode) -> Unit = {},
onToggleQuickSettings: () -> Unit = {},
Expand All @@ -81,8 +83,7 @@ fun CameraControlsOverlay(
(PreviewViewModel.ImageCaptureEvent) -> Unit
) -> Unit = { _, _, _, _ -> },
onStartVideoRecording: () -> Unit = {},
onStopVideoRecording: () -> Unit = {},
blinkState: BlinkState
onStopVideoRecording: () -> Unit = {}
) {
// Show the current zoom level for a short period of time, only when the level changes.
var firstRun by remember { mutableStateOf(true) }
Expand All @@ -95,7 +96,7 @@ fun CameraControlsOverlay(
}

CompositionLocalProvider(LocalContentColor provides Color.White) {
Box(Modifier.fillMaxSize()) {
Box(modifier.fillMaxSize()) {
if (previewUiState.videoRecordingState == VideoRecordingState.INACTIVE) {
ControlsTop(
modifier = Modifier
Expand Down Expand Up @@ -144,10 +145,10 @@ private fun ControlsTop(
Row(Modifier.weight(1f), verticalAlignment = Alignment.CenterVertically) {
// button to open default settings page
SettingsNavButton(
Modifier
modifier = Modifier
.padding(12.dp)
.testTag(SETTINGS_BUTTON),
onNavigateToSettings
onNavigateToSettings = onNavigateToSettings
)
if (!isQuickSettingsOpen) {
QuickSettingsIndicators(
Expand Down Expand Up @@ -208,7 +209,7 @@ private fun ControlsBottom(
Row(Modifier.weight(1f), horizontalArrangement = Arrangement.SpaceEvenly) {
if (!isQuickSettingsOpen && videoRecordingState == VideoRecordingState.INACTIVE) {
FlipCameraButton(
modifier = modifier.testTag(FLIP_CAMERA_BUTTON),
modifier = Modifier.testTag(FLIP_CAMERA_BUTTON),
onClick = onFlipCamera,
// enable only when phone has front and rear camera
enabledCondition = currentCameraSettings.isBackCameraAvailable &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ import kotlinx.coroutines.launch
*/
@Composable
fun CameraXViewfinder(
modifier: Modifier = Modifier,
surfaceRequest: SurfaceRequest,
modifier: Modifier = Modifier,
implementationMode: ImplementationMode = ImplementationMode.PERFORMANCE
) {
val currentImplementationMode by rememberUpdatedState(implementationMode)
Expand Down
Loading

0 comments on commit 46a3b36

Please sign in to comment.