refactor(fe): replace route-based authorize views with store-driven rendering#3781
refactor(fe): replace route-based authorize views with store-driven rendering#3781
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the new-styling authorization UX by consolidating multiple route groups into a single /authorize route and switching view selection to be driven primarily by store state (and URL search params for OpenID init/resume), including a unified redirect animation and a promise-based authorization handoff.
Changes:
- Consolidates route-based authorize flow into a single
/authorizepage/layout with store-driven rendering. - Adds OpenID init/resume handling via URL search params and inlined
onMountlogic. - Introduces a unified redirect animation view and updates authorization to accept
Promise<bigint | undefined>.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/frontend/src/routes/(new-styling)/callback/+page.svelte | Redirect OpenID callback handling to /authorize with flow=openid-resume. |
| src/frontend/src/routes/(new-styling)/authorize/+page.ts | Adds load-time flow detection for normal/OpenID init/OpenID resume. |
| src/frontend/src/routes/(new-styling)/authorize/+layout.svelte | New unified authorize layout handling channel establishment, errors, and shared chrome. |
| src/frontend/src/routes/(new-styling)/authorize/+page.svelte | New single authorize page selecting views by store state and flow. |
| src/frontend/src/routes/(new-styling)/authorize/views/AuthWizardView.svelte | Extracts auth wizard view and switches to prop-driven handlers. |
| src/frontend/src/routes/(new-styling)/authorize/views/ContinueView.svelte | Refactors continue/account-selection view to use props and promise-based authorization. |
| src/frontend/src/routes/(new-styling)/authorize/views/UpgradeSuccessView.svelte | Refactors upgrade-success view to call an injected onAuthorize handler. |
| src/frontend/src/routes/(new-styling)/authorize/views/RedirectAnimationView.svelte | Adds new shared redirect animation view with 10s fallback dialog. |
| src/frontend/src/lib/utils/reroute.ts | Removes reroute special-casing for ?openid=... since /authorize now handles it. |
| src/frontend/src/lib/stores/channelHandlers/delegation.ts | Awaits promised account number concurrently with authentication before delegating. |
| src/frontend/src/lib/stores/authorization.store.ts | Changes authorized payload to hold a promise-based account number. |
| src/frontend/src/routes/(new-styling)/(resuming-channel)/resume-openid-authorize/+page.svelte | Removes obsolete OpenID resume route page. |
| src/frontend/src/routes/(new-styling)/(resuming-channel)/+layout.svelte | Removes obsolete resuming-channel layout. |
| src/frontend/src/routes/(new-styling)/(pending-channel)/init-openid-authorize/+page.ts | Removes obsolete OpenID init load route. |
| src/frontend/src/routes/(new-styling)/(pending-channel)/init-openid-authorize/+page.svelte | Removes obsolete OpenID init redirect page. |
| src/frontend/src/routes/(new-styling)/(pending-channel)/+layout.svelte | Removes obsolete pending-channel layout. |
| src/frontend/src/routes/(new-styling)/(channel)/authorize/upgrade-success/+layout.ts | Removes obsolete route guard for upgrade-success child route. |
| src/frontend/src/routes/(new-styling)/(channel)/authorize/+layout.svelte | Removes obsolete channel/authorize layout (including old redirect screen). |
| src/frontend/src/routes/(new-styling)/(channel)/authorize/(panel)/+page.ts | Removes obsolete first-visit redirect logic to /authorize/continue. |
| src/frontend/src/routes/(new-styling)/(channel)/authorize/(panel)/+layout.svelte | Removes obsolete panel layout wrapper for authorize flow. |
| src/frontend/src/routes/(new-styling)/(channel)/+layout.svelte | Removes obsolete channel layout now replaced by unified /authorize layout. |
Comments suppressed due to low confidence (1)
src/frontend/src/routes/(new-styling)/authorize/views/ContinueView.svelte:109
accountNumberPromisecan reject (e.g.,get_default_account/throwCanisterError), but it's passed toonAuthorizewithout being awaited/handled. That means the UI commits to the authorized/redirect-animation state even if account resolution fails, and the error won't be surfaced by thistry/catch. Consider resolving the account number before callingonAuthorize, or ensure the promise passed toonAuthorizenever rejects (e.g., handle errors before authorizing and avoid setting authorized state on failure).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
808939e to
c5ece6e
Compare
2a3f202 to
62de7bb
Compare
6cc5f0f to
824ac47
Compare
62de7bb to
0d7f581
Compare
824ac47 to
5443c5f
Compare
0d7f581 to
7d43bc7
Compare
5443c5f to
a2dd683
Compare
2d530e4 to
ba1a45b
Compare
fbc1029 to
858e794
Compare
27691ce to
0f231f6
Compare
…architecture (dfinity#3777) The authorize flow previously had request handling intertwined with UI components — handlers would trigger UI changes directly, and UI components would drive handler logic. This made the flow hard to follow and brittle to change. This PR introduces a clean separation: **handlers ← stores → UI**. Channel handlers write to stores and observe stores (via `waitForStore`). The UI reads stores to decide what to render, and writes to stores when the user takes action. Neither side knows about the other. # Changes - **Extract channel handlers into dedicated files.** `channelHandlers/icrc25.ts` (standards, permissions), `channelHandlers/delegation.ts` (ICRC-34), and `channelHandlers/attributes.ts` (legacy + ICRC-3 attributes) — each self-contained with their own imports. - **Split authorization store into context and outcome.** `authorizationStore` holds the request context (`effectiveOrigin`), `authorizedStore` holds the outcome (`accountNumber`). Handlers `waitForStore(authorizedStore)` instead of polling a combined state. - **Add `waitForStore` utility.** Generic helper that resolves when a Svelte store value satisfies a condition — eliminates repeated subscribe/unsubscribe boilerplate across handlers. - **Gate attribute handlers on dapp opt-in.** Attributes are only served to known dapps with `certifiedAttributes: true`, or in dev environments (`fetch_root_key`). Unknown origins get an error response. - **Channel error component.** Extracted `ChannelError.svelte` with per-error-type messages, shared across channel, pending-channel, and resuming-channel layouts. - **Fix pending/resuming channel layout guards.** Pending-channel now renders errors. Resuming-channel waits for channel establishment before rendering children, shows errors when appropriate. - **Remove `AuthorizationChannel.svelte`.** Layouts call `channelStore.establish()` directly and gate rendering on store state. --- <div align="left">← Previous: dfinity#3791</div> <div align="right">Next: dfinity#3781 →</div>
Replace three route groups (channel, pending-channel, resuming-channel) with a single /authorize route. URL params determine the channel mode, store state drives which view renders — no child route navigation.
0f231f6 to
578a840
Compare
aterga
left a comment
There was a problem hiding this comment.
Pre-approving, although the CI is red
My biggest wish for this PR is a document the code a bit better to aid navigating
The unified /authorize refactor dropped the pendingOpenIdIssuerStore.set call from the deleted resume-openid-authorize page without migrating it to the new resumeOpenId() handler. Without it, handleLegacyAttributes early-returns and never responds to the RP, so the II window never closes after auth — which is what the e2e attribute tests wait on. Also address PR review feedback: - Document when onAuthorize is invoked on UpgradeSuccessView and ContinueView (aterga). - Reword the redirect-animation branch comment on +page.svelte (aterga).
The authorize flow previously used three route groups with child routes to render different views — making the flow scattered and hard to follow. Users navigated between
/authorize,/authorize/continue, and/authorize/upgrade-success, with separate routes for OpenID init and resume flows.This PR consolidates everything into a single
/authorizeroute. URL search params (?openid=<issuer>,?flow=openid-resume) determine the channel establishment mode, and store state drives which view renders — no more child route navigation.Changes
/authorizeroute replaces three route groups.(channel),(pending-channel), and(resuming-channel)are deleted. One layout handles all channel modes based on URL params.AuthWizardView,ContinueView,UpgradeSuccessView, andRedirectAnimationVieware co-located with the route underauthorize/views/.ProgressRingredirect screen, with a breathing pulse after the draw completes and a 10s fallback dialog.authorize()acceptsPromise<bigint | undefined>so the redirect animation can start immediately while account resolution continues in the background.ContinueViewandUpgradeSuccessViewreceiveeffectiveOriginandonAuthorizeas props rather than importing authorization stores directly.onMount— no separate component for something that doesn't render.