Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import androidx.compose.ui.platform.testTag
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.input.PasswordVisualTransformation
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.TextUnit
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import androidx.lifecycle.compose.collectAsStateWithLifecycle
Expand Down Expand Up @@ -267,6 +268,7 @@ private fun Body(
private fun ColumnScope.MainContent(
state: State,
onEventSent: (event: Event) -> Unit,
textFontSize: TextUnit = 16.sp,
) {
Comment on lines 268 to 272
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MainContent introduces textFontSize, but the only call site in this file (BodyMainContent(state, onEventSent)) doesn’t pass it, so it will always be the default. Either pass a value from state/config (e.g., state.titleFontSize) or remove this parameter to avoid dead customization points.

Copilot uses AI. Check for mistakes.
when (val mode = state.config.mode) {
is BiometricMode.Default -> {
Expand Down Expand Up @@ -300,8 +302,8 @@ private fun ColumnScope.MainContent(
style =
MaterialTheme.typography.headlineSmall.copy(
color = MaterialTheme.colorScheme.onSurface,
fontSize = textFontSize,
),
fontSize = MaterialTheme.typography.headlineMedium.fontSize,
textAlign = TextAlign.Center,
)

Expand Down Expand Up @@ -344,9 +346,9 @@ private fun ColumnScope.MainContent(
Text(
text = subtitle,
style =
MaterialTheme.typography.headlineSmall.copy(
MaterialTheme.typography.labelLarge.copy(
color = MaterialTheme.colorScheme.onSurface,
fontSize = 16.sp,
fontSize = textFontSize,
),
textAlign = TextAlign.Center,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package com.k689.identid.ui.common.biometric

import android.content.Context
import android.net.Uri
import androidx.compose.ui.unit.TextUnit
import androidx.compose.ui.unit.sp
import androidx.lifecycle.viewModelScope
import com.k689.identid.R
import com.k689.identid.config.BiometricUiConfig
Expand Down Expand Up @@ -79,6 +81,7 @@ data class State(
val notifyOnAuthenticationFailure: Boolean = true,
val quickPinSize: Int = 6,
val isPinInputEnabled: Boolean = true,
val titleFontSize: TextUnit = 16.sp,
) : ViewState

sealed class Effect : ViewSideEffect {
Expand Down
125 changes: 65 additions & 60 deletions app/src/main/java/com/k689/identid/ui/common/pin/PinScreen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ package com.k689.identid.ui.common.pin
import android.content.Context
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.ColumnScope
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.rememberScrollState
import androidx.compose.foundation.verticalScroll
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.SheetState
Expand All @@ -39,9 +39,11 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.text.input.PasswordVisualTransformation
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.TextUnit
import androidx.compose.ui.unit.dp
import androidx.compose.ui.unit.sp
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.navigation.NavController
import com.k689.identid.R
Expand Down Expand Up @@ -122,38 +124,40 @@ fun PinScreen(
}
},
) { paddingValues ->
Content(
state = state,
effectFlow = viewModel.effect,
onEventSend = { event -> viewModel.setEvent(event) },
onNavigationRequested = { navigationEffect ->
handleNavigationEffect(
context,
navigationEffect,
navController,
)
},
paddingValues = paddingValues,
coroutineScope = scope,
modalBottomSheetState = bottomSheetState,
)

if (isBottomSheetOpen) {
WrapModalBottomSheet(
onDismissRequest = {
viewModel.setEvent(
Event.BottomSheet.UpdateBottomSheetState(
isOpen = false,
),
Column {
Content(
state = state,
effectFlow = viewModel.effect,
onEventSend = { event -> viewModel.setEvent(event) },
onNavigationRequested = { navigationEffect ->
handleNavigationEffect(
context,
navigationEffect,
navController,
)
},
sheetState = bottomSheetState,
) {
SheetContent(
onEventSent = {
viewModel.setEvent(it)
paddingValues = paddingValues,
coroutineScope = scope,
modalBottomSheetState = bottomSheetState,
)

if (isBottomSheetOpen) {
WrapModalBottomSheet(
onDismissRequest = {
viewModel.setEvent(
Event.BottomSheet.UpdateBottomSheetState(
isOpen = false,
),
)
},
)
sheetState = bottomSheetState,
) {
SheetContent(
onEventSent = {
viewModel.setEvent(it)
},
)
}
}
}
}
Expand Down Expand Up @@ -189,21 +193,22 @@ private fun handleNavigationEffect(

@OptIn(ExperimentalMaterial3Api::class)
@Composable
private fun Content(
private fun ColumnScope.Content(
state: State,
effectFlow: Flow<Effect>,
onEventSend: (Event) -> Unit,
onNavigationRequested: (Effect.Navigation) -> Unit,
paddingValues: PaddingValues,
coroutineScope: CoroutineScope,
modalBottomSheetState: SheetState,
textFontSize: TextUnit = 16.sp,
) {
Column(
modifier =
Modifier
.fillMaxSize()
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Column is both fillMaxSize() and weight(1f) as a direct child of the parent Column. Combining fillMaxSize with weight is redundant and can lead to confusing measurement behavior; typically you want fillMaxWidth() with weight(1f) (or drop weight if fillMaxSize is intended).

Suggested change
.fillMaxSize()
.fillMaxWidth()

Copilot uses AI. Check for mistakes.
.padding(paddingValues)
.verticalScroll(rememberScrollState()),
.weight(1.0f),
) {
Comment on lines 206 to 212
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Content no longer scrolls. If the available height is smaller than the combined height of the header texts + PIN field (e.g., small devices, landscape, or large accessibility font), the content can become clipped with no way to reach it. Consider restoring a verticalScroll(rememberScrollState()) (or using a LazyColumn) so the PIN entry remains reachable in constrained layouts.

Copilot uses AI. Check for mistakes.
VSpacer.Large()
AppIconAndText(
Expand All @@ -213,45 +218,43 @@ private fun Content(
.padding(bottom = SPACING_LARGE.dp),
appIconAndTextData = AppIconAndTextDataUi(),
)

Column(
modifier =
Modifier
.fillMaxWidth()
.padding(vertical = SPACING_LARGE.dp),
.padding(vertical = SPACING_LARGE.dp)
.weight(1.0f),
verticalArrangement = Arrangement.spacedBy(SPACING_SMALL.dp, Alignment.Top),
) {
Spacer(modifier = Modifier.weight(1f))
Text(
text = state.title,
modifier = Modifier.testTag(TestTag.PinScreen.TITLE),
textAlign = TextAlign.Center,
modifier = Modifier.testTag(TestTag.PinScreen.TITLE).align(Alignment.CenterHorizontally),
style =
MaterialTheme.typography.headlineSmall.copy(
color = MaterialTheme.colorScheme.onSurface,
fontSize = textFontSize,
),
)
Text(
text = state.subtitle,
modifier = Modifier.align(Alignment.CenterHorizontally),
textAlign = TextAlign.Center,
style =
MaterialTheme.typography.headlineSmall.copy(
color = MaterialTheme.colorScheme.onSurface,
color = MaterialTheme.colorScheme.onSurface.copy(alpha = 0.6f),
fontSize = textFontSize,
),
)
}

Column(
modifier =
Modifier
.fillMaxWidth()
.padding(top = SPACING_LARGE.dp),
verticalArrangement = Arrangement.spacedBy(SPACING_SMALL.dp, Alignment.Top),
) {
PinFieldLayout(
modifier = Modifier.fillMaxWidth(),
state = state,
onPinInput = { quickPin ->
onEventSend(Event.OnQuickPinEntered(quickPin))
},
)
Spacer(modifier = Modifier.weight(2.8f))
}
Comment on lines 250 to 258
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacer(modifier = Modifier.weight(2.8f)) introduces a hard-to-reason-about magic ratio in the layout. Consider replacing this with a named constant (documenting the intent) or using a more deterministic arrangement (e.g., verticalArrangement = Arrangement.SpaceBetween/Center) to avoid future layout regressions.

Copilot uses AI. Check for mistakes.
}

Expand Down Expand Up @@ -321,19 +324,21 @@ private fun PinFieldLayout(
@Composable
private fun PinScreenEmptyPreview() {
PreviewTheme {
Content(
state =
State(
pinFlow = PinFlow.CREATE_WITH_ACTIVATION,
pinState = PinValidationState.ENTER,
),
effectFlow = Channel<Effect>().receiveAsFlow(),
onEventSend = {},
onNavigationRequested = {},
paddingValues = PaddingValues(10.dp),
coroutineScope = rememberCoroutineScope(),
modalBottomSheetState = rememberModalBottomSheetState(),
)
Column {
Content(
state =
State(
pinFlow = PinFlow.CREATE_WITH_ACTIVATION,
pinState = PinValidationState.ENTER,
),
effectFlow = Channel<Effect>().receiveAsFlow(),
onEventSend = {},
onNavigationRequested = {},
paddingValues = PaddingValues(10.dp),
coroutineScope = rememberCoroutineScope(),
modalBottomSheetState = rememberModalBottomSheetState(),
)
}
}
}

Expand Down