-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: WalletDropdownFundLink
#1021
Changes from 49 commits
c62ca07
1f8aa0e
ae3e8dd
89112aa
ec45503
9212b30
5bb09e7
6468a62
7a98d5a
7d8fd70
c80f4b4
bff46b6
7495d92
a8dc224
48a926f
dd771bf
3cbe66f
3b5a079
83b0cc3
06ada06
7e0f393
d726dff
ad0127b
4252671
96597ca
48a25bc
c74b3a1
99816a0
5325599
7387e2d
2efd1d7
71ac9f5
320004a
c2d8dfe
a0f5829
c1d3365
a9c6a57
f162d4b
0ba3f3c
07d4bf3
5cda967
0b536ac
6874ac2
e262968
3f15e98
09e82f7
56a243a
a4ba654
91ee34e
df97e48
f040a52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@coinbase/onchainkit": patch | ||
--- | ||
|
||
**feat**: `WalletDropdownFundLink` - add a wallet dropdown link for the keys.coinbase.com funding flow by @0xAlec #1021 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -82,6 +82,20 @@ export type WalletDropdownDisconnectReact = { | |
}; | ||
``` | ||
|
||
## `WalletDropdownFundLinkReact` | ||
|
||
```ts | ||
export type WalletDropdownFundLinkReact = { | ||
className?: string; // Optional className override for the element | ||
icon?: ReactNode; // Optional icon override | ||
rel?: string; // Specifies the relationship between the current document and the linked document | ||
openIn?: 'popup' | 'tab'; // Whether to open the funding flow in a tab or a popup window | ||
target?: string; // Where to open the target if `openIn` is set to tab | ||
text?: string; // Optional text override | ||
popupSize?: 'sm' | 'md' | 'lg'; // Size of the popup window if `openIn` is set to `popup` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we should even allow the popupSize to be adjustable. It seems to add a bit of complexity. What if we only supported the "recommended" size - what we think looks best? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not opposed to just hardcoding since we already allow the user to override the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but IMO having to perform this calculation as a developer is pretty annoying, we should have some sort of pre-built responsive design BUT still allow the user full customizability There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed + to having a pre-build responsive design What type of size customizability would the user want? Personally, as long as the popup window looks "normal" and everything is in view then I would be happy with it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How the popup looks like? I didn't see a screenshot in the recap. Can we see how each form looks like. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added screenshots on desktop |
||
}; | ||
``` | ||
|
||
## `WalletDropdownLinkReact` | ||
|
||
```ts | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { fill } from '../../styles/theme'; | ||
|
||
export const fundWalletSvg = ( | ||
<svg | ||
role="img" | ||
aria-label="fund-wallet-svg" | ||
width="100%" | ||
height="100%" | ||
viewBox="0 0 20 20" | ||
fill="none" | ||
xmlns="http://www.w3.org/2000/svg" | ||
> | ||
<path | ||
d="M8 16C12.4183 16 16 12.4183 16 8C16 3.58172 12.4183 0 8 0C3.58172 0 0 3.58172 0 8C0 12.4183 3.58172 16 8 16ZM1.63962 4.33685C5.1477 6.36224 9.63276 5.16047 11.6582 1.65239C9.63276 5.16047 10.8345 9.64553 14.3413 11.6702C10.8345 9.64553 6.35021 10.846 4.32482 14.3541C6.35021 10.846 5.1477 6.36224 1.63962 4.33685Z" | ||
fill="#0A0B0D" | ||
0xAlec marked this conversation as resolved.
Show resolved
Hide resolved
|
||
className={fill.defaultReverse} | ||
/> | ||
</svg> | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,118 @@ | ||
import { fireEvent, render, screen } from '@testing-library/react'; | ||
import { expect, it, vi } from 'vitest'; | ||
import { version } from '../../version'; | ||
import type { WindowSizes } from '../types'; | ||
import { WalletDropdownFundLink } from './WalletDropdownFundLink'; | ||
|
||
const FUNDING_URL = `http://keys.coinbase.com/fund?dappName=&dappUrl=http%3A%2F%2Flocalhost%3A3000%2F&version=${version}&source=onchainkit`; | ||
|
||
describe('WalletDropdownFundLink', () => { | ||
0xAlec marked this conversation as resolved.
Show resolved
Hide resolved
|
||
it('renders correctly with default props', () => { | ||
render(<WalletDropdownFundLink />); | ||
|
||
const linkElement = screen.getByRole('link'); | ||
expect(linkElement).toBeInTheDocument(); | ||
expect(linkElement).toHaveAttribute('href', FUNDING_URL); | ||
expect(screen.getByText('Fund wallet')).toBeInTheDocument(); | ||
0xAlec marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
|
||
it('renders correctly with custom icon element', () => { | ||
const customIcon = <svg aria-label="custom-icon" />; | ||
render(<WalletDropdownFundLink icon={customIcon} />); | ||
|
||
const linkElement = screen.getByRole('link'); | ||
expect(linkElement).toBeInTheDocument(); | ||
expect(linkElement).toHaveAttribute('href', FUNDING_URL); | ||
expect(screen.getByText('Fund wallet')).toBeInTheDocument(); | ||
expect(screen.getByLabelText('custom-icon')).toBeInTheDocument(); | ||
}); | ||
|
||
it('renders correctly with custom text', () => { | ||
render(<WalletDropdownFundLink text="test" />); | ||
|
||
const linkElement = screen.getByRole('link'); | ||
expect(linkElement).toBeInTheDocument(); | ||
expect(linkElement).toHaveAttribute('href', FUNDING_URL); | ||
expect(screen.getByText('test')).toBeInTheDocument(); | ||
}); | ||
|
||
it('opens a new window when clicked with type="window" (default size medium)', () => { | ||
// Mock window.open | ||
const mockOpen = vi.fn(); | ||
vi.stubGlobal('open', mockOpen); | ||
|
||
// Mock window.screen | ||
vi.stubGlobal('screen', { width: 1024, height: 768 }); | ||
|
||
render(<WalletDropdownFundLink openIn="popup" />); | ||
|
||
const linkElement = screen.getByText('Fund wallet'); | ||
fireEvent.click(linkElement); | ||
|
||
// Check if window.open was called with the correct arguments | ||
expect(mockOpen).toHaveBeenCalledWith( | ||
expect.stringContaining('http://keys.coinbase.com/fund'), | ||
undefined, | ||
expect.stringContaining( | ||
'width=297,height=371,resizable,scrollbars=yes,status=1,left=364,top=199', | ||
), | ||
); | ||
|
||
// Clean up | ||
vi.unstubAllGlobals(); | ||
}); | ||
|
||
const testCases: WindowSizes = { | ||
sm: { width: '23vw', height: '28.75vw' }, | ||
md: { width: '29vw', height: '36.25vw' }, | ||
lg: { width: '35vw', height: '43.75vw' }, | ||
}; | ||
|
||
const minWidth = 280; | ||
const minHeight = 350; | ||
|
||
for (const [size, { width, height }] of Object.entries(testCases)) { | ||
it(`opens a new window when clicked with type="window" and popupSize="${size}"`, () => { | ||
const mockOpen = vi.fn(); | ||
const screenWidth = 1024; | ||
const screenHeight = 768; | ||
const innerWidth = 1024; | ||
const innerHeight = 768; | ||
|
||
vi.stubGlobal('open', mockOpen); | ||
vi.stubGlobal('screen', { width: screenWidth, height: screenHeight }); | ||
|
||
render( | ||
<WalletDropdownFundLink | ||
openIn="popup" | ||
popupSize={size as keyof typeof testCases} | ||
/>, | ||
); | ||
|
||
const linkElement = screen.getByText('Fund wallet'); | ||
fireEvent.click(linkElement); | ||
|
||
const vwToPx = (vw: string) => | ||
Math.round((Number.parseFloat(vw) / 100) * innerWidth); | ||
|
||
const expectedWidth = Math.max(minWidth, vwToPx(width)); | ||
const expectedHeight = Math.max(minHeight, vwToPx(height)); | ||
const adjustedHeight = Math.min( | ||
expectedHeight, | ||
Math.round(innerHeight * 0.9), | ||
); | ||
const expectedLeft = Math.round((screenWidth - expectedWidth) / 2); | ||
const expectedTop = Math.round((screenHeight - adjustedHeight) / 2); | ||
expect(mockOpen).toHaveBeenCalledWith( | ||
expect.stringContaining('http://keys.coinbase.com/fund'), | ||
undefined, | ||
expect.stringContaining( | ||
`width=${expectedWidth},height=${adjustedHeight},resizable,scrollbars=yes,status=1,left=${expectedLeft},top=${expectedTop}`, | ||
), | ||
); | ||
|
||
vi.unstubAllGlobals(); | ||
vi.clearAllMocks(); | ||
}); | ||
} | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
import { useCallback, useMemo } from 'react'; | ||
import { useEffect, useState } from 'react'; | ||
import { cn, pressable, text as themeText } from '../../styles/theme'; | ||
import { version } from '../../version'; | ||
import { useIcon } from '../hooks/useIcon'; | ||
import type { WalletDropdownFundLinkReact } from '../types'; | ||
import { getWindowDimensions } from '../utils/getWindowDimensions'; | ||
|
||
export function WalletDropdownFundLink({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit. alphabetical order There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in new diff, updated in |
||
className, | ||
icon = 'fundWallet', | ||
rel, | ||
popupFeatures, | ||
openIn = 'tab', | ||
target, | ||
text = 'Fund wallet', | ||
0xAlec marked this conversation as resolved.
Show resolved
Hide resolved
|
||
popupSize = 'md', | ||
}: WalletDropdownFundLinkReact) { | ||
const [fundingUrl, setFundingUrl] = useState(''); | ||
|
||
const iconSvg = useIcon({ icon }); | ||
|
||
useEffect(() => { | ||
const currentURL = window.location.href; | ||
const tabName = document.title; | ||
const url = `http://keys.coinbase.com/fund?dappName=${encodeURIComponent( | ||
tabName, | ||
)}&dappUrl=${encodeURIComponent(currentURL)}&version=${encodeURIComponent( | ||
version, | ||
)}&source=onchainkit`; | ||
setFundingUrl(url); | ||
}, []); | ||
|
||
const handleClick = useCallback( | ||
(e: React.MouseEvent) => { | ||
e.preventDefault(); | ||
const { width, height } = getWindowDimensions(popupSize); | ||
|
||
const left = Math.round((window.screen.width - width) / 2); | ||
const top = Math.round((window.screen.height - height) / 2); | ||
|
||
const windowFeatures = | ||
popupFeatures || | ||
`width=${width},height=${height},resizable,scrollbars=yes,status=1,left=${left},top=${top}`; | ||
window.open(fundingUrl, target, windowFeatures); | ||
}, | ||
[fundingUrl, popupFeatures, popupSize, target], | ||
); | ||
|
||
const overrideClassName = cn( | ||
pressable.default, | ||
'relative flex items-center px-4 py-3', | ||
className, | ||
); | ||
|
||
const linkContent = useMemo( | ||
() => ( | ||
<> | ||
<div className="-translate-y-1/2 absolute top-1/2 left-4 flex h-4 w-4 items-center justify-center"> | ||
{iconSvg} | ||
</div> | ||
<span className={cn(themeText.body, 'pl-6')}>{text}</span> | ||
</> | ||
), | ||
[iconSvg, text], | ||
); | ||
|
||
if (openIn === 'tab') { | ||
return ( | ||
<a | ||
className={overrideClassName} | ||
href={fundingUrl} | ||
target={target} | ||
rel={rel} | ||
> | ||
{linkContent} | ||
</a> | ||
); | ||
} | ||
return ( | ||
<button type="button" className={overrideClassName} onClick={handleClick}> | ||
{linkContent} | ||
</button> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
import { renderHook } from '@testing-library/react'; | ||
import { describe, expect, it } from 'vitest'; | ||
import { fundWalletSvg } from '../../internal/svg/fundWallet'; | ||
import { walletSvg } from '../../internal/svg/walletSvg'; | ||
import { useIcon } from './useIcon'; | ||
|
||
describe('useIcon', () => { | ||
it('should return null when icon is undefined', () => { | ||
const { result } = renderHook(() => useIcon({ icon: undefined })); | ||
expect(result.current).toBeNull(); | ||
}); | ||
|
||
it('should return walletSvg when icon is "wallet"', () => { | ||
const { result } = renderHook(() => useIcon({ icon: 'wallet' })); | ||
expect(result.current).toBe(walletSvg); | ||
}); | ||
|
||
it('should return fundWalletSvg when icon is "fundWallet"', () => { | ||
const { result } = renderHook(() => useIcon({ icon: 'fundWallet' })); | ||
expect(result.current).toBe(fundWalletSvg); | ||
}); | ||
|
||
it('should memoize the result for undefined', () => { | ||
const { result, rerender } = renderHook(() => useIcon({}), { | ||
initialProps: {}, | ||
}); | ||
|
||
const initialResult = result.current; | ||
rerender({}); | ||
expect(result.current).toBe(initialResult); | ||
}); | ||
|
||
it('should memoize the result for wallet', () => { | ||
const { result, rerender } = renderHook(({ icon }) => useIcon({ icon }), { | ||
initialProps: { icon: 'wallet' }, | ||
}); | ||
|
||
const initialResult = result.current; | ||
rerender({ icon: 'wallet' }); | ||
expect(result.current).toBe(initialResult); | ||
}); | ||
|
||
it('should memoize the result for fundWallet', () => { | ||
const { result, rerender } = renderHook(({ icon }) => useIcon({ icon }), { | ||
initialProps: { icon: 'fundWallet' }, | ||
}); | ||
|
||
const initialResult = result.current; | ||
rerender({ icon: 'fundWallet' }); | ||
expect(result.current).toBe(initialResult); | ||
}); | ||
|
||
it('should memoize the result for custom icon', () => { | ||
const customIcon = <svg aria-label="custom-icon" />; | ||
const { result, rerender } = renderHook(({ icon }) => useIcon({ icon }), { | ||
initialProps: { icon: customIcon }, | ||
}); | ||
|
||
const initialResult = result.current; | ||
rerender({ icon: customIcon }); | ||
expect(result.current).toBe(initialResult); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import { isValidElement, useMemo } from 'react'; | ||
import { fundWalletSvg } from '../../internal/svg/fundWallet'; | ||
import { walletSvg } from '../../internal/svg/walletSvg'; | ||
|
||
export const useIcon = ({ icon }: { icon?: React.ReactNode }) => { | ||
return useMemo(() => { | ||
if (icon === undefined) { | ||
return null; | ||
} | ||
switch (icon) { | ||
case 'wallet': | ||
return walletSvg; | ||
case 'fundWallet': | ||
return fundWalletSvg; | ||
} | ||
if (isValidElement(icon)) { | ||
return icon; | ||
} | ||
}, [icon]); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.w3schools.com/tags/att_a_target.asp
target
is a different attribute thanhref
-target
will control wherehref
opens (i.e. new tab or replace current tab)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
href
is hardcoded here to be keys.coinbase.com/fundThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks I was confusing the two
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think overall there is an interesting question to ask. What are properties we want to be able to customize and what we don't want to have customized.
Otherwise it feels a weird overalap with the Link component.