fix(auth): handle edge-to-edge insets in MFA challenge and auth screens#2412
fix(auth): handle edge-to-edge insets in MFA challenge and auth screens#2412demolaf wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the custom method picker layout to render edge-to-edge across the entire screen, adjusts system insets handling by removing safeDrawingPadding from several sub-screens, and fixes a form validation bug in SignUpUI where the sign-up button remained disabled when a display name was not required. A new unit test is also added to verify this fix. The review feedback suggests avoiding the reuse of the high-level modifier parameter inside the NavHost of FirebaseAuthScreen to prevent layout issues during navigation, adding Modifier.fillMaxSize() to the Scaffold in MfaChallengeDefaults.kt to ensure full-screen coverage, and making the entire terms checkbox row clickable in the demo activity to improve accessibility.
| if (customMethodPickerLayout != null) { | ||
| // Takes over the entire screen — no logo, no ToS/Privacy footer, and no | ||
| // automatic inset handling. See the KDoc on customMethodPickerLayout. | ||
| Box(modifier = modifier.fillMaxSize()) { |
There was a problem hiding this comment.
The high-level modifier parameter of FirebaseAuthScreen should be applied to the root Surface of the screen (at line 198) rather than being passed down to individual child screens inside the NavHost.
Reusing the high-level modifier here is problematic because any modifier styling (such as background, padding, etc.) passed by the caller will only apply to the MethodPicker screen and will suddenly disappear when the user navigates to other screens in the flow (like EmailAuthScreen or PhoneAuthScreen).
The root Surface should be updated to use modifier, and this inner Box should use Modifier.fillMaxSize() instead.
| Box(modifier = modifier.fillMaxSize()) { | |
| Box(modifier = Modifier.fillMaxSize()) { |
| modifier = modifier | ||
| .padding(innerPadding), |
There was a problem hiding this comment.
Similarly, AuthMethodPicker should use a local Modifier rather than reusing the high-level modifier parameter of FirebaseAuthScreen to ensure consistent layout and styling across all screens in the authentication flow.
| modifier = modifier | |
| .padding(innerPadding), | |
| modifier = Modifier | |
| .padding(innerPadding), |
| } | ||
|
|
||
| if (state.error != null) { | ||
| Scaffold { innerPadding -> |
There was a problem hiding this comment.
The Scaffold should be configured with Modifier.fillMaxSize() to ensure it occupies the entire screen area. Without this, the Scaffold's background color and layout might not cover the full screen on some devices or screen configurations, especially when the content is short.
| Scaffold { innerPadding -> | |
| Scaffold(modifier = Modifier.fillMaxSize()) { innerPadding -> |
| Row( | ||
| modifier = Modifier.padding(horizontal = 16.dp), | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| Checkbox( | ||
| checked = termsAccepted, | ||
| onCheckedChange = onTermsAcceptedChange | ||
| ) |
There was a problem hiding this comment.
To improve accessibility and make the touch target larger, the entire Row should be clickable to toggle the checkbox state, rather than requiring the user to tap the small checkbox directly.
We can achieve this by making the Row clickable and setting onCheckedChange to null on the Checkbox so that the click event is handled uniformly by the row.
Row(
modifier = Modifier
.clickable { onTermsAcceptedChange(!termsAccepted) }
.padding(horizontal = 16.dp, vertical = 8.dp),
verticalAlignment = Alignment.CenterVertically
) {
Checkbox(
checked = termsAccepted,
onCheckedChange = null
)
Fixes #2403.
The MFA challenge screen had no inset handling, so its title collided with the status bar/camera cutout on edge-to-edge devices. Also found a related, opposite bug in six other auth screens combining Scaffold's innerPadding with a redundant, double-counted safeDrawingPadding() call.
MfaChallengeDefaults.kt: wrappedDefaultMfaChallengeContentin aScaffoldSignInUI,SignUpUI,SignInEmailLinkUI,ResetPasswordUI,EnterPhoneNumberUI,EnterVerificationCodeUI: removed the redundant.safeDrawingPadding(), added a uniform16.dpmarginFirebaseAuthScreen.customMethodPickerLayoutnow renders full-screen (was Scaffold-confined) so fully custom layouts can go edge-to-edge; direct behavior change since library is pre-1.0Preview