Skip to content

Centralize wallet selector#728

Merged
corbanbrook merged 1 commit into
masterfrom
centralized-wallet-selector
Apr 21, 2025
Merged

Centralize wallet selector#728
corbanbrook merged 1 commit into
masterfrom
centralized-wallet-selector

Conversation

@Agusx1211
Copy link
Copy Markdown
Member

Centralized wallet selectors handles both redirect and non-redirect cases

@Agusx1211 Agusx1211 requested review from a team as code owners April 21, 2025 15:42
@Agusx1211 Agusx1211 requested review from Copilot and removed request for a team April 21, 2025 15:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR centralizes the wallet selector logic to handle both redirect and non-redirect sign-up cases. Key changes include replacing the onExistingWallets/onExistingWalletsWithTarget callbacks with a unified WalletSelectionUiHandler, adding new properties (target and isRedirect) to AuthCodePkceSignupArgs, and exposing wallet selector registration functions in the Wallets and Manager classes.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/wallet/wdk/src/sequence/wallets.ts Added WalletSelectionUiHandler support, new signup args properties, and methods to register/unregister wallet selectors
packages/wallet/wdk/src/sequence/types/wallet.ts Defined new types for wallet selector options, context, and result
packages/wallet/wdk/src/sequence/manager.ts Forwarded wallet selector registration/unregistration functions to the Wallets module
Comments suppressed due to low confidence (1)

packages/wallet/wdk/src/sequence/wallets.ts:47

  • [nitpick] The new 'target' property in AuthCodePkceSignupArgs might benefit from a more descriptive name (e.g., 'redirectTarget') to clarify its purpose for developers reading the code.
target: string

Comment thread packages/wallet/wdk/src/sequence/wallets.ts
@corbanbrook corbanbrook merged commit 2366abf into master Apr 21, 2025
3 checks passed
@corbanbrook corbanbrook deleted the centralized-wallet-selector branch April 21, 2025 16:04
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.

3 participants