Skip to content
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

Merged
merged 51 commits into from
Aug 15, 2024
Merged
Changes from 1 commit
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
c62ca07
`WalletDropdownFundLink` component
0xAlec Aug 9, 2024
1f8aa0e
types
0xAlec Aug 9, 2024
ae3e8dd
window mode
0xAlec Aug 9, 2024
89112aa
docs
0xAlec Aug 9, 2024
ec45503
lint
0xAlec Aug 9, 2024
9212b30
fix imports
0xAlec Aug 9, 2024
5bb09e7
organize export
0xAlec Aug 9, 2024
6468a62
changeset
0xAlec Aug 9, 2024
7a98d5a
fix build
0xAlec Aug 9, 2024
7d8fd70
alphabetical export
0xAlec Aug 9, 2024
c80f4b4
format
0xAlec Aug 9, 2024
bff46b6
cleanup
0xAlec Aug 9, 2024
7495d92
update prop names
0xAlec Aug 9, 2024
a8dc224
add test for `windowSize`
0xAlec Aug 9, 2024
48a926f
don't use prop destructuring
0xAlec Aug 9, 2024
dd771bf
lint and format
0xAlec Aug 9, 2024
3cbe66f
organize and add comments
0xAlec Aug 9, 2024
3b5a079
Update witty-crabs-change.md
0xAlec Aug 9, 2024
83b0cc3
remove component docs
0xAlec Aug 9, 2024
06ada06
add `onchainkit=version` to params
0xAlec Aug 9, 2024
7e0f393
`Fund Wallet` default copy
0xAlec Aug 9, 2024
d726dff
format
0xAlec Aug 9, 2024
ad0127b
add newline
0xAlec Aug 9, 2024
4252671
fix tests
0xAlec Aug 9, 2024
96597ca
sentence case
0xAlec Aug 9, 2024
48a25bc
fix
0xAlec Aug 9, 2024
c74b3a1
add `fundingUrl`
0xAlec Aug 9, 2024
99816a0
format
0xAlec Aug 9, 2024
5325599
fix test
0xAlec Aug 9, 2024
7387e2d
final
0xAlec Aug 9, 2024
2efd1d7
add windowsizes
0xAlec Aug 9, 2024
71ac9f5
use viewports
0xAlec Aug 9, 2024
320004a
`getWindowDimensions`
0xAlec Aug 9, 2024
c2d8dfe
remove comment
0xAlec Aug 9, 2024
a0f5829
`overrideClassName`
0xAlec Aug 9, 2024
c1d3365
update funding url
0xAlec Aug 14, 2024
a9c6a57
`windowSize` -> `popupSize`
0xAlec Aug 14, 2024
f162d4b
`openIn=window` -> `popup`
0xAlec Aug 14, 2024
0ba3f3c
fix tests
0xAlec Aug 14, 2024
07d4bf3
fix dimensions tests
0xAlec Aug 14, 2024
5cda967
change query params
0xAlec Aug 14, 2024
0b536ac
tweak size of `sm`
0xAlec Aug 14, 2024
6874ac2
add `popupFeatures` as an override
0xAlec Aug 14, 2024
e262968
fixes
0xAlec Aug 14, 2024
3f15e98
add comments
0xAlec Aug 14, 2024
09e82f7
wrap using `useMemo` and `useCallback`
0xAlec Aug 14, 2024
56a243a
`useIcon` hook
0xAlec Aug 14, 2024
a4ba654
`useEffect` for `fundingUrl`
0xAlec Aug 15, 2024
91ee34e
Update witty-crabs-change.md
0xAlec Aug 15, 2024
df97e48
alphabetical order
0xAlec Aug 15, 2024
f040a52
alphabetical
0xAlec Aug 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
windowSize -> popupSize
0xAlec committed Aug 14, 2024
commit a9c6a57088fe44f204a483f5a4a2ed9d38cf53e0
2 changes: 1 addition & 1 deletion site/docs/pages/wallet/types.mdx
Original file line number Diff line number Diff line change
@@ -93,7 +93,7 @@ export type WalletDropdownFundLinkReact = {
openIn?: 'window' | 'tab'; // Whether to open the funding flow in a tab or a window
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this is a popup should we probably use that word instead?

Copy link
Contributor Author

@0xAlec 0xAlec Aug 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a popup window but I like window abbreviation better since popup has a negative connotation and makes it seems like it's spam (popup spam)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window is a very special word in web. As it means both the global object, and the window browser size.

I am kind of hesitant to use that word.

target?: string; // Where to open the target if `openIn` is set to tab
Copy link
Contributor Author

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 than href - target will control where href opens (i.e. new tab or replace current tab)

Copy link
Contributor Author

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/fund

Copy link
Contributor

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

Copy link
Contributor

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.

text?: string; // Optional text override
windowSize?: 's' | 'm' | 'l'; // Size of the popup window if `openIn` is set to window
popupSize?: 'sm' | 'md' | 'lg'; // Size of the popup window if `openIn` is set to window
};
```

10 changes: 5 additions & 5 deletions src/wallet/components/WalletDropdownFundLink.test.tsx
Original file line number Diff line number Diff line change
@@ -63,16 +63,16 @@ describe('WalletDropdownFundLink', () => {
});

const testCases: WindowSizes = {
s: { width: '23vw', height: '28.75vw' },
m: { width: '29vw', height: '36.25vw' },
l: { width: '35vw', height: '43.75vw' },
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 windowSize="${size}"`, () => {
it(`opens a new window when clicked with type="window" and popupSize="${size}"`, () => {
const mockOpen = vi.fn();
const screenWidth = 1024;
const screenHeight = 768;
@@ -85,7 +85,7 @@ describe('WalletDropdownFundLink', () => {
render(
<WalletDropdownFundLink
openIn="window"
windowSize={size as keyof typeof testCases}
popupSize={size as keyof typeof testCases}
/>,
);

4 changes: 2 additions & 2 deletions src/wallet/components/WalletDropdownFundLink.tsx
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ export function WalletDropdownFundLink({
openIn = 'tab',
target,
text = 'Fund wallet',
0xAlec marked this conversation as resolved.
Show resolved Hide resolved
windowSize = 'm',
popupSize = 'md',
}: WalletDropdownFundLinkReact) {
const iconSvg = useMemo(() => {
0xAlec marked this conversation as resolved.
Show resolved Hide resolved
if (icon === undefined) {
@@ -29,7 +29,7 @@ export function WalletDropdownFundLink({

const handleClick = (e: React.MouseEvent) => {
0xAlec marked this conversation as resolved.
Show resolved Hide resolved
0xAlec marked this conversation as resolved.
Show resolved Hide resolved
e.preventDefault();
const { width, height } = getWindowDimensions(windowSize);
const { width, height } = getWindowDimensions(popupSize);

const left = Math.round((window.screen.width - width) / 2);
const top = Math.round((window.screen.height - height) / 2);
4 changes: 2 additions & 2 deletions src/wallet/types.ts
Original file line number Diff line number Diff line change
@@ -107,7 +107,7 @@ export type WalletDropdownFundLinkReact = {
openIn?: 'window' | 'tab'; // Whether to open the funding flow in a tab or a window
target?: string; // Where to open the target if `openIn` is set to tab
text?: string; // Optional text override
windowSize?: 's' | 'm' | 'l'; // Size of the popup window if `openIn` is set to window
popupSize?: 'sm' | 'md' | 'lg'; // Size of the popup window if `openIn` is set to window
};

/**
@@ -123,7 +123,7 @@ export type WalletDropdownLinkReact = {
};

export type WindowSizes = Record<
's' | 'm' | 'l',
'sm' | 'md' | 'lg',
{
width: string;
height: string;
12 changes: 6 additions & 6 deletions src/wallet/utils/getWindowDimensions.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import type { WindowSizes } from '../types';

const windowSizes: WindowSizes = {
s: { width: '23vw', height: '28.75vw' },
m: { width: '29vw', height: '36.25vw' },
l: { width: '35vw', height: '43.75vw' },
const popupSizes: WindowSizes = {
sm: { width: '23vw', height: '28.75vw' },
md: { width: '29vw', height: '36.25vw' },
lg: { width: '35vw', height: '43.75vw' },
};

export const getWindowDimensions = (size: keyof typeof windowSizes) => {
const { width, height } = windowSizes[size];
export const getWindowDimensions = (size: keyof typeof popupSizes) => {
const { width, height } = popupSizes[size];

// Define minimum sizes (in pixels)
const minWidth = 280;