fix(swift-example-app): expose Picker options to UI automation#3903
Conversation
SwiftUI `Picker`s rendered as menu popovers (the Form default, or an explicit `.pickerStyle(.menu)` / `MenuPickerStyle()`) surface only a single "Dismiss context menu" element to idb's accessibility tree — the option rows are unreachable, so UI automation can neither query nor tap a specific option. This blocked automated runs of the identity funding flows (top-up, create-from-addresses) and the token-register / builder identity pickers. Add a shared AccessiblePicker helper with two presentation modifiers: - `.accessibleFormPicker` (.pickerStyle(.navigationLink)) for pickers in a Form/List — pushes a real list whose rows expose per-row identifiers. - `.accessibleInlinePicker` (.pickerStyle(.inline)) for non-Form pickers — lays the rows out in place (addressable by AXLabel). Apply them, with stable per-row accessibilityIdentifiers, to the funding-source, source-wallet, denomination, recipient-identity, and transition (token-register / builder / contract / document-type / token) pickers. The Create funding-source row id includes the account type so BIP44 #0 and Platform Payment #0 no longer collide. Pure SwiftUI/accessibility change in the example app — no FFI, no business logic. Verified on the iOS simulator with idb: option rows now appear in `describe-all` and tapping one updates the selection; drove a top-up end-to-end (ID-05, identity balance increased). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughIntroduces ChangesAccessible Picker Infrastructure and View Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Review complete (commit 2ff2000) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionInputView.swift`:
- Around line 124-135: The accessibility identifiers in the Picker blocks use
fixed hardcoded strings like "transition.selectPicker" and
"transition.select.none" that are not namespaced by the input key, causing
collisions when multiple TransitionInputView instances render together. Replace
the hardcoded accessibility identifier prefixes with a namespaced pattern that
includes the input key (such as input.name) in all Picker container
accessibleInlinePicker calls and in the tag accessibilityIdentifier values for
both the "Select..." option and the ForEach option items. Apply this namespacing
pattern consistently across all picker blocks in the file, including the ones
for token, contract, documentType, identity, and recipientIdentity selections,
so that each picker instance has unique, collision-free identifiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e6afa0b1-32d4-4f85-baa7-8563d04c2393
📒 Files selected for processing (5)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/Components/AccessiblePicker.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/Components/RecipientPickerView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/CreateIdentityView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TopUpIdentityView.swiftpackages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/TransitionInputView.swift
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "fae8cd0337158187cc5f2297cda29bc34cc6183122aea006669cf741307a2776"
)Xcode manual integration:
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Pure SwiftUI accessibility/automation change in SwiftExampleApp; no FFI, Rust, or consensus surface touched. One in-scope suggestion from Codex stands: the shared RecipientPickerView is always Form-hosted at its call sites, so the inline-picker choice forfeits the per-row identifiers that the new helpers were designed to expose. Claude's expanding-inline-picker note is an acknowledged trade-off in the PR description and is dropped.
🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/Components/RecipientPickerView.swift`:
- [SUGGESTION] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/Components/RecipientPickerView.swift:136: Use the navigationLink helper for the local-identity picker so per-row identifiers actually surface
Every current call site of `RecipientPickerView` embeds it in a `Form` `Section` (TransferCreditsView, TokenTransferActionView, TokenMintActionView, TokenFreezeActionView, TokenUnfreezeActionView, TokenDestroyFrozenFundsActionView). AccessiblePicker.swift documents that with `.accessibleInlinePicker` SwiftUI does not surface per-row `accessibilityIdentifier`s — every option inherits the container id — so the carefully-constructed `recipient.identity.<base58>` ids on lines 129 and 133 are effectively dead, and idb has to fall back to selecting by `displayName`. `PersistentIdentity.displayName` can collide (two identities with the same alias / DPNS name), making automation ambiguous. Since the picker is always Form-hosted, switching to `.accessibleFormPicker` preserves the stable per-row ids the helpers were designed to expose.
| } | ||
| } | ||
| .pickerStyle(.menu) | ||
| .accessibleInlinePicker("recipient.identityPicker") |
There was a problem hiding this comment.
🟡 Suggestion: Use the navigationLink helper for the local-identity picker so per-row identifiers actually surface
Every current call site of RecipientPickerView embeds it in a Form Section (TransferCreditsView, TokenTransferActionView, TokenMintActionView, TokenFreezeActionView, TokenUnfreezeActionView, TokenDestroyFrozenFundsActionView). AccessiblePicker.swift documents that with .accessibleInlinePicker SwiftUI does not surface per-row accessibilityIdentifiers — every option inherits the container id — so the carefully-constructed recipient.identity.<base58> ids on lines 129 and 133 are effectively dead, and idb has to fall back to selecting by displayName. PersistentIdentity.displayName can collide (two identities with the same alias / DPNS name), making automation ambiguous. Since the picker is always Form-hosted, switching to .accessibleFormPicker preserves the stable per-row ids the helpers were designed to expose.
| .accessibleInlinePicker("recipient.identityPicker") | |
| .accessibleFormPicker("recipient.identityPicker") |
source: ['codex']
There was a problem hiding this comment.
Resolved in 2ff2000 — Use the navigationLink helper for the local-identity picker so per-row identifiers actually surface no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
Address automated review feedback on the accessible-picker change: - TransitionInputView: namespace every picker's accessibility identifiers by the input's stable field key (`transition.\(input.name).…`). A single builder renders many `TransitionInputView` instances, so fixed strings like `transition.selectPicker` / `transition.select.none` collided across input fields and made automation queries ambiguous. (CodeRabbit.) - RecipientPickerView: use `.accessibleFormPicker` (navigationLink) instead of `.accessibleInlinePicker`. All six call sites embed this view in a Form `Section`, so the inline style needlessly forfeited the per-row `recipient.identity.<base58>` identifiers (inline rendering drops them). navigationLink pushes a real list and surfaces them. (thepastaclaw/Codex.) Verified on the iOS simulator with idb: the recipient picker now pushes a list whose rows report unique `recipient.identity.<base58>` ids (no duplicates). App builds clean for the simulator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Pure SwiftUI accessibility fix scoped to RecipientPickerView and TransitionInputView. Carried-forward prior finding is FIXED at head 2ff2000 — RecipientPickerView.swift:141 now uses .accessibleFormPicker("recipient.identityPicker"). The codex finding about TransitionDetailView identifies a pre-existing inaccessible picker in a file not modified by this PR; tracked as out-of-scope rather than blocking. No in-scope issues remain.
2 participants
Issue being fixed or feature implemented
Several SwiftExampleApp flows start with a SwiftUI
Pickerthat renders as a menu popover (theFormdefault, or an explicit.pickerStyle(.menu)/MenuPickerStyle()). When opened, idb's accessibility tree shows only an element labeled "Dismiss context menu" — the actual option rows ("Platform Payment #0 — 0.0231 DASH", wallets, identities, etc.) are not exposed, so UI automation can neither query nor tap a specific option (a blind coordinate tap just dismisses the menu).This blocked automated runs of the identity funding flows — ID-05 (top up), ID-06 (top up from Platform addresses), ID-08 (create from Platform addresses) — and the token-register / builder identity pickers. The features work fine manually; this is a testability/accessibility fix, not a product-bug fix.
QA tracking: #3897 (updated — the Picker-popover blocker is now marked RESOLVED).
What was done?
Added a shared helper,
AccessiblePicker.swift, with two presentation modifiers (no behaviour change — selection bindings /onChangeuntouched):.accessibleFormPicker(_:)→.pickerStyle(.navigationLink)for pickers inside aForm/List. Pushes a realList; per-rowaccessibilityIdentifiers surface as distinct, tappable rows..accessibleInlinePicker(_:)→.pickerStyle(.inline)for pickers not in a Form (where.navigationLinkrenders poorly). Rows are laid out in place; addressable byAXLabel.Applied them — with stable per-row identifiers — to:
TopUpIdentityView— funding-source picker (topup.fundingSource.*)CreateIdentityView— source-wallet, funding-source, denomination pickers (createIdentity.*)RecipientPickerView— local-identity picker (recipient.identity.*)TransitionInputView— identity / recipient-identity / contract / document-type / token / generic-select pickers (transition.*)The Create funding-source row id includes the account type (
…account.<accountType>.<accountIndex>) because that picker lists both Core and Platform-Payment accounts, so a bare#0would collide (BIP44 #0 vs Platform Payment #0).Pure SwiftUI / accessibility change in the example app — no FFI, no Rust, no business logic (per
packages/swift-sdk/CLAUDE.md).How Has This Been Tested?
Built
packages/swift-sdk/build_ios.sh --target sim(clean) and drove the booted iOS simulator (iPhone 17, fully-provisioned QA wallet) withidb:idb ui describe-alllistedtopup.fundingSource.account.0("Platform Payment #0 …") instead of "Dismiss context menu"; selected it (amount pre-filled, submit enabled), set a small amount, tapped Top Up Balance. Success banner showed the new balance, and SwiftData ground truth confirmed the identity balance increased to match. No errors/panics in the app log.CreateIdentityViewsource-wallet picker exposed all rows; selecting a wallet revealed the funding-source picker, which exposed unique rows and propagated selection (verified the id-collision fix; drove to submit-ready, did not broadcast the create).Note:
.inlinepickers expose rows but SwiftUI does not surface their per-row ids (rows inherit the container id; address byAXLabel) — documented in the helper.Menu { Button }navigation menus (e.g. the Identities "+") were never affected — they already expose their children.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit