Repository navigation
fix(connect-a-wallet-widget): harden wallet flows and QA (#245) - #249
Merged
L03TJ3 merged 3 commits intoOct 20, 2026
Conversation
edehvictor
marked this pull request as ready for review
October 13, 2026 16:41
This was referenced Oct 13, 2026
Collaborator
Author
|
@L03TJ3 please review |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Deferred host connections are blocked, and stale transaction failures can dismiss another widget’s confirmation and leave it busy.
2 open findings
What changed in this PR
Hardens the existing Connect-a-wallet widget for #245, following #116, with independent chain reads, guarded wallet actions, and expanded QA.
Changes:
- Uses chain-specific HTTP reads while retaining wallet signing.
- Adds address-validation feedback and widget-wide busy guards.
- Adds mocked interaction coverage and documents manual QA limits and screenshot storage.
| File | Description |
|---|---|
| tests/widgets/connect-a-wallet-widget/walletFixture.ts | Adds wallet and HTTP mocks. |
| tests/widgets/connect-a-wallet-widget/QA.md | Records verification, evidence, and limitations. |
| tests/widgets/connect-a-wallet-widget/interactions.spec.ts | Tests wallet flows, failures, and responsive states. |
| packages/connect-a-wallet-widget/src/widgetRuntimeContract.ts | Exposes validation and busy state. |
| packages/connect-a-wallet-widget/src/components/ConnectAWalletWidgetView.tsx | Displays errors and disables competing actions. |
| packages/connect-a-wallet-widget/src/components/ChainLinkRow.tsx | Supports widget-wide action disabling. |
| packages/connect-a-wallet-widget/src/components/AddressLinkForm.tsx | Shows validation feedback and blocks busy edits. |
| packages/connect-a-wallet-widget/src/adapter.ts | Adds HTTP clients, operation guards, and receipt checks. |
| examples/storybook/src/stories/connect-a-wallet-widget/ConnectAWalletWidget.interactions.stories.tsx | Adds production-path interaction stories. |
| docs/qa-guide.md | Clarifies screenshot storage. |
| docs/demo-environment.md | Separates tracked evidence from temporary artifacts. |
| AGENTS.md | Clarifies screenshot tracking requirements. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Collaborator
Author
|
I have Implemented the review by copilot. You can inspect @L03TJ3 |
L03TJ3
approved these changes
Oct 19, 2026
Contributor
|
@edehvictor can you just respond to the remaining open questions, rest is approved and seems fine |
Collaborator
Author
L03TJ3
merged commit Oct 20, 2026
63997e5
into
GoodDollar:plan/connect-a-wallet-widget
1 of 3 checks passed
This was referenced Oct 20, 2026
L03TJ3
added a commit
that referenced
this pull request
Oct 30, 2026
* Add ConnectAWalletWidget package with Storybook coverage and Playwright specs Implements the ConnectAWalletWidget for linking a secondary wallet address to the connected host wallet's GoodID across Fuse, Celo, and XDC via @goodsdks/citizen-sdk's IdentitySDK. Adds QA runtime-fixture stories, a manual showcase story, MDX docs, and a Playwright states spec covering the disconnected, connecting, address-entry, checking, mixed per-chain row statuses, unsupported-network, and top-level error/retry states. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add GoodDollar header and primary-identity card to connect-a-wallet-widget Every widget in this repo renders its own GoodDollar/chain-badge header inside its own view (see StreamingWidgetView) rather than relying on the shared MiniAppShell for it — this widget was missing that, and the "Primary verified identity" card showing which host address is linking from, per the #113 design reference. Regenerated Playwright screenshots to reflect both changes. * Add ConnectAWalletWidget package with Storybook coverage and Playwright specs Implements the ConnectAWalletWidget for linking a secondary wallet address to the connected host wallet's GoodID across Fuse, Celo, and XDC via @goodsdks/citizen-sdk's IdentitySDK. Adds QA runtime-fixture stories, a manual showcase story, MDX docs, and a Playwright states spec covering the disconnected, connecting, address-entry, checking, mixed per-chain row statuses, unsupported-network, and top-level error/retry states. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Add GoodDollar header and primary-identity card to connect-a-wallet-widget Every widget in this repo renders its own GoodDollar/chain-badge header inside its own view (see StreamingWidgetView) rather than relying on the shared MiniAppShell for it — this widget was missing that, and the "Primary verified identity" card showing which host address is linking from, per the #113 design reference. Regenerated Playwright screenshots to reflect both changes. * Sammajayi/connect a wallet fixes (#135) * fix: UI polish and lint fixes for connect-a-wallet-widget - Fix missing Alert import crash on error/unsupported states - Remove unused address prop from ChainLinkRow (lint fix) - Restructure PrimaryIdentityCard layout (shield/text/copy) - Reduce GlowCard shadow opacity and radius - Update unlink SVG path to match lucide icon - Add flexWrap to ChainLinkRow for 320px viewport support - Reduce input/button sizes for small viewports - Center address in verified identity box with wide gap - Adjust heading spacing and checkmark position - Update Playwright test screenshots * fix: address Copilot review findings on connect-a-wallet-widget Adds a changeSecondaryAddress adapter action and "Change address" button so users can check a different address without reloading, replaces a hardcoded RGBA background with the $infoMuted token, and swaps the copy-address control for an accessible, focusable Button. * fix: address pheobeayo review findings on connect-a-wallet-widget Remove unused ChainRowCard export, disambiguate the duplicate "Connect or Disconnect Address" heading, swap hover-tracked useState for hoverStyle on the disconnect button (RN Web/touch compatible), and clear the copy-feedback timeout on unmount. * fix: restore hover text contrast on disconnect button hoverStyle only flips the background to red; icon/label were still hardcoded to error color, making them invisible on hover. Reintroduce a small hover flag via onHoverIn/onHoverOut to swap text/icon to white on hover, per pheobeayo's review. * test: refresh connect-a-wallet-widget screenshots and fix stale spec assertion The ready-state heading changed to 'Linked address' in earlier review-fix commits; update the unsupported-network assertion and regenerate caw-05, 06, 08 and 09 so the committed evidence matches the current UI. * fix: clear pending copy timeout before scheduling a new one * fix(connect-a-wallet-widget): harden wallet flows and QA (#245) (#249) * fix(connect-a-wallet-widget): harden wallet flows and QA (#245) * fix(connect-a-wallet-widget): isolate stale dialogs and defer host connection * docs(connect-a-wallet-widget): keep screenshot evidence test-generated --------- Co-authored-by: BountyCoder <bountycoder@gooddollar.org> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: goodbounties-nanoclaw-agent[bot] <307944451+goodbounties-nanoclaw-agent[bot]@users.noreply.github.com> Co-authored-by: Samuel Ajayi <Samuelajayi554@gmail.com> Co-authored-by: LewisB <laurence@gooddollar.org> Co-authored-by: Victor Edeh <edehvictor715@gmail.com>
This branch is waiting to be deployed
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.
Closes #245. Follow-up to #116, targeting
plan/connect-a-wallet-widget.Changes
Verification
pnpm test:demo tests/widgets/connect-a-wallet-widget --workers=1 --retries=0: 31/31 passed, exit 0, on October 15, including the review regression cases.pnpm buildandpnpm lintremain blocked by the existing UISelect.tsx:7Role type error, reproduced from unchanged base commit3ce69ab6.Evidence And Limits
Details: QA report.
Evidence: screenshots, 19 test-generated screenshots covering desktop/mobile states. Manual captures are not tracked; live-wallet observations and the public unlink receipt remain documented in the QA report.
42220, which shared core parsed as hexadecimal. The chain-ID parsing correction is not included; it is separate from the connection-availability change above. These results do not establish unmodified WalletConnect compatibility.Commits: initial implementation
074b5479, review fixesf42bc703, and screenshot cleanup7c36eb25.The October 19 cleanup changes only QA documentation and removes the manual PNG. Whitespace/reference checks passed, and all 19 remaining PNG names match test-script outputs. Runtime tests were not rerun for this documentation-only cleanup.