Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
demolaf wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
64 changes: 38 additions & 26 deletions app/src/main/java/com/firebaseui/android/demo/CustomMethodPickerDemoActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.safeDrawingPadding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.LazyRow
Expand Down Expand Up @@ -51,7 +52,6 @@ import com.firebase.ui.auth.configuration.theme.AuthUIAsset
import com.firebase.ui.auth.configuration.theme.AuthUITheme
import com.firebase.ui.auth.configuration.theme.ProviderStyleDefaults
import com.firebase.ui.auth.ui.components.AuthProviderButton
import com.firebase.ui.auth.ui.method_picker.MethodPickerTermsConfiguration
import com.firebase.ui.auth.ui.screens.FirebaseAuthScreen

class CustomMethodPickerDemoActivity : ComponentActivity() {
Expand Down Expand Up @@ -141,32 +141,19 @@ class CustomMethodPickerDemoActivity : ComponentActivity() {
Log.d("CustomMethodPickerDemo", "Auth cancelled")
},
customMethodPickerLayout = { providers, onProviderSelected ->
// customMethodPickerLayout now renders as the entire screen (no
// built-in logo/ToS footer/inset handling), so the terms checkbox
// that used to live in customMethodPickerTermsConfiguration is
// rendered inline here instead, and this composable owns its own
// insets via Modifier.safeDrawingPadding() in SpotlightMethodPicker.
SpotlightMethodPicker(
providers = providers,
onProviderSelected = onProviderSelected,
enabled = termsAccepted
enabled = termsAccepted,
termsAccepted = termsAccepted,
onTermsAcceptedChange = { termsAccepted = it }
)
},
customMethodPickerTermsConfiguration = MethodPickerTermsConfiguration(
content = {
Row(
modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp),
verticalAlignment = Alignment.CenterVertically
) {
Checkbox(
checked = termsAccepted,
onCheckedChange = { termsAccepted = it }
)
Text(
text = "I have read and accept the Terms of Service and Privacy Policy",
style = MaterialTheme.typography.bodySmall,
modifier = Modifier.padding(start = 8.dp)
)
}
},
accepted = termsAccepted,
disableProvidersUntilAccepted = true,
),
)
}
}
Expand All @@ -179,6 +166,8 @@ fun SpotlightMethodPicker(
providers: List<AuthProvider>,
onProviderSelected: (AuthProvider) -> Unit,
enabled: Boolean = true,
termsAccepted: Boolean = true,
onTermsAcceptedChange: (Boolean) -> Unit = {},
) {
val stringProvider = LocalAuthUIStringProvider.current

Expand All @@ -196,7 +185,11 @@ fun SpotlightMethodPicker(
val anonymous = groups["anonymous"]?.firstOrNull()

LazyColumn(
modifier = Modifier.fillMaxSize(),
// customMethodPickerLayout now renders as the entire screen, so this composable is
// responsible for its own insets.
modifier = Modifier
.fillMaxSize()
.safeDrawingPadding(),
contentPadding = PaddingValues(vertical = 48.dp),
verticalArrangement = Arrangement.spacedBy(12.dp),
horizontalAlignment = Alignment.CenterHorizontally,
Expand Down Expand Up @@ -289,11 +282,29 @@ fun SpotlightMethodPicker(
}
}
}

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

@gemini-code-assist gemini-code-assist Bot Jul 22, 2026

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
                )

Text(
text = "I have read and accept the Terms of Service and Privacy Policy",
style = MaterialTheme.typography.bodySmall,
modifier = Modifier.padding(start = 8.dp)
)
}
}
}
}

@Composable
private fun ProviderIconButton(
fun ProviderIconButton(
style: AuthUITheme.ProviderStyle,
contentDescription: String,
onClick: () -> Unit,
Expand Down Expand Up @@ -330,12 +341,12 @@ private fun ProviderIconButton(
}

@Composable
private fun AuthUIAsset.asPainter(): Painter = when (this) {
fun AuthUIAsset.asPainter(): Painter = when (this) {
is AuthUIAsset.Resource -> painterResource(resId)
is AuthUIAsset.Vector -> rememberVectorPainter(image)
}

private fun styleForProvider(provider: AuthProvider): AuthUITheme.ProviderStyle = when (provider) {
fun styleForProvider(provider: AuthProvider): AuthUITheme.ProviderStyle = when (provider) {
is AuthProvider.Facebook -> ProviderStyleDefaults.Facebook
is AuthProvider.Twitter -> ProviderStyleDefaults.Twitter
is AuthProvider.Github -> ProviderStyleDefaults.Github
Expand All @@ -346,6 +357,7 @@ private fun styleForProvider(provider: AuthProvider): AuthUITheme.ProviderStyle
backgroundColor = provider.buttonColor ?: Color(0xFF666666),
contentColor = provider.contentColor ?: Color.White
)

else -> AuthUITheme.ProviderStyle(
icon = null,
backgroundColor = Color(0xFF666666),
Expand Down
42 changes: 29 additions & 13 deletions auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import androidx.compose.animation.core.tween
import androidx.compose.animation.fadeIn
import androidx.compose.animation.fadeOut
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
Expand Down Expand Up @@ -104,6 +105,14 @@ import kotlinx.coroutines.tasks.await
* @param authenticatedContent Optional slot that allows callers to render the authenticated
* state themselves. When provided, it receives the current [AuthState] alongside an
* [AuthSuccessUiContext] containing common callbacks (sign out, manage MFA, reload user).
* @param customMethodPickerLayout Optional slot that fully replaces the method-picker screen.
* When provided, it renders as the *entire* screen content — edge-to-edge, with no logo, no
* Terms of Service/Privacy Policy footer, and no automatic system-inset handling. The caller is
* responsible for its own insets (e.g. `Modifier.safeDrawingPadding()`) and for displaying any
* required legal disclosures. [customMethodPickerTermsConfiguration] is ignored when this is set.
* @param customMethodPickerTermsConfiguration Optional custom Terms of Service/Privacy Policy
* footer for the *default* method-picker layout. Ignored when [customMethodPickerLayout] is
* provided, since that slot takes over the whole screen.
*
* @since 10.0.0
*/
Expand Down Expand Up @@ -207,19 +216,26 @@ fun FirebaseAuthScreen(
}
) {
composable(AuthRoute.MethodPicker.route) {
Scaffold { innerPadding ->
AuthMethodPicker(
modifier = modifier
.padding(innerPadding),
providers = configuration.providers,
logo = logoAsset,
termsOfServiceUrl = configuration.tosUrl,
privacyPolicyUrl = configuration.privacyPolicyUrl,
lastSignInPreference = lastSignInPreference.value,
customLayout = customMethodPickerLayout,
termsConfiguration = customMethodPickerTermsConfiguration,
onProviderSelected = onProviderSelected,
)
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()) {

@gemini-code-assist gemini-code-assist Bot Jul 22, 2026

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

customMethodPickerLayout(configuration.providers, onProviderSelected)
}
} else {
Scaffold { innerPadding ->
AuthMethodPicker(
modifier = modifier
.padding(innerPadding),
Comment on lines +228 to +229

@gemini-code-assist gemini-code-assist Bot Jul 22, 2026

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),

providers = configuration.providers,
logo = logoAsset,
termsOfServiceUrl = configuration.tosUrl,
privacyPolicyUrl = configuration.privacyPolicyUrl,
lastSignInPreference = lastSignInPreference.value,
termsConfiguration = customMethodPickerTermsConfiguration,
onProviderSelected = onProviderSelected,
)
}
}
}

Expand Down
Loading
Loading