Skip to content

fix(auth): handle edge-to-edge insets in MFA challenge and auth screens#2412

Closed
demolaf wants to merge 1 commit into
version-10.0.0-beta04from
claude/firebaseui-android-2403-1ec5f6
Closed

fix(auth): handle edge-to-edge insets in MFA challenge and auth screens#2412
demolaf wants to merge 1 commit into
version-10.0.0-beta04from
claude/firebaseui-android-2403-1ec5f6

Conversation

@demolaf

@demolaf demolaf commented Jul 22, 2026

Copy link
Copy Markdown
Member

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: wrapped DefaultMfaChallengeContent in a Scaffold
  • SignInUI, SignUpUI, SignInEmailLinkUI, ResetPasswordUI, EnterPhoneNumberUI, EnterVerificationCodeUI: removed the redundant .safeDrawingPadding(), added a uniform 16.dp margin
  • FirebaseAuthScreen.customMethodPickerLayout now renders full-screen (was Scaffold-confined) so fully custom layouts can go edge-to-edge; direct behavior change since library is pre-1.0

Preview

Screenshot_1784708851

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
Box(modifier = modifier.fillMaxSize()) {
Box(modifier = Modifier.fillMaxSize()) {

Comment on lines +228 to +229
modifier = modifier
.padding(innerPadding),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
modifier = modifier
.padding(innerPadding),
modifier = Modifier
.padding(innerPadding),

}

if (state.error != null) {
Scaffold { innerPadding ->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
Scaffold { innerPadding ->
Scaffold(modifier = Modifier.fillMaxSize()) { innerPadding ->

Comment on lines +288 to +295
Row(
modifier = Modifier.padding(horizontal = 16.dp),
verticalAlignment = Alignment.CenterVertically
) {
Checkbox(
checked = termsAccepted,
onCheckedChange = onTermsAcceptedChange
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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
                )

@demolaf
demolaf changed the base branch from master to version-10.0.0-beta04 July 22, 2026 08:58
@demolaf demolaf linked an issue Jul 22, 2026 that may be closed by this pull request
@demolaf demolaf closed this Jul 22, 2026
@demolaf
demolaf deleted the claude/firebaseui-android-2403-1ec5f6 branch July 22, 2026 09:38
@github-project-automation github-project-automation Bot moved this from Backlog to Done in studio-2394994192-60a69 Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant