Skip to content

Fix SignerPicker not showing in SmartWalletsDemo#92

Merged
tomas-martins-crossmint merged 3 commits into
mainfrom
tomas/fix-signer-picker-visibility-demo-app
May 27, 2026
Merged

Fix SignerPicker not showing in SmartWalletsDemo#92
tomas-martins-crossmint merged 3 commits into
mainfrom
tomas/fix-signer-picker-visibility-demo-app

Conversation

@tomas-martins-crossmint
Copy link
Copy Markdown
Contributor

SignerPicker was invisible even when the wallet had selectable signers. The picker used a .task attached to a Group/Section that starts empty, and SwiftUI doesn't fire .task on an empty section — so by the time wallet.signers() returned, the layout had already settled and the section never appeared.

Changes

  • AppState now owns delegatedSigners and exposes a loadSigners() method that calls wallet?.signers(). It's called after fetchBalance() in both loadWallet and switchChain (for cached wallets), so signers are always populated before any view renders.
  • loadSigners() also restores the auto-selection behavior from the old .task: if no signer is currently selected, it picks the first selectable locator and calls selectSigner.
  • SignerPicker drops the local @State var delegatedSigners and the .task entirely, reading from appState.delegatedSigners instead. Picker now shows whenever there's at least one selectable signer (was count > 1). Locator fallback fixed to signer.locator ?? signer.signer since the API can return the locator in either field.
  • SignersView.loadSigners() delegates to appState.loadSigners() and reads from appState.delegatedSigners to stay in sync.

Move delegatedSigners into AppState so it's loaded before any view renders,
removing the dependency on .task firing on an empty Section. Auto-selection
of the first signer is preserved in loadSigners(). Show picker whenever at
least one selectable signer exists (not just when count > 1). Fix locator
fallback to signer field for API responses that use either field.
@tomas-martins-crossmint tomas-martins-crossmint marked this pull request as ready for review May 21, 2026 22:19
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
Examples/SmartWalletsDemo/SmartWalletsDemo/Playground/AppState.swift:95-110
**`createWallet` skips `loadSigners()`, leaving the signer unselected**

After wallet creation, `loadSigners()` is never called, so `delegatedSigners` stays `[]` and `selectSigner` is never invoked. The `SignerPicker` binding falls back to `opts.first?.id` visually (the recovery locator), but `wallet.useSigner(config)` is never called on the underlying SDK — meaning wallet operations after creation may run without an explicitly configured signer. Both `loadWallet` and `switchChain` call `await loadSigners()` after `fetchBalance()`, but `createWallet` does not, creating an inconsistent state.

### Issue 2 of 2
Examples/SmartWalletsDemo/SmartWalletsDemo/Playground/Signers/SignersView.swift:34-44
**Inconsistency between `SignerPicker` and `SignersView` for signers where `locator` is nil**

`SignerPicker` now uses `signer.locator ?? signer.signer` so a signer where only the `signer` field is populated will appear as a selectable option in the picker. However, `SignersView` still uses `id: \.locator` for `ForEach` and guards `if let locator = signer.locator` — meaning that same signer won't be rendered in the Signers management screen and can't be removed. If the API does return entries with `locator == nil`, those signers end up in an inconsistent state: selectable but not removable.

Reviews (1): Last reviewed commit: "Fix SignerPicker not showing in SmartWal..." | Re-trigger Greptile

…sistency

Call loadSigners() in createWallet so delegatedSigners is populated and the
first signer is auto-selected immediately after creation, matching the behavior
of loadWallet and switchChain.

Precompute signerItems in SignersView using locator ?? signer so delegated
signers where only the signer field is populated are displayed and removable,
consistent with how SignerPicker already handles them.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Examples/SmartWalletsDemo/SmartWalletsDemo/Playground/Signers/SignersView.swift:14-26
The `signerItems` computed property is sandwiched between `@State` property declarations (`isLoadingSigners` above and `alertTitle`/`alertMessage`/`showAlert` below), making the property group harder to scan. Consider grouping all stored `@State` properties together and placing computed properties in a separate section.

```suggestion
    @State private var isLoadingSigners = false
    @State private var alertTitle = ""
    @State private var alertMessage = ""
    @State private var showAlert = false

    private var isRemovingSigner: Bool { removingSignerLocator != nil }

    private var signerItems: [(locator: String, model: WalletDelegatedSignerConfigApiModel)] {
        appState.delegatedSigners.compactMap { signer in
            guard let locator = signer.locator ?? signer.signer else { return nil }
            return (locator: locator, model: signer)
        }
    }
```

Reviews (2): Last reviewed commit: "Address signer state gaps after wallet c..." | Re-trigger Greptile

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Reviews (3): Last reviewed commit: "Group @State properties before computed ..." | Re-trigger Greptile

@tomas-martins-crossmint tomas-martins-crossmint merged commit aed3fc1 into main May 27, 2026
3 checks passed
@tomas-martins-crossmint tomas-martins-crossmint deleted the tomas/fix-signer-picker-visibility-demo-app branch May 27, 2026 16:35
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.

2 participants