Uh oh!
There was an error while loading. Please reload this page.
feat(auth): make reauthentication a navigation destination - #2471
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the reauthentication flow in FirebaseUI Auth by integrating it directly into the host's back stack as a wrapped destination (AuthRoute.Reauth) and managing its presentation via a custom ReauthSceneStrategy. This replaces the previous separate modal sheet and custom content implementations, which have been removed along with their state savers. Additionally, parallel sync is enabled in gradle.properties. Feedback on these changes suggests using rememberUpdatedState for more idiomatic state tracking in FirebaseAuthScreen and adding a safety check in reauthDestinations to ensure the active reauthentication request ID matches the back stack key before rendering.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
4da9222 to
9d4ce9cCompare9d4ce9c to
b21a1d6Compareb21a1d6 to
7118ce9Compare7118ce9 to
0c038b1Compare0c038b1 to
cb2cd58Comparecb2cd58 to
077b653Compare
russellwheatley
left a comment
There was a problem hiding this comment.
LGTM - leftover bits can follow up separately
Reauthentication was a host-owned
ModalBottomSheetcontaining its ownNavDisplayand back stack. It is now a single destination on the main back stack —AuthRoute.Reauth(requestId, userUid, step)— rendered as a sheet by anOverlayScenescene strategy. One stack means oneonBack, one place that can pop, and no way for two stacks to disagree about a back press.ReauthPresentationStateand its saver are gone: the marker rides on the key, sorememberNavBackStackpersists it.ReauthSheetContentandCustomReauthContentare gone too — all three were internal.ReauthContentStateis unchanged.ReauthDestinations.kt/ReauthSceneStrategy.kt: the reauth entry and the sheet/bare presentations, honouringconfiguration.transitionsbetween steps and hiding the sheet viaOverlayScene.onRemove.EmailAuthDestinations.kt: the email step body is now a sharedEmailAuthStep, so the reachability guard applies to reauth too — a restored stack holding an unoffered step no longer renders a sign-up form mid-reauthentication.RequiresMfa.The reauth surface now stays mounted through
RetryingOperationand reports it as loading, rather than disappearing and uncovering the flow behind it.Added tests for the shared reachability guard, the transition between reauth steps, the MFA challenge pop, and the retry behaviour.
Maintainer note: Fixes internal CPRN-403