Implement Fund Card component #1718

rustam-cb · 2024-12-11T18:36:49Z

What changed? Why?
Implemented a new component FundCard
P/PS: https://docs.google.com/document/d/1y57fxsga2ZLF8-ieH2OY7jvqx5468txQuek1WTYPBxE/edit?tab=t.0

Usage:

FundCard assetSymbol="ETH" />

Component is customizable and there are few components that can be passed into it.

  AmountInputComponent 
  HeaderComponent
  AmountInputTypeSwithComponent
  PaymentMethodSelectorDropdownComponent
  SubmitButtonComponent

Fiat input	Crypto input

No input (default)	payment method dropdown

Notes to reviewers
Overall component structure:

FundCard
- HeaderComponent
- AmountInputComponent
- PaymentMethodDropdownComponent
- SubmitButtonComponent

How has it been tested?

Unit tests
Manual testing

vercel · 2024-12-11T18:38:34Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
onchainkit-coverage	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 20, 2024 11:36pm
onchainkit-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 20, 2024 11:36pm
onchainkit-routes	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 20, 2024 11:36pm

dschlabach · 2024-12-20T19:50:53Z

src/fund/components/FundCardAmountInput.tsx

+        `}
+      </style>
+
+      <div className="flex" style={{ height: '78px' }}>


Stick to tailwind classes for consistency

preferably h-20 here

dschlabach · 2024-12-20T19:51:52Z

src/fund/components/FundCardAmountInput.tsx

+          className={cn(
+            text.body,
+            'border-none bg-transparent',
+            'text-[3.75rem] leading-none outline-none',


should be text-6xl instead of arbitrary

dschlabach · 2024-12-20T19:52:12Z

src/fund/components/FundCardAmountInput.tsx

+        className={cn(
+          text.body,
+          'border-none bg-transparent',
+          'text-[3.75rem] leading-none outline-none',


dschlabach · 2024-12-20T19:52:33Z

src/fund/components/FundCardAmountInput.tsx

+        style={{
+          position: 'absolute',
+          opacity: 0,
+          whiteSpace: 'nowrap',
+          pointerEvents: 'none',
+        }}


move into Tailwind utilities for consistency

@dschlabach Where is the Tailwing utilities? I can't find any in the codebase. (The css I see is autogenerated and I can't add anything there)

All Tailwind utilities are accessible anywhere in /src. You can apply them directly as classes. https://tailwindcss.com/docs/utility-first

dschlabach · 2024-12-20T20:14:03Z

src/internal/svg/successSvg.tsx

@@ -11,7 +13,7 @@ export const successSvg = (
    <title>Success SVG</title>
    <path
      d="M8 0C3.58 0 0 3.58 0 8C0 12.42 3.58 16 8 16C12.42 16 16 12.42 16 8C16 3.58 12.42 0 8 0ZM6.72667 11.5333L3.73333 8.54L4.67333 7.6L6.72667 9.65333L11.44 4.94L12.38 5.88L6.72667 11.5333Z"
-      fill="#65A30D"
+      className={className}


This will break color styling

@dschlabach It used to be hardcoded 65A30D, and now I'm using icon.success. Do you mean that icon.success is a different tone than 65A30D ?

This is likely hardcoded because the icon fill color should not update based off of themes / modes

dschlabach · 2024-12-20T20:14:12Z

src/internal/svg/errorSvg.tsx

@@ -11,7 +13,7 @@ export const errorSvg = (
    <title>Error SVG</title>
    <path
      d="M8 16C12.4183 16 16 12.4183 16 8C16 3.58171 12.4183 0 8 0C3.58172 0 0 3.58171 0 8C0 12.4183 3.58172 16 8 16ZM11.7576 5.0909L8.84853 8L11.7576 10.9091L10.9091 11.7576L8 8.84851L5.09093 11.7576L4.2424 10.9091L7.15147 8L4.2424 5.0909L5.09093 4.24239L8 7.15145L10.9091 4.24239L11.7576 5.0909Z"
-      fill="#E11D48"
+      className={className}


This will break color styling

dschlabach · 2024-12-20T20:16:13Z

src/internal/svg/coinbasePaySvg.tsx

@@ -12,7 +14,7 @@ export const coinbasePaySvg = (
      fillRule="evenodd"
      clipRule="evenodd"
      d="M10.0145 14.1666C7.82346 14.1666 6.04878 12.302 6.04878 9.99996C6.04878 7.69788 7.82346 5.83329 10.0145 5.83329C11.9776 5.83329 13.6069 7.33677 13.9208 9.30552H17.9163C17.5793 5.02774 14.172 1.66663 10.0145 1.66663C5.63568 1.66663 2.08301 5.39926 2.08301 9.99996C2.08301 14.6007 5.63568 18.3333 10.0145 18.3333C14.172 18.3333 17.5793 14.9722 17.9163 10.6944H13.9208C13.6069 12.6632 11.9776 14.1666 10.0145 14.1666Z"
-      fill="#f9fafb"
+      className={className}


Doesn't this break default styling in things like CheckoutButton?

I have set color icon.foreground by default

This will break the default styling for this SVG when themes are applied. If you need a CoinbasePaySVG to update then we might have to create a new SVG for that

dschlabach · 2024-12-20T20:17:23Z

src/internal/components/TextInput.tsx

@@ -49,6 +51,7 @@ export function TextInput({

  return (
    <input
+      style={style}


Why do we need to add an inline style attribute here? We should try to stick to className for consistency

dschlabach · 2024-12-20T20:32:20Z

src/fund/components/FundCard.tsx

+  /**
+   * Fetches and sets the exchange rate for the asset
+   */
+  useExchangeRate(assetSymbol);


Should this be moved into the provider or is there a reason it's here? A bit hard to tell what side effect it's having

cpcramer · 2024-12-20T21:20:07Z

src/internal/svg/coinbasePaySvg.tsx

+    width="16"
+    height="16"


We should lean towards width 100% and height 100% - This allows for more flexible icons, leaving the sizing to the parent component.

cpcramer · 2024-12-20T21:25:34Z

I recommend splitting this into smaller PRs, it will make reviewing and updating a lot easier and faster!

rustam-cb added 28 commits November 14, 2024 13:29

Onramp - event subscriptions migration

9fe2fde

Fund form

5c95ff9

Merge branch 'main' into fund-form

7c3fba9

Initial utils

a0486e4

Update

bffb90f

Fix lint issues

a56f488

Update

34916c7

Merge branch 'main' into onramp-api-utils

455e4bd

Merge branch 'main' into fund-form

d40eca0

Update

fc9db1f

Update

595f54e

Update

486d618

Update

6ed6c16

Update

b16da4a

Update

b7b2cd2

Update

bb348df

Update

e92e933

Update

2651a5f

Update

e91f671

Merge branch 'onramp-api-utils' into fund-form

e5b0cae

Update

a27489b

Merge branch 'main' into onramp-api-utils

51d7041

Use Api key from the config

eefc65d

Merge branch 'main' into fund-form

dbb6b80

Merge branch 'onramp-api-utils' into fund-form

eb92aa5

Update

edd3766

Update

839a045

Update

c7521d7

vercel bot had a problem deploying to Preview – onchainkit December 11, 2024 18:37 Failure

Update

af2203e

vercel bot deployed to Preview – onchainkit-routes December 19, 2024 23:24 View deployment

vercel bot deployed to Preview – onchainkit-playground December 19, 2024 23:25 View deployment

vercel bot deployed to Preview – onchainkit December 19, 2024 23:25 View deployment

vercel bot deployed to Preview – onchainkit-coverage December 19, 2024 23:26 View deployment

rustam-cb requested review from cpcramer, dschlabach and alessey December 19, 2024 23:28

dschlabach reviewed Dec 20, 2024

View reviewed changes

cpcramer reviewed Dec 20, 2024

View reviewed changes

Address comments

af24c30

vercel bot deployed to Preview – onchainkit-routes December 20, 2024 23:29 View deployment

Format

f6f1845

vercel bot deployed to Preview – onchainkit-routes December 20, 2024 23:29 View deployment

vercel bot deployed to Preview – onchainkit-playground December 20, 2024 23:32 View deployment

vercel bot deployed to Preview – onchainkit December 20, 2024 23:32 View deployment

Format

98c055d

vercel bot deployed to Preview – onchainkit-routes December 20, 2024 23:33 View deployment

vercel bot deployed to Preview – onchainkit December 20, 2024 23:34 View deployment

vercel bot deployed to Preview – onchainkit-playground December 20, 2024 23:34 View deployment

vercel bot deployed to Preview – onchainkit-coverage December 20, 2024 23:36 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Fund Card component #1718

Implement Fund Card component #1718

rustam-cb commented Dec 11, 2024 •

edited

Loading

vercel bot commented Dec 11, 2024 •

edited

Loading

dschlabach Dec 20, 2024

dschlabach Dec 20, 2024

dschlabach Dec 20, 2024

dschlabach Dec 20, 2024

dschlabach Dec 20, 2024

rustam-cb Dec 20, 2024

cpcramer Dec 20, 2024

dschlabach Dec 20, 2024

rustam-cb Dec 20, 2024

cpcramer Dec 20, 2024

dschlabach Dec 20, 2024

dschlabach Dec 20, 2024

rustam-cb Dec 20, 2024

cpcramer Dec 20, 2024

dschlabach Dec 20, 2024

dschlabach Dec 20, 2024

cpcramer Dec 20, 2024

cpcramer commented Dec 20, 2024

Implement Fund Card component #1718

Are you sure you want to change the base?

Implement Fund Card component #1718

Conversation

rustam-cb commented Dec 11, 2024 • edited Loading

vercel bot commented Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpcramer commented Dec 20, 2024

rustam-cb commented Dec 11, 2024 •

edited

Loading

vercel bot commented Dec 11, 2024 •

edited

Loading