Feat: Integrate ubeswap savings widget #619 by Oluwatomilola · Pull Request #625 · GoodDollar/GoodProtocolUI

Oluwatomilola · 2026-03-03T01:25:20Z

Description

This PR integrates the GoodDollar Savings Widget into GoodDapp as a first-class page.

Changes include:
Added a new Savings menu item to the sidebar.
Created a new /savings page using the same layout structure as Bridge/Swap/Claim.
Embedded the GooddollarSavingsWidget via a dedicated wrapper component.
Added contextual FAQs for the Savings feature.
This implements the requirements described in issue #619.

About: https://github.com/GoodDollar/GoodProtocolUI/issues/#619
fixes: #619 #619 #619

How Has This Been Tested?

Ran the app locally using yarn start.

Navigated to the new Savings route from the sidebar.

Verified that the Savings Widget loads correctly.

Confirmed layout consistency with Bridge/Swap/Claim pages.

Manually tested wallet connection and widget rendering.

Checklist:

- Add Savings page component with widget integration - Add /savings route to router - Add Savings menu item to sidebar - Add 6 FAQ items for Savings section - Use split-screen layout matching other pages

sourcery-ai

Hey - I've found 2 issues, and left some high level feedback:

The savings widget script is reloaded on every account change but only ever appended (and only to the container div), which can lead to multiple <script> tags being added over time; consider loading it once (empty dependency array) and/or guarding with an id/existence check, and appending to document.head or body instead of the container.
The <div id="savings-widget-container"> only hosts the script element and does not render a specific web component instance (e.g., <gooddollar-savings-widget ... />), so if the widget does not self-mount into the container, it may never display; double-check whether explicitly rendering the custom element is required and add it if so.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The savings widget script is reloaded on every `account` change but only ever appended (and only to the container div), which can lead to multiple `<script>` tags being added over time; consider loading it once (empty dependency array) and/or guarding with an `id`/existence check, and appending to `document.head` or `body` instead of the container.
- The `<div id="savings-widget-container">` only hosts the script element and does not render a specific web component instance (e.g., `<gooddollar-savings-widget ... />`), so if the widget does not self-mount into the container, it may never display; double-check whether explicitly rendering the custom element is required and add it if so.

## Individual Comments

### Comment 1
<location path="src/pages/gd/Savings/index.tsx" line_range="37-46" />
<code_context>
+    useEffect(() => {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid reloading the widget script on every account change unless the widget actually depends on it.

Because the effect depends on `account` but the script URL/injection don’t, every account change tears down and reinjects the script, causing extra network/CPU work and potential re‑init bugs. If the widget truly doesn’t depend on `account`, make the dependency array `[]` so it runs only on mount. If it does depend on `account`, prefer passing that via the widget’s own API (e.g., attributes/props) instead of reloading the script itself.

Suggested implementation:

```typescript
    useEffect(() => {

```

```typescript
    }, [])

```

If the dependency array is not exactly `[account]` (for example `[account, chainId]` or similar), you should update that full dependency array to `[]` instead, so that the widget script is only loaded once on mount and not reloaded on account changes. If the widget actually needs `account` to render correctly, prefer passing `account` into the widget via its attributes/props or configuration object inside the effect, rather than by reloading the script tag itself.
</issue_to_address>

### Comment 2
<location path="src/pages/gd/Savings/index.tsx" line_range="22" />
<code_context>
+    </VStack>
+)
+
+const SavingsWidgetContainer: React.FC = () => {
+    const containerRef = useRef<HTMLDivElement>(null)
+    const [isLoading, setIsLoading] = useState(true)
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the widget container to use a reusable script-loading hook, a single derived status, and NativeBase layout components to simplify control flow and concerns.

You can simplify this component without changing behavior by extracting the script-loading concern, tightening dependencies, and consolidating state.

### 1. Extract script loading into a small hook

This removes DOM manipulation from the page component and makes it reusable/testable:

```ts
// hooks/useExternalScript.ts
import { useEffect, useState } from 'react'

export const useExternalScript = (src: string) => {
  const [status, setStatus] = useState<'idle' | 'loading' | 'ready' | 'error'>('idle')

  useEffect(() => {
    if (!src) return
    setStatus('loading')

    const script = document.createElement('script')
    script.src = src
    script.async = true
    script.defer = true

    script.onload = () => setStatus('ready')
    script.onerror = () => setStatus('error')

    document.body.appendChild(script)

    return () => {
      script.onload = null
      script.onerror = null
      script.remove()
    }
  }, [src])

  return status
}
```

Then your widget container becomes focused on layout:

```tsx
const SavingsWidgetContainer: React.FC = () => {
  const containerRef = useRef<HTMLDivElement>(null)
  const status = useExternalScript(
    'https://unpkg.com/@gooddollar/savings-widget@latest/dist/index.js'
  )
  const isLoading = status === 'loading'
  const hasError = status === 'error'
  const isReady = status === 'ready'

  // ...rendering below...
}
```

### 2. Remove unnecessary `account` dependency

The script doesn’t depend on `account`, so reloading it on account change adds complexity and flicker. If you keep the current inline approach, you can change:

```ts
useEffect(() => {
  // ...
}, [account])
```

to:

```ts
useEffect(() => {
  // ...
}, []) // load once per mount
```

### 3. Consolidate loading/error/success handling

Instead of two booleans and inline `display` logic, derive a single `status`:

```tsx
const [status, setStatus] = useState<'loading' | 'error' | 'ready'>('loading')

useEffect(() => {
  // on success:
  setStatus('ready')
  // on error:
  setStatus('error')
}, [])
```

Then JSX becomes more linear:

```tsx
{status === 'loading' && (
  <VStack alignItems="center" justifyContent="center" minH="300px" w="100%">
    <Spinner color="gdPrimary" size="lg" />
    <Text fontSize="sm" color="goodGrey.400" mt={4}>
      Loading savings widget...
    </Text>
  </VStack>
)}

{status === 'error' && (
  <VStack alignItems="center" justifyContent="center" minH="300px" w="100%">
    <Text fontSize="sm" color="red.500" textAlign="center">
      Unable to load the savings widget. Please try again later.
    </Text>
  </VStack>
)}

<Box
  as="div"
  ref={containerRef}
  id="savings-widget-container"
  w="100%"
  minH="500px"
  display={status === 'ready' ? 'flex' : 'none'}
/>
```

### 4. Keep layout consistent with NativeBase

Instead of a raw `div`, use `Box as="div"` to stay within the NativeBase layout system (as above). This keeps styling consistent and avoids mixing paradigms while still giving the web component the `div` it needs.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

sourcery-ai · 2026-03-03T01:27:42Z

src/pages/gd/Savings/index.tsx

+    useEffect(() => {
+        const loadWidget = async () => {
+            try {
+                setIsLoading(true)
+                setHasError(false)
+
+                // Try to load the savings widget script from CDN
+                // The widget is available as a web component from GoodSDKs/savings-widget
+                const script = document.createElement('script')
+                script.src = 'https://unpkg.com/@gooddollar/savings-widget@latest/dist/index.js'


suggestion (bug_risk): Avoid reloading the widget script on every account change unless the widget actually depends on it.

Because the effect depends on account but the script URL/injection don’t, every account change tears down and reinjects the script, causing extra network/CPU work and potential re‑init bugs. If the widget truly doesn’t depend on account, make the dependency array [] so it runs only on mount. If it does depend on account, prefer passing that via the widget’s own API (e.g., attributes/props) instead of reloading the script itself.

Suggested implementation:

useEffect(() => {

}, [])

If the dependency array is not exactly [account] (for example [account, chainId] or similar), you should update that full dependency array to [] instead, so that the widget script is only loaded once on mount and not reloaded on account changes. If the widget actually needs account to render correctly, prefer passing account into the widget via its attributes/props or configuration object inside the effect, rather than by reloading the script tag itself.

sourcery-ai · 2026-03-03T01:27:42Z

src/pages/gd/Savings/index.tsx

+    </VStack>
+)
+
+const SavingsWidgetContainer: React.FC = () => {


issue (complexity): Consider refactoring the widget container to use a reusable script-loading hook, a single derived status, and NativeBase layout components to simplify control flow and concerns.

You can simplify this component without changing behavior by extracting the script-loading concern, tightening dependencies, and consolidating state.

1. Extract script loading into a small hook

This removes DOM manipulation from the page component and makes it reusable/testable:

// hooks/useExternalScript.ts import { useEffect, useState } from 'react' export const useExternalScript = (src: string) => { const [status, setStatus] = useState<'idle' | 'loading' | 'ready' | 'error'>('idle') useEffect(() => { if (!src) return setStatus('loading') const script = document.createElement('script') script.src = src script.async = true script.defer = true script.onload = () => setStatus('ready') script.onerror = () => setStatus('error') document.body.appendChild(script) return () => { script.onload = null script.onerror = null script.remove() } }, [src]) return status }

Then your widget container becomes focused on layout:

const SavingsWidgetContainer: React.FC = () => { const containerRef = useRef<HTMLDivElement>(null) const status = useExternalScript( 'https://unpkg.com/@gooddollar/savings-widget@latest/dist/index.js' ) const isLoading = status === 'loading' const hasError = status === 'error' const isReady = status === 'ready' // ...rendering below... }

2. Remove unnecessary account dependency

The script doesn’t depend on account, so reloading it on account change adds complexity and flicker. If you keep the current inline approach, you can change:

useEffect(() => { // ... }, [account])

to:

useEffect(() => { // ... }, []) // load once per mount

3. Consolidate loading/error/success handling

Instead of two booleans and inline display logic, derive a single status:

const [status, setStatus] = useState<'loading' | 'error' | 'ready'>('loading') useEffect(() => { // on success: setStatus('ready') // on error: setStatus('error') }, [])

Then JSX becomes more linear:

{status === 'loading' && ( <VStack alignItems="center" justifyContent="center" minH="300px" w="100%"> <Spinner color="gdPrimary" size="lg" /> <Text fontSize="sm" color="goodGrey.400" mt={4}> Loading savings widget... </Text> </VStack> )} {status === 'error' && ( <VStack alignItems="center" justifyContent="center" minH="300px" w="100%"> <Text fontSize="sm" color="red.500" textAlign="center"> Unable to load the savings widget. Please try again later. </Text> </VStack> )} <Box as="div" ref={containerRef} id="savings-widget-container" w="100%" minH="500px" display={status === 'ready' ? 'flex' : 'none'} />

4. Keep layout consistent with NativeBase

Instead of a raw div, use Box as="div" to stay within the NativeBase layout system (as above). This keeps styling consistent and avoids mixing paradigms while still giving the web component the div it needs.

Oluwatomilola

@L03TJ3 kindly review

L03TJ3 · 2026-03-30T16:05:49Z

@Oluwatomilola your build fails, can you check the build logs

Oluwatomilola · 2026-03-30T17:48:47Z

Alright, on it.

- Fix faqSavingsCopy array left unclosed after merge with master (copies.tsx) - Replace Box with plain div to fix TypeScript error on 'as' prop (Savings/index.tsx)

L03TJ3 · 2026-04-03T14:16:12Z

.kombai/resources/image-b0682ce1-1.png

dont need to be committed

Noted. I would remove it.

L03TJ3 · 2026-04-03T14:20:19Z

src/pages/gd/Savings/index.tsx

+                <VStack alignItems="center" justifyContent="center" minH="300px" w="100%">
+                    <Text fontSize="sm" color="red.500" textAlign="center" mb={4} fontWeight="bold">
+                        Unable to Load Savings Widget
+                    </Text>
+                    <Text fontSize="xs" color="goodGrey.500" textAlign="center" mb={3}>
+                        The widget script is not available. To enable this feature:
+                    </Text>
+                    <VStack fontSize="xs" color="goodGrey.400" textAlign="left" alignItems="flex-start" space={1}>
+                        <Text>1. Clone GoodSDKs/packages/savings-widget</Text>
+                        <Text>2. Run: yarn install && yarn build</Text>
+                        <Text>3. Copy dist/index.global.js to public/</Text>
+                        <Text>4. Restart the development server</Text>
+                    </VStack>
+                    <Button
+                        size="sm"
+                        colorScheme="blue"
+                        variant="ghost"
+                        mt={3}
+                        onPress={() =>
+                            window.open(
+                                'https://github.com/GoodDollar/GoodSDKs/tree/main/packages/savings-widget',


What is this?
why would this make any sense to show on a user-facing page?

My bad.
Exposing build/setup instructions in a user-facing page is not appropriate. That was a misstep on my part while trying to handle the missing widget script case.

L03TJ3 · 2026-04-03T14:24:10Z

src/pages/Savings/index.global.js

Have you read the readme for the savings-widget?
https://github.com/GoodDollar/GoodSDKs/blob/main/packages/savings-widget/README.md

Yes, I have.
I will read through it again to see what I'm missing.

L03TJ3 · 2026-04-03T14:25:19Z

src/index.tsx

revert any changes that don't are not related to the requested change

Alright, will do that.

L03TJ3 · 2026-04-03T14:33:00Z

src/pages/gd/Savings/index.tsx

+)
+
+const SavingsWidgetContainer: React.FC = () => {
+    const containerRef = useRef<HTMLDivElement>(null)


I had expected you followed the readme at least and how to use it.
there are ways that the iife (the web-component compiled file, not the one from dist)
could have worked.

But you might have had some blockers, and I would have expected them to be raised.
It could have been an easy fix if you would have followed the readme.
You will now have to remove and revert a lot of this.

Please, when things are unclear, ask questions, push draft PRs.
you raised a couple of blockers before but there was never code for me to review.
and 'I have some eslint errors' is really hard to help with.

However. what was also missing was a published npm package.
That is my bad.
I am going to publish it now and you can use add @goodsdks/savings-widget as package. You can look at the demo-webcomponents to see how it can be used: https://github.com/GoodDollar/GoodSDKs/blob/main/apps/demo-webcomponents/src/index.js

The demo loads it high-level in html. once you import from the package you can use the html anywhere in react. (if anything not working, please raise blockers early)

Thanks for the detailed feedback.

My apologies for deviating from the intended integration approach.
I did follow the README. I cloned the goodSDK quite alright, but I was a bit unclear on the subsequent steps to integrating the component and I indeed had blockers. I should have stated them clearly, my bad.

Moving forward I will raise blockers early, and I’ll make sure to push draft PRs with partial progress next time so issues can be caught earlier.

Please can you add a link to the npm package if it's available.

I’ll go ahead and make the required changes.

you can add it with yarn: yarn add @goodsdks/savings-widget

Oluwatomilola added 2 commits March 3, 2026 02:02

feat: integrate savings widget

6bbf63f

feat: integrate ubeswap savings widget GoodDollar#619

a7e9668

- Add Savings page component with widget integration - Add /savings route to router - Add Savings menu item to sidebar - Add 6 FAQ items for Savings section - Use split-screen layout matching other pages

Oluwatomilola had a problem deploying to external March 3, 2026 01:25 — with GitHub Actions Failure

sourcery-ai bot reviewed Mar 3, 2026

View reviewed changes

Oluwatomilola added 2 commits March 5, 2026 18:09

feat: integrate savings widget

98b0632

feat: integrate savings widget

3e104f4

Oluwatomilola had a problem deploying to external March 5, 2026 17:32 — with GitHub Actions Failure

Oluwatomilola requested a deployment to external March 8, 2026 14:54 — with GitHub Actions Waiting

feat: savings widget buttons-fix

22fd77c

Oluwatomilola force-pushed the feat-savings-integration branch from 1ca3f78 to 22fd77c Compare March 11, 2026 12:05

Oluwatomilola requested a deployment to external March 11, 2026 12:05 — with GitHub Actions Waiting

Oluwatomilola requested a deployment to external March 11, 2026 14:03 — with GitHub Actions Waiting

Oluwatomilola requested a deployment to external March 11, 2026 14:47 — with GitHub Actions Waiting

feat: savings widget buttons-fix

52390d5

Oluwatomilola force-pushed the feat-savings-integration branch from 5fe3994 to 52390d5 Compare March 11, 2026 15:15

Oluwatomilola requested a deployment to external March 11, 2026 15:16 — with GitHub Actions Waiting

Merge branch 'master' into feat-savings-integration

7b7c260

Oluwatomilola temporarily deployed to external March 11, 2026 15:18 — with GitHub Actions Inactive

Oluwatomilola commented Mar 23, 2026

View reviewed changes

Oluwatomilola had a problem deploying to preview March 26, 2026 15:30 — with GitHub Actions Failure

Oluwatomilola temporarily deployed to external March 30, 2026 20:11 — with GitHub Actions Inactive

Oluwatomilola deployed to preview April 2, 2026 17:21 — with GitHub Actions View deployment

fix: resolve build errors from bad merge conflict

0732551

- Fix faqSavingsCopy array left unclosed after merge with master (copies.tsx) - Replace Box with plain div to fix TypeScript error on 'as' prop (Savings/index.tsx)

Oluwatomilola force-pushed the feat-savings-integration branch from 6ccb13d to 0732551 Compare April 2, 2026 22:59

Oluwatomilola deployed to external April 2, 2026 22:59 — with GitHub Actions Active

Oluwatomilola deployed to preview April 3, 2026 11:48 — with GitHub Actions View deployment

L03TJ3 requested changes Apr 3, 2026

View reviewed changes

Conversation

Oluwatomilola commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Checklist:

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

sourcery-ai bot Mar 3, 2026

Choose a reason for hiding this comment

1. Extract script loading into a small hook

2. Remove unnecessary account dependency

3. Consolidate loading/error/success handling

4. Keep layout consistent with NativeBase

Uh oh!

Oluwatomilola left a comment

Choose a reason for hiding this comment

Uh oh!

L03TJ3 commented Mar 30, 2026

Uh oh!

Oluwatomilola commented Mar 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Oluwatomilola commented Mar 3, 2026 •

edited

Loading

2. Remove unnecessary `account` dependency