Feat/goodreserve widget

[Ryjen1]

Overview

This PR finalizes the goodreserve-widget implementation, resolves styling conflicts, aligns the layout/colors to the Figma specification, and integrates the real GoodReserve SDK.

Fixes #15
Closes #44

Changes Made

• SDK Isolation: Isolated the @goodsdks/good-reserve library by creating a clean typed seam (sdk.ts), keeping direct SDK/blockchain interactions decoupled from the adapter state machine.
• State Machine Fixes: Resolved bugs around idle timeouts, quote expiration, error recovery, and network switching.
• Display & Calculations: Corrected swap calculation discrepancies, success screen values, price label text, and transaction explorer URL construction.
• Styling Migration: Completed migration to the Tamagui theme contract, resolving sub-theme naming collisions between the reserve widget and core components.
• Figma Alignment: Adjusted padding, margins, colors, and layout hierarchies to align with the Figma specs and the Stitch prototype.
• Amount Input Web Fix: Fixed focus/input issue on web browsers making the swap amount field editable.
• Test Optimization: Increased Storybook test-runner timeouts to mitigate setup-phase cold-start timeouts in CI.

Verification Checklist

• Monorepo install, build, and lint runs cleanly (pnpm build & pnpm lint).
• Storybook play/interaction tests: 33/33 tests pass.
• Playwright integration tests: 17/17 tests pass.

Evidence

• Verified all 16 widget states (from grw-01 to grw-16) in the Playwright snapshot suite under tests/widgets/goodreserve-widget/test-results/.
• Updated Storybook reference screenshots under examples/storybook/src/stories/goodreserve-widget/screenshots/.

Remaining Risks

• The @goodsdks/good-reserve dependency is imported dynamically as it hasn't been published to npm yet. Playwright runs use a mocked provider; full end-to-end network validation should be performed in staging once the SDK is released.

[pheobeayo]

Reviewed against issue #15's acceptance criteria. Pulled the branch, typechecked the widget source under strict against viem 2.52, read through the adapter, SDK seam, error mapper, and view, and ran Storybook locally to check the UI against the Figma states. This is a careful, well-structured PR, happy to approve after a few clarifications.

Procedural: this PR targets copilot/build-goodreserve-widget, not the default branch, confirming that's an intentional stacked base. "Fixes #15 / Closes #44" will only resolve once the chain lands on default.

What's solid

• BigInt discipline throughout: minReturn is derived once in base units, and the exact minReturnRaw is carried into executeSwap, so the floor shown to the user is the floor signed on-chain. The insufficient-balance gate compares in BigInt too — no last-decimal float drift.
• Stale-quote rejection via quoteExpiresAt, the live eth_chainId re-check before signing, the double-submit guard, and the best-effort post-swap balance refresh are all the right calls.
• mapReserveError sanitizing revert reasons so raw viem output (RPC URLs / addresses) never reaches the UI.
• Complete state coverage — all 16 issue states have Playwright specs, including a live-adapter buy flow on the injected fake SDK.
• UI holds up in Storybook: the drawer-based states (confirm-dialog, slippage-selection) and the swap-success screen match the Figma layout, and the live Interactive / LiveFakeSdk stories exercise the drawer transitions and 390px fill behavior correctly.

Before merge — confirm or file follow-ups (none blocking)

• SDK seam fidelity. Everything downstream is type-checked against sdk.ts, but the seam is a hand-mirrored copy of an unpublished PR #35 surface, so the type checker can't catch a divergence in the seam itself. Could you link the exact PR #35 commit these signatures were copied from? (See inline note on the asymmetric arg orders.)
  optionalDependencies: "@goodsdks/good-reserve": "*" your own publish checklist in sdk.ts says to pin a version. Fine to defer, but please file the follow-up so * doesn't ship silently.

• New states need an extra UI look. quote-ready-xdc and sdk-initializing are added with no prior baseline; nothing has vetted them before this PR. The XDC (chain 50) variant is worth a careful pass, since the network pill, token labels, and $reserve* theming differ from the Celo path and that's where a token mistake would surface.

• Committed tests/**/test-results/grw-.png. Intended as baselines, or run artefacts that should be git-ignored? If baselines, they'll churn on every UI tweak. (The curated screenshots/.png set is the right reference set; the test-results/ copies look like Playwright run output.)

Nice work on the BigInt correctness and the deterministic fake-SDK harness, that's the hard part done right.

[Ryjen1]

Hi @pheobeayo
Thanks for the careful review. I went through your three non-blocking points and updated things accordingly.

Procedural: yes, this work is intentionally stacked on copilot/build-goodreserve-widget as part of issue #15 / PR #39, not on default.

SDK seam fidelity: I added the source-of-truth note at the top of sdk.ts pointing to GoodSDKs PR #35, merge commit 53fa2f56947e10e29864c0ab4f22733ca54d3b3a, and good-reserve head fcd7b278558db9624f2b67a7cbe22c09f662bd32. I also cross-referenced apps/demo-reserve-swap/src/components/ReserveSwap.tsx, mirrored the demo's /10000 scaling for reserveRatio / exitContribution, kept the optionalDependencies: "*" checklist note until npm publishing is real, and documented the buy(...) / sell(...) arg-order asymmetry in the inline GoodReserveSDKLike comment.

XDC path: I verified the demo's XDC flow. The env, symbol, decimals fallback, and network label were already correct; the only mismatch was the explorer URL, which I updated to explorer.xdc.org/tx/ and annotated in ReserveSwapView.tsx.

tests/**/test-results/.png: I untracked the 16 grw-.png artifacts, added /test-results/ to .gitignore to cover nested Playwright output, and updated AGENTS.md to clarify that the canonical evidence is the curated examples/storybook/src/stories//screenshots/ set while tests//test-results/ is transient.

Extra fix: I restored the missing QuoteReadyXdc story export, regenerated the XDC screenshot, and confirmed all 17 widget Playwright tests now pass.

Verification:

• pnpm build (8/8)
• pnpm lint (13/13)
• pnpm test:demo tests/widgets/goodreserve-widget (17/17)
• pnpm test:storybook (35/35)

[pheobeayo]

Welldone @Ryjen1

Re-reviewed at a8a4d9f. The two new commits address all four follow-ups from my last pass, pulled the branch and diffed against the previously reviewed HEAD. Approving.
Resolved

SDK seam provenance (item 1) — fully addressed. sdk.ts now cites the GoodSDKs PR #35 merge commit (53fa2f5), the good-reserve head SHA (fcd7b27), the exact mirrored file list, and a demo cross-reference for the PPM /10_000 pool-field scaling. The adapter and view now point back to sdk.ts for that provenance too. This is exactly what makes the seam auditable, thank you.
test-results/ PNGs (— all 16 grw-.png artifacts deleted, and .gitignore now has **/test-results/ so they won't re-creep. Curated screenshots/.png correctly retained as the reference set.
XDC story (item from UI pass) — QuoteReadyXdc export restored in the stories file (a8a4d9f), wired to xdcQuoteReady. The new-state coverage gap is closed.

One new thing to confirm (non-blocking)
The explorer URL changed from xdcscan.com/tx/… to explorer.xdc.org/tx/…, rationale being "match the SDK author's demo." Heads up that this moved it toward the older explorer: the current canonical XDC explorers are the XDCScan family (xdcscan.com / xdcscan.io, Etherscan-style), while explorer.xdc.org / observer.xdc.org are the older BlocksScan-era surface. The link still resolves, so this isn't a blocker, but the original xdcscan.com was arguably the more current choice. Worth a quick confirm that you want to track the demo here rather than the canonical explorer, if the demo itself is stale on this, matching it locks in the staleness.
Everything else from the prior review still stands as solid. Nice, tight turnaround on the follow-ups.

[Ryjen1]

Hi @pheobeayo,I agree, xdcscan.com is a stronger choice. The original switch to explorer.xdc.org was following the SDK demo, but as you flagged, the demo is stale on this point. xdcscan.com is the Etherscan-style explorer registered in viem's own XDC chain definition (chainId 50), which is a more reliable source of truth. I haave reverted it in my latest change

[pheobeayo]

Thanks for the work here. Before this can move toward merge, there are a few structural and process items to address , flagged inline. Grouping them:
Must fix

• Use the real SDK, not a mock. @goodsdks/good-reserve is now published remove the dynamic await import() seam entirely (we don't use dynamic imports in this codebase) and switch to a static import against the published package.
• Revert packages/ui/presets.ts to master. Don't modify shared design-system tokens for widget-local styling; we'll align on the right pattern for localized styling first.
• Fix test-results hygiene. Playwright screenshots should output to the per-widget folder under root /test, matching master and the other open PRs, please follow the existing structure rather than introducing a new layout.
• Decompose ReserveSwapView.tsx into per-state subcomponents instead of one large component with nested conditionals.
• Restructure errors.ts away from the long if-chain, and let's decide whether it should be shared with citizen-claim-widget rather than duplicated.

Please ask questions. Several of these trace back to assumptions made instead of questions asked. The clearest example: the SDK package wasn't published, and instead of asking "this isn't published, what's expected?", the PR built a mock and lazy-import workaround around it. Asking would have resolved it immediately (the package just needed publishing). When an approach is unclear or something's blocking, please raise it in the issue/PR before writing code , it saves you from having to remove or refactor large sections later.

[pheobeayo]

You've already documented the correct end-state in the header checklist (lines 28–33): pin the version and replace these mirrored types with import type from the package. The package is published now, so please action that checklist in this PR rather than leaving it as a future note, remove SDK_SPECIFIER, loadGoodReserveSdkConstructor, the try/catch, and the optionalDependency "*", and switch to a static import. We don't use dynamic await import() anywhere in this codebase.
The process point still stands: the package being unpublished was a blocker worth raising as a question up front, rather than building a lazy-import workaround around, asking would have surfaced that it just needed publishing.

[pheobeayo]

Blocking — please revert this file to master. This is the shared design-system preset that every widget consumes, and this PR deletes the entire governance* token set and rewrites the light theme branch to suit this one widget. This breaks the contract that other widgets rely on. Localized styling for a new widget must not be achieved by mutating the shared preset.
This needs a discussion before any change here, not a unilateral edit. The question to raise: how do we expect localised/widget-scoped styling to be assigned within our design system a widget-scoped sub-theme, token overrides at the widget boundary, etc.? Let's align on that pattern first, then apply it. For now, restore presets.ts as-is on master.

[L03TJ3]

@Ryjen1 not handled.
also showing there was a governance styling already in the branch.

[L03TJ3]

@pheobeayo FYI: https://github.com/GoodDollar/GoodWidget/blob/main/docs/widget-author-instructions.md

this is on a high-level what is expected.
next week I will update the instructions as the best way that it has been implemented so far can be seen here:

https://github.com/edehvictor/GoodWidget/blob/651b6452866eccbf34216e86bbfe8babb1587332/packages/governance-widget/src/config.ts
Handling the config before passing down into GoodWidgetProvider
https://github.com/edehvictor/GoodWidget/blob/651b6452866eccbf34216e86bbfe8babb1587332/packages/governance-

[pheobeayo]

Red flag on structure. This is one ~690-line component handling the entire state machine via nested inline conditionals (if (state.status === …) branches and inline ternaries inside the render). That's hard to read, hard to review, and hard to maintain. Please decompose it: one focused subcomponent per state/screen (initializing, success, the main swap view, the confirm and slippage drawers), with the top-level component just routing on status. Take a look at how the existing widgets structure their views and follow that house style.

[L03TJ3]

@Ryjen1 not followed up on this comment

[pheobeayo]

Resolved via config-based theming. The FIGMA object is gone (0 references); colors now resolve through goodReserveWidgetConfig.themes.light/dark with widget-scoped $reserve* token extensions only where the shared preset has no equivalent, and GoodReserveWidget.tsx merges config < host < themeOverrides into GoodWidgetProvider. Integrators can override any of it via themeOverrides={{ themes: { light: { reserveCard: '…' } } }}, matching the governance-widget pattern you linked. No raw hex remains in the view. Verified locally.

[pheobeayo]

Also
Fixed. The explorer anchor content is now wrapped in (with a comment noting bare strings break on React Native), and FIGMA.primary is now $primary. The TokenBadge text that was FIGMA.text resolves through the component sub-theme now. Verified locally.

[L03TJ3]

Hey, besides any follow up review that @pheobeayo will give.
one note from me. @Ryjen1

The way to work with the design preset is that for the most part we re-use styling as defined by the preset.
We have light/dark theme (light is flagged as governance).
and it works that if you apply something like '$background' it will either use light or dark value depending on the GoodWidgetProvider default theme.

We don't want to see any hardcoded local styling within components.

the highest level where we can see hardcoded 'single-use' values is dark/light theme component styling (light_Button/dark_Button).
you use the 'createComponent' for this. random example:

GoodWidget/packages/claim-widget-theme-demo/src/ClaimWidget.tsx

Line 31 in e6c8f93

const StreakCard = createComponent(Card, {

that results in being able to define light_StreakCard/dark_StreakCard that can be placed in the preset as you see with other examples.

We only update light/dark if the usage will likely translate to other components.
We only update the higher-level token set if global changes are needed.
any changes to the token set translates to all widgets that use the system, so most of the time should be left untouched.

Is the design system values finalized? not 100%, any suggestions you might have or things that just don't look good can be raised and we can think of what values make better sense and/or look better.

[L03TJ3]

Only screenshots that should be committed are the ones that are output through the playwright tests and put in the widget specific folders in /tests/

no screenshots should be in storybook.

[pheobeayo]

Resolved. No screenshots remain under examples/storybook/.../screenshots/, committed screenshots are Playwright output under tests/widgets/goodreserve-widget/ only. Verified locally.

[L03TJ3]

default is 18, only on fuse its 2 but we have not deployed the reserve on fuse

[L03TJ3]

@Ryjen1 not followed up on this comment

[L03TJ3]

I shared these instructions: #53 (comment)

Two things that stand out:

1. the colors are maybe not the exact hex values as on figma, but if you compare them you will see how close they are together.
   its perfectly fine to use them from the default-preset. you can always comment if needed: Hey used this because it was a close match. or you can request to confirm what is expected.

In the end, we want consistency across widgets, so a green color that is slightly different but would align with success green color used in other widgets: perfectly fine.

2. The comment points out 'There is no light design'.
   I did push an update 2 weeks ago e6c8f93

and got merged in the base-pull-request for good-reserve, also two weeks ago. my bad for not pointing this out to you earlier, but that update includes light governance theming

[L03TJ3]

And a third thing.
We try and ship our widgets with a default design system.
most aspects in terms of UI should be overridable by an integrator of the widget.

The current 'FIGMA' object setup does not expose these variables to be overwritten by integrators.

1. It has to be passed down through config in the GoodWidgetProvider wrapper of the good-reserve. I think the best example so far is this:
   (the theme/styling definitions)
   https://github.com/edehvictor/GoodWidget/blob/651b6452866eccbf34216e86bbfe8babb1587332/packages/governance-widget/src/config.ts

Handling the config before passing down into GoodWidgetProvider
https://github.com/edehvictor/GoodWidget/blob/651b6452866eccbf34216e86bbfe8babb1587332/packages/governance-widget/src/GovernanceWidgetProvider.tsx

And some comment context:
#54 (comment)
#54 (comment)

2. An example. there is here FIGMA.text. this is used 18 times across SwapReserveView.
   only places where it currently can be overridden is where its used within createComponent, but only per component.

const AmountCard = createComponent(Card, {
...
  color: FIGMA.text,
...
})


// Can then do as integrator

light_AmountCard: {
 color: <custom color>
}

This will cause drift. if FIGMA.text is used across this view and only per component and step it can be upgraded, but not everywhere.

So the flow to apply is what you see as I shared examples at point 1.

goodReserveWidgetConfig = {
 themes: {
     light: {
     	textColor: <your value>
     },
     dark: {
	textColor: <your value, make sure the color make sense for light + dark, or use different value>
     }
}

Above makes it possible that if someone integrates it and wants to update textColor across the widget, it only has to do:

    <GoodReserveWidget
      themeOverrides={{
        themes: {
          light: {
	    textColor: <some value>
          },
        },
      }}

1. named component styling light_/dark_componentName is only used when light/dark use different colors
2. as most as possible (slight drifts are fine) sizing/coloring should be used as defined by the default design preset.
   you might need to add exports in your PR to get the original token preset and mergeOverrrideMaps from ui-package.

[pheobeayo]

Acknowledged your note that light governance theming was merged into the (e6c8f93). The widget is currently dark-only by design (documented in widgetRuntimeContract.ts, defaultTheme fixed to 'dark'), and the config defines identical light_/dark_ sub-theme pairs. @L03TJ3 — given the base now includes light governance theming, do you want the reserve widget to provide distinct light values, or is dark-only still acceptable for this widget? Leaving open for your call.

[L03TJ3]

@Ryjen1
Anchor is a wrapper. inside it hosts {children}
Text should always be wrapped with the <Text> component.

on web this does not cause issues directly. but tamagui is a react-native first component library (we map it to react-native-web, for web).

Having text not Wrapped with <Text> breaks on react-native environments.

Side note: FIGMA.primary is not a color. I assume its supposed to be '$primary'?

cc: @pheobeayo

[L03TJ3]

Hardcoded values only when they differ from the preset.
font-sizing / weights, all are passed down through the default preset.

[L03TJ3]

A single hook this big should raise concern and should look into what can be taken out as 'single responsibility'.

While it can happen during building you just test things out of it all works and ties together. before pushing for review these type of flows/components/hooks should be cleaned up.

I asked AI to breakdown the responsibilities of this single hook 'useGoodReserveAdatper`

  High-level, it owns these responsibilities:

  1. Wallet/runtime integration
     It reads provider, address, chain, and connection state from useWallet(), decides whether the widget is in no_provider or
     unsupported_chain, and reacts to chain/provider changes.

  2. SDK/bootstrap lifecycle
     It constructs GoodReserveSDK, creates viem clients, chooses the reserve environment, reads reserve stats, caches decimals, and
     reinitializes when chain context changes.

  3. Local widget state machine
     It owns the main adapter state:
      - status
      - direction
      - amount
      - balances
      - quote
      - warning/error
      - tx hash
      - success payload
      - quote freshness metadata

  4. Balance loading and normalization
     It fetches stable/G$ balances, stores raw balance context in refs, and remaps balances depending on buy/sell direction.

  5. Quote pipeline
     It debounces input, validates parseability, checks insufficient balance, calls SDK quote methods, derives display values like
     minimum received and price, and moves the state through amount_editing -> quote_loading -> quote_ready / quote_error.

  6. Transaction execution flow
     It validates stale quote and chain before submit, executes buy/sell, tracks hash, waits for resolution, sets success/error state,
     and refreshes balances afterward.

  7. UI overlay coordination
     It manages slippage/confirm-dialog transitions and restores the previous underlying state when those overlays close.

  8. User action adapter
     It exposes the actions object that the UI uses:
      - connect
      - switch chain
      - set direction
      - set amount
      - set max
      - open/close overlays
      - execute swap
      - refresh

  9. Error translation and guardrails
     It maps low-level SDK/provider failures into user-facing errors and enforces behavioral safety checks like:
      - no double submit
      - no stale quote submit
      - no unsupported-chain submit

  That is why it feels massive: it is simultaneously:

  - a runtime adapter
  - a state machine
  - a quote controller
  - a transaction controller
  - a UI coordinator

  If you want a cleaner mental model for the contributor, I’d describe it as:

  - runtime/bootstrap
  - quote flow
  - swap flow
  - UI state orchestration

  Those are the four natural seams. If this were to be refactored later, those are probably the first boundaries to split on.

[L03TJ3]

thats too much, way to much.
the final hook should be just be an accumulation of external helpers/handlers.

[pheobeayo]

Addressed. The adapter is now 196 lines and reads as thin glue: it composes four seams — useReserveBootstrap (runtime/SDK/chain), useReserveQuote (debounced quote pipeline), useReserveSwap (execution), and a small actions object of UI-action adapters that delegate to applyStatePatch. That maps to the four boundaries you outlined. Heaviest extracted file is useReserveBootstrap, but it's cohesive (refs factory, ref-sync, chain map, bootstrap — all runtime concerns), not a hidden monolith. Verified locally.

[L03TJ3]

Is this a wrong merge? or what is the reason for this change?
I don't see this version of Drawer.tsx in either the parent-pull-request or in the main branch.

Any changes to components from packages-ui should be treated very carefully and always better to ask feedback before applying changes.
Drawer is a recurrent component in different widgets, as far as I know and have seen working.
so I find it unlikely these changes are required.

[Ryjen1]

Hello @L03TJ3, regarding this, let me explain what I did here.

Drawer.tsx is not a new file, it already exists in main and in the target branch (copilot/build-goodreserve-widget). The version you're seeing in this PR is a modified version that I had to update to fix a mounting issue

As part of the feedback to modularize the widget, I extracted the Slippage and Confirm dialogs into their own drawer components. After making that change, the Drawer portal stopped mounting

After debugging it, I found that the issue was with the createComponent HOC wrapping Sheet.Overlay, Sheet.Frame, and Sheet.Handle. Tamagui's Sheet clones these components internally and attaches refs to them, but the HOC prevents those refs from being forwarded correctly, which causes the drawer not to mount properly

To fix it, I removed the HOC wrappers for those Sheet subcomponents, used the style object constants directly, and manually called registerComponent on the main component. That's why Drawer.tsx appears in the diff

Please let me know what you think.

[pheobeayo]

@L03TJ3 Confirmed still modified vs the base branch (+33/−18) — verified locally. The contributor documented the reason inline: wrapping Sheet.Overlay/Frame/Handle with createComponent throws "Function components cannot be given refs" because Sheet clones its compound parts and forwards refs, which createComponent doesn't pass through. Their fix uses the Sheet parts directly with style-object constants, plus a manual registerComponent call to keep them in the override chain.
Since Drawer is shared across widgets, this is your call, not something I should sign off on. @L03TJ3 — is the createComponent → registerComponent + style-object approach acceptable for the shared Drawer, or should the ref-forwarding be fixed in createComponent (or handled widget-side) instead? @Ryjen1 please hold this file until that's confirmed.

[L03TJ3]

Yes, should be fixed in createComponent and supporting forwarding refs

export function createComponent(base: TamaguiComponent, options: Record<string, any> & { name: string; extends?: string }): any {
  const { extends: extendsName, ...rest } = options

  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  const styledComponent = styled(base as any, rest) as any
  const component =
    typeof styledComponent.styleable === 'function'
      ? styledComponent.styleable((props: any, ref: React.Ref<unknown>) =>
          React.createElement(styledComponent, { ...props, ref }),
        )
      : React.forwardRef<unknown, any>((props, ref) =>
          React.createElement(styledComponent, { ...props, ref }),
        )

  component.displayName = rest.name

  const variantKeys = rest.variants ? Object.keys(rest.variants) : []

  registerComponent({
    name: rest.name,
    extends: extendsName,
    themeKeys: [...THEME_KEY_NAMES] as string[],
    variants: variantKeys,
  })

  return component
}

Drawer can be reverted to its original version

[L03TJ3]

@Ryjen1 not handled.
also showing there was a governance styling already in the branch.

[L03TJ3]

@pheobeayo FYI: https://github.com/GoodDollar/GoodWidget/blob/main/docs/widget-author-instructions.md

this is on a high-level what is expected.
next week I will update the instructions as the best way that it has been implemented so far can be seen here:

https://github.com/edehvictor/GoodWidget/blob/651b6452866eccbf34216e86bbfe8babb1587332/packages/governance-widget/src/config.ts
Handling the config before passing down into GoodWidgetProvider
https://github.com/edehvictor/GoodWidget/blob/651b6452866eccbf34216e86bbfe8babb1587332/packages/governance-

[L03TJ3]

@Ryjen1
Usage of AI tooling is not discouraged. AI is fast and can be used as good tool to produce results faster.
However, AI writes very verbose code and (my assumption) for efficiency just dumps everything in a single file without clearer instructions.
As a first big task pass, thats fine.

The reason why we have the human contributor/human-in-the-loop flow with the GoodBounties is to produce: maintable/readable code, that should result in efficient production ready widgets.

After things are tested and proven to work you should go back with the AI and clean up in a step-by-step flow (not all instructions/requests at once):

• separation of concerns, KISS, DRY etc. (but prevent ending up with a large set of single line helper methods, AI is very good in using that to 'clean up'
• include comments and request it to do so (more then what you would likely put yourself
• have it sometimes explain the implementation and code back, it can raise bugs hidden in the verbose code.

[pheobeayo]

#53 (comment)
#53 (comment)

@Ryjen1 These issues were not tackled, Please resolve them

[Ryjen1]

Not done yet, please I would request review when I finalise

[Ryjen1]

Hi @pheobeayo @L03TJ3,

I've pushed another round of changes

• Removed the mock types and dynamic imports. The widget now uses static imports from @goodsdks/good-reserve
• Reverted packages/ui/src/presets.ts back to match main
• Removed the hardcoded FIGMA color constants. Theme values are now mapped through design system tokens in packages/goodreserve-widget/src/config.ts and applied via mergeThemeOverrides, following the same pattern as the governance widget
• Split the Slippage and Confirm drawers into their own components
• Broke useGoodReserveAdapter into smaller hooks: useReserveBootstrap, useReserveQuote, and useReserveSwap
• Fixed the transaction flow so the success state only shows after the transaction receipt is confirmed. The transaction hash is now exposed through onHash, so users can open the explorer while the transaction is still pending
• Gated the live Playwright integration test behind GOODRESERVE_LIVE_TEST=1 to keep CI passing
• Removed the Storybook screenshots from examples/ and kept only the Playwright visual snapshots under tests/
• Wrapped all Anchor children with Text to avoid rendering issues in React Native

[pheobeayo]

@L03TJ3 Confirmed still modified vs the base branch (+33/−18) — verified locally. The contributor documented the reason inline: wrapping Sheet.Overlay/Frame/Handle with createComponent throws "Function components cannot be given refs" because Sheet clones its compound parts and forwards refs, which createComponent doesn't pass through. Their fix uses the Sheet parts directly with style-object constants, plus a manual registerComponent call to keep them in the override chain.
Since Drawer is shared across widgets, this is your call, not something I should sign off on. @L03TJ3 — is the createComponent → registerComponent + style-object approach acceptable for the shared Drawer, or should the ref-forwarding be fixed in createComponent (or handled widget-side) instead? @Ryjen1 please hold this file until that's confirmed.

[pheobeayo]

Addressed. The adapter is now 196 lines and reads as thin glue: it composes four seams — useReserveBootstrap (runtime/SDK/chain), useReserveQuote (debounced quote pipeline), useReserveSwap (execution), and a small actions object of UI-action adapters that delegate to applyStatePatch. That maps to the four boundaries you outlined. Heaviest extracted file is useReserveBootstrap, but it's cohesive (refs factory, ref-sync, chain map, bootstrap — all runtime concerns), not a hidden monolith. Verified locally.

[pheobeayo]

Resolved via config-based theming. The FIGMA object is gone (0 references); colors now resolve through goodReserveWidgetConfig.themes.light/dark with widget-scoped $reserve* token extensions only where the shared preset has no equivalent, and GoodReserveWidget.tsx merges config < host < themeOverrides into GoodWidgetProvider. Integrators can override any of it via themeOverrides={{ themes: { light: { reserveCard: '…' } } }}, matching the governance-widget pattern you linked. No raw hex remains in the view. Verified locally.

[pheobeayo]

Also
Fixed. The explorer anchor content is now wrapped in (with a comment noting bare strings break on React Native), and FIGMA.primary is now $primary. The TokenBadge text that was FIGMA.text resolves through the component sub-theme now. Verified locally.

[pheobeayo]

Resolved. No screenshots remain under examples/storybook/.../screenshots/, committed screenshots are Playwright output under tests/widgets/goodreserve-widget/ only. Verified locally.

[pheobeayo]

Acknowledged your note that light governance theming was merged into the (e6c8f93). The widget is currently dark-only by design (documented in widgetRuntimeContract.ts, defaultTheme fixed to 'dark'), and the config defines identical light_/dark_ sub-theme pairs. @L03TJ3 — given the base now includes light governance theming, do you want the reserve widget to provide distinct light values, or is dark-only still acceptable for this widget? Leaving open for your call.

[L03TJ3]

I will share my AI review with you and hope you can follow up on it and humanly verify that the code is maintable.
I was going down a rabbit hole of things to change, but the result is: AI overengineered.

I'll leave the dark/light theme support out of the review for now so any comments toward it, just skip.
okay to finalize with just dark-theme. I will do a pass to match it for light theming and will update our supporting docs to better explain how the system is intended to be used.

The Review

GoodReserve Deletion Review

Scope: packages/goodreserve-widget/src hooks and components.

Review lens: remove defensive/AI-shaped code that does not appear to carry current behavior. This is not a bug review and does not assume hypothetical edge cases are product requirements.

Findings

1. Remove sanitizeAmount and amount.ts

Files:

• packages/goodreserve-widget/src/amount.ts
• packages/goodreserve-widget/src/ReserveSwapView.tsx
• packages/goodreserve-widget/src/useGoodReserveAdapter.ts

This helper is not justified by the current flow:

• user amount input is already constrained to regular decimal input in QA
• setMaxAmount uses state.tokenInBalance, which comes from contract/SDK balances formatted with formatUnits
• quote/swap execution uses parseUnits as the real parser

Reasonable removal:

• delete packages/goodreserve-widget/src/amount.ts
• remove the import from ReserveSwapView.tsx
• change actions.setInputAmount(sanitizeAmount(event.target.value)) to actions.setInputAmount(event.target.value)
• remove the import from useGoodReserveAdapter.ts
• change setInputAmount to store the passed value directly
• change setMaxAmount to use state.tokenInBalance directly

This removes a helper and comments that imply a problem the actual data path does not show.

2. Collapse most of the ref mirror layer

Files:

• packages/goodreserve-widget/src/useReserveBootstrap.ts
• packages/goodreserve-widget/src/useGoodReserveAdapter.ts
• packages/goodreserve-widget/src/useReserveQuote.ts

ReserveRefs currently mirrors a large amount of React state into refs:

• tokenInBalanceRef
• directionRef
• statusRef
• mountedRef
• balancesRef
• previousStatusRef

Only sdkRef, publicClientRef, and decimalsRef clearly represent external mutable runtime handles. The rest mainly exists to avoid dependencies or guard defensive cases.

Reasonable removals without changing intended behavior:

• replace refs.statusRef.current in useReserveQuote with state.status
• replace refs.tokenInBalanceRef.current in useReserveQuote with state.tokenInBalance
• pass state.direction into useReserveBootstrap or refreshBalances instead of mirroring it through directionRef
• in setDirection, derive the new in/out balances from current state.tokenInBalance and state.tokenOutBalance instead of using balancesRef
• remove useReserveRefSync once those mirrored refs are gone
• remove the mountedRef guard from applyStatePatch unless there is a real observed unmount warning or leak

I would keep previousStatusRef for now unless overlay state is redesigned. It is carrying actual modal close behavior: slippage/confirm needs to return to the prior swap state.

3. Simplify applyStatePatch

File: packages/goodreserve-widget/src/useGoodReserveAdapter.ts

applyStatePatch currently:

• checks mountedRef
• loops through every patch key
• skips setState if values are equal

For this widget, that is a lot of code for render avoidance. A normal patch setter is easier to read:

const applyStatePatch = useCallback((patch: Partial<ReserveSwapWidgetAdapterState>) => {
  setState((current) => ({ ...current, ...patch }))
}, [])

This may allow an extra render in some no-op patch cases, but it does not change visible behavior and makes the state flow much easier to review.

4. Remove redundant component sub-themes from reserve config

File: packages/goodreserve-widget/src/config.ts

Most light_Reserve* / dark_Reserve* entries duplicate styling already defined in createComponent inside ReserveSwapView.tsx.

Examples:

• ReserveSwapShell already sets $reserveCard, $reserveBadge, and $textColor
• ReserveAmountCard already sets $reserveInputCard
• ReserveTokenBadge already sets $reserveBadge
• ReserveSurface already sets $surface
• ReserveSuccessIcon already sets $primary

5. Remove unused reserve theme values

File: packages/goodreserve-widget/src/config.ts

These values are defined but not consumed in packages/goodreserve-widget/src:

• reserveAccentSoft
• reservePositive

Reasonable removal:

• delete them from reserveTokenPreset
• delete them from reserveTheme

6. Remove unused component helper props

File: packages/goodreserve-widget/src/ReserveSwapView.tsx

DetailRow accepts valueColor, but every call site uses the default.

Reasonable removal:

• remove the valueColor prop
• set value text color directly to $textColor

File: packages/goodreserve-widget/src/ReserveSwapView.tsx

CollapsibleSection accepts testID and defaultOpen, but no current call site passes either.

Reasonable removal:

• remove testID
• remove defaultOpen
• initialize open to true

7. Remove unused quote/state fields if public API compatibility is not required

Files:

• packages/goodreserve-widget/src/widgetRuntimeContract.ts
• packages/goodreserve-widget/src/useReserveQuote.ts

priceImpactPercent is always set to 'N/A' and is not rendered by ReserveSwapView.

Reasonable removal if this package is not treating adapter state as a stable public API:

• remove priceImpactPercent from ReserveSwapQuoteView
• remove the assignment in useReserveQuote
• remove the mock fixture values

File: packages/goodreserve-widget/src/widgetRuntimeContract.ts

hasProvider is set and present in fixtures, but it is not read by the goodreserve view. The visible state is driven by status.

Reasonable removal if external consumers do not rely on adapter state shape:

• remove hasProvider from state
• remove all assignments and fixtures

This is slightly broader than the other removals because it touches exported types and Storybook fixtures.

8. Simplify useMemo around actions

File: packages/goodreserve-widget/src/useGoodReserveAdapter.ts

The actions object is memoized with a long dependency list. Current consumers do not appear to depend on stable action object identity.

Reasonable removal:

• build actions as a plain object
• keep executeSwap as-is if desired

This removes a lot of dependency bookkeeping and makes the action code easier to scan.

9. Reduce narrative comments after simplification

Files:

• packages/goodreserve-widget/src/ReserveSwapView.tsx
• packages/goodreserve-widget/src/useReserveBootstrap.ts
• packages/goodreserve-widget/src/useReserveQuote.ts
• packages/goodreserve-widget/src/useReserveSwap.ts

There are many block comments explaining what the next few lines already say. Once the code is reduced, most of these should go too.

Suggested First Cleanup PR

Start with the smallest no-behavior-change cleanup:

1. Delete amount.ts and remove its call sites.
2. Remove unused reserveAccentSoft and reservePositive.
3. Remove redundant light_Reserve* / dark_Reserve* config entries.
4. Remove unused DetailRow and CollapsibleSection props.

That gives an immediate readability win without touching async SDK flow.

[L03TJ3]

@Ryjen1

[L03TJ3]

Yes, should be fixed in createComponent and supporting forwarding refs

export function createComponent(base: TamaguiComponent, options: Record<string, any> & { name: string; extends?: string }): any {
  const { extends: extendsName, ...rest } = options

  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  const styledComponent = styled(base as any, rest) as any
  const component =
    typeof styledComponent.styleable === 'function'
      ? styledComponent.styleable((props: any, ref: React.Ref<unknown>) =>
          React.createElement(styledComponent, { ...props, ref }),
        )
      : React.forwardRef<unknown, any>((props, ref) =>
          React.createElement(styledComponent, { ...props, ref }),
        )

  component.displayName = rest.name

  const variantKeys = rest.variants ? Object.keys(rest.variants) : []

  registerComponent({
    name: rest.name,
    extends: extendsName,
    themeKeys: [...THEME_KEY_NAMES] as string[],
    variants: variantKeys,
  })

  return component
}

Drawer can be reverted to its original version

[L03TJ3]

Hey @Ryjen1 making sure you have seen the review request
are things being worked on, in progress?

let us know please!

[Ryjen1]

Sure, Thanks for the review @L03TJ3 . Still on it

[L03TJ3]

Thanks for the contribution, nice following the design references and good final clean up!