Repository navigation
Conversation
📊 Build Bundle StatsThe latest build generated the following assets: |
|
✅ No security or compliance issues detected. Reviewed everything up to c084f78. Security OverviewDetected Code Changes
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Restarting an in-progress login can retain iiAnswered: true, hiding the recovery prompt if the replacement popup closes prematurely.
1 open issue
What changed in this PR
This PR hides the Internet Identity reopen prompt once authentication reaches the delegation stage, preventing unnecessary login restarts while preserving the overlay layout.
Changes:
- Tracks
iiAnsweredin auth state and exposesisFinishingLogin. - Hides the recovery prompt while login finishes, retaining its space.
- Adds tests for the auth signal, failure reset, and prompt visibility.
| File | Description |
|---|---|
src/governance-app-frontend/src/routes/index.tsx |
Passes login completion state to the recovery prompt. |
src/governance-app-frontend/src/features/login/components/LoginRestart.tsx |
Hides the prompt while login finishes without shifting layout. |
src/governance-app-frontend/src/features/login/components/LoginRestart.spec.tsx |
Tests delayed visibility and finishing-state hiding. |
src/governance-app-frontend/src/common/hooks/useInternetIdentity.ts |
Exposes isFinishingLogin to components. |
src/governance-app-frontend/src/common/auth/internetIdentity.ts |
Records the first auth-agent fetch during login. |
src/governance-app-frontend/src/common/auth/internetIdentity.test.ts |
Tests the fetch signal and its reset after failure. |
Review effort: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A late fetch from a replaced client can mark the replacement login as answered and prematurely hide its recovery prompt, so the callback needs a client-generation guard and a regression test.
1 open issue
1 resolved since last review
Review effort: Balanced
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Mocked tests cannot establish that the real auth SDK waits for II’s response before agent requests, including local root-key fetching and client replacement, leaving login-recovery ordering unverified.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
With auth 10, the II window closes before the first delegation exists, so the app waits about 1 s on the "Authenticating" overlay. The "Closed the window? Open Internet Identity again" prompt appears 3 s after the click, so it is on screen for that whole wait, after II succeeded. A click restarts a login that is almost done.
Changes
iiAnsweredto the auth state. The first agent call of a login sets it.isFinishingLogintouseInternetIdentity.LoginRestartonce II answered. Its space stays, so the logo does not move.Screenshots