Sammajayi/connect a wallet fixes by sammajayi · Pull Request #135 · GoodDollar/GoodWidget

sammajayi · 2026-07-30T11:20:17Z

Description

Redesigned the connect-wallet widget UI following the design reference in #113 in reference to the main pr in #116, executed following the plan in #114 . Fixed crash on error/unsupported states, addressed overflow on small viewports, and updated component styling to match the design system.

PR about #116

How Has This Been Tested?

Ran pnpm test: demo tests/widgets/connect-a-wallet-widget - all 8 Playwright state tests pass
Manual testing in Storybook for visual verification
Verified build and lint pass

Checklist:

- 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

Copilot

Pull request overview

This PR refines the Connect-a-wallet widget by aligning its UI more closely with the design reference, improving responsiveness on small viewports, and hardening edge/error states. It also updates supporting UI primitives and Playwright state tests to match the new copy and layout.

Changes:

Updated Connect-a-wallet widget view/layout (header, info callout, responsive chain rows, revised copy) and improved small-viewport behavior.
Added link / unlink icons and tuned GlowCard shadow styling to better match the design system.
Hardened Playwright state tests with a more reliable “wait until rendered” helper and added mobile/desktop layout screenshots.

Reviewed changes

Copilot reviewed 8 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/widgets/connect-a-wallet-widget/states.spec.ts	Improves test stability via `waitForText` and adds mobile/desktop layout coverage.
packages/ui/src/components/Icon.tsx	Adds `link` / `unlink` SVG paths for the redesigned connect/disconnect buttons.
packages/ui/src/components/GlowCard.ts	Adjusts glow shadow intensity/radius to better fit updated visual style.
packages/connect-a-wallet-widget/src/components/shared.tsx	Allows chain row card wrapping and simplifies `ActionButton` sizing for responsiveness.
packages/connect-a-wallet-widget/src/components/PrimaryIdentityCard.tsx	Redesigns the primary identity card and adds copy-to-clipboard affordance.
packages/connect-a-wallet-widget/src/components/ConnectAWalletWidgetView.tsx	Restructures the widget layout (header/callout/footer) and changes form/ready-state rendering.
packages/connect-a-wallet-widget/src/components/ChainLinkRow.tsx	Redesigns per-chain rows with status indicator + icon buttons and wrapping layout.
packages/connect-a-wallet-widget/src/components/AddressLinkForm.tsx	Updates form copy/typography and input/button sizing.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

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.

pheobeayo · 2026-08-18T01:14:37Z

<Heading level={6} fontSize="$1">Connect or Disconnect Address</Heading>
This is the exact same string as the heading in AddressLinkForm.tsx, but it's showing at a different point in the flow (after the address has already been checked, next to "Change address"). Two headings with identical text at different steps could be confusing when navigating by heading (e.g. screen readers), and it reads oddly sitting above rows that are already linked/checked. Could we give this one distinct copy, e.g. "Linked address" or "Manage connections"?

sammajayi · 2026-08-18T01:45:54Z

@pheobeayo, thanks for your review, will check it out right away.

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.

sammajayi · 2026-08-18T01:57:57Z

@pheobeayo, I have addressed all the reviews. Kindly check it out

pheobeayo

Good — this round addresses three of the four items I flagged, but introduces one new visual bug in the process. Here's the follow-up review.

✅ Resolved cleanly:

ChainRowCard dead code — removed from shared.tsx.
Duplicate "Connect or Disconnect Address" heading — now reads "Linked address" in the ready-state section, distinct from the form heading.
Copy-confirmation timeout — now tracked in a timeoutRef and cleared on unmount via useEffect. Clean fix, hooks are still called unconditionally before the !walletAddress early return, so no Rules of Hooks issue.

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.

sammajayi · 2026-08-18T20:19:02Z

@pheobeayo I have fixed as requested

pheobeayo

Nice work — all the review findings are addressed now. Confirmed:

ChainRowCard dead code removed from shared.tsx
Heading copy disambiguated ("Linked address" vs. the form's "Connect or Disconnect Address")
Copy-confirmation timeout cleaned up on unmount in PrimaryIdentityCard
Disconnect button hover fixed — onHoverIn/onHoverOut now flip the icon/text to white so they stay readable against the hoverStyle red background, and it stays RN Web-compatible

Tested against the Playwright state stories and the state machine, empty/error states, and the "always show exactly one of Connect/Disconnect" invariant all hold up. changeSecondaryAddress is a solid addition too — good catch fixing the dead-end flow.

Approving 🚀

pheobeayo · 2026-08-19T08:54:43Z

@L03TJ3 Kindly check through and review!

sammajayi · 2026-08-29T21:02:55Z

Hi @L03TJ3, awaiting your review

sammajayi · 2026-09-16T21:47:23Z

Gm team @L03TJ3 @pheobeayo

L03TJ3 · 2026-10-05T11:10:49Z

When code-reviewer or maintainer leaves a comment or review you are expected to respond to the comment explaining the fix you commited and referencing the commit where the change happened (for future issues, current resolved items are okay for now)
follow up my comment I just wrote
if any changes happened in commits that affect UI you should also always make sure to run the test-suite again and update the included screenshots for your PR. the current ones seem stale comparing it to the later commits and changes

L03TJ3 · 2026-10-05T11:11:21Z

@sammajayi please follow up on the comments and request a review from me when done (not by comment, by using the github ui request review button)

…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.

sammajayi · 2026-10-05T13:22:36Z

@L03TJ3 Thanks for your response.
The previous fixes and screenshot were stale. So I re-ran the suite, fixed the stale spec assertion, and regenerated the screenshots. The Storybook tests and the connect-a-wallet Playwright spec pass.

sammajayi · 2026-10-06T15:43:08Z

@L03TJ3 I amsorry, there was an oversight.
I have fixed it accordingly

sammajayi force-pushed the sammajayi/connect-a-wallet-fixes branch from bbfde2f to ed45fce Compare July 30, 2026 11:24

sirpy requested a lite review from Copilot August 11, 2026 12:44

Copilot started reviewing on behalf of sirpy August 11, 2026 12:45 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

Comment thread packages/connect-a-wallet-widget/src/components/ConnectAWalletWidgetView.tsx

Comment thread packages/connect-a-wallet-widget/src/components/PrimaryIdentityCard.tsx Outdated

Comment thread packages/connect-a-wallet-widget/src/components/PrimaryIdentityCard.tsx Outdated

L03TJ3 requested review from a team and pheobeayo and removed request for a team August 17, 2026 11:52

pheobeayo suggested changes Aug 18, 2026

View reviewed changes

sammajayi force-pushed the sammajayi/connect-a-wallet-fixes branch from 919cb1f to fed0846 Compare August 18, 2026 01:58

pheobeayo suggested changes Aug 18, 2026

View reviewed changes

Comment thread packages/connect-a-wallet-widget/src/components/ChainLinkRow.tsx

pheobeayo approved these changes Aug 19, 2026

View reviewed changes

L03TJ3 linked an issue Sep 29, 2026 that may be closed by this pull request

[GoodBounty] Finalize Connect-a-wallet Widget PR #122

Open

12 tasks

sammajayi requested a review from L03TJ3 October 5, 2026 13:22

fix: clear pending copy timeout before scheduling a new one

4d77827

L03TJ3 approved these changes Oct 9, 2026

View reviewed changes

L03TJ3 merged commit ca32e32 into GoodDollar:plan/connect-a-wallet-widget Oct 9, 2026
1 check passed

Conversation

sammajayi commented Jul 30, 2026

Description

How Has This Been Tested?

Checklist:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pheobeayo Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sammajayi commented Aug 18, 2026

Uh oh!

sammajayi commented Aug 18, 2026

Uh oh!

pheobeayo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sammajayi commented Aug 18, 2026

Uh oh!

pheobeayo left a comment

Choose a reason for hiding this comment

Uh oh!

pheobeayo commented Aug 19, 2026

Uh oh!

sammajayi commented Aug 29, 2026

Uh oh!

sammajayi commented Sep 16, 2026

Uh oh!

L03TJ3 commented Oct 5, 2026

Uh oh!

L03TJ3 commented Oct 5, 2026

Uh oh!

sammajayi commented Oct 5, 2026

Uh oh!

sammajayi commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sammajayi commented Oct 6, 2026 •

edited

Loading