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: onchainkit initial commit #11

Merged
merged 14 commits into from
Jul 25, 2024
Merged

feat: onchainkit initial commit #11

merged 14 commits into from
Jul 25, 2024

Conversation

rin-st
Copy link
Member

@rin-st rin-st commented Jul 2, 2024

Adds support of OnchainKIt
Adds page with OnchainKit components

Fixes #8

Notes:
See Readme for networks support
Our react-query, viem and wagmi versions should be very close or even the same as in OnchainKit, or multiple error occurs. Not sure how to make it up to date all the time
Tailwind integrations requires to define theme variables, and some of them overwrite ours (works for other projects with tailwind)

To test

yarn cli -e scaffold-eth/create-eth-extensions:onchainkit-init

TODO:

@technophile-04
Copy link
Contributor

Ohh while testing scaffold-eth/create-eth#74

I was getting when I navigate to /onchainkit-examples :
image

And also theme behaves in different way :

Screenshot 2024-07-09 at 3 58 38 PM

@rin-st
Copy link
Member Author

rin-st commented Jul 9, 2024

I was getting when I navigate to /onchainkit-examples :

Updated version, now it should work. It's because of

Our react-query, viem and wagmi versions should be very close or even the same as in OnchainKit, or multiple error occurs.

I didn't know when that when merging package.jsons , merge-packages doesn't just take lib from second package's when versions are different but tries to find intersection first. I'll add pr to mention it later


And also theme behaves in different way :

It's because of

Tailwind integrations requires to define theme variables, and some of them overwrite ours (works for other projects with tailwind)

Removed secondary related vars and extends, so fills works fine. But not sure onchainkit will have necessary colors after it 🤷‍♂️ .


when testing don't forget to get onchainkit API key (see readme)

Thanks!

@technophile-04
Copy link
Contributor

Thanks Rinat! Also noticed that when you do yarn next:check-types there are some error related to address not being assignable to string maybe due to moduleResolution: "bundler"?

@rin-st rin-st marked this pull request as ready for review July 18, 2024 10:53
@rin-st
Copy link
Member Author

rin-st commented Jul 18, 2024

Updated scaffold-eth/create-eth#74 and this pr. Everything seems work and no errors during yarn next:check-types

@technophile-04
Copy link
Contributor

Thanks @rin-st!! Seems to work great! Will try to test / review tomorrow, Also I am not sure whats the best way to handle this but at some places our SE-2 colors are changed for example :

Screenshot 2024-07-22 at 11 30 57 PM

Notice it has changed it color to violet when you hover over the Switch Icon

@rin-st
Copy link
Member Author

rin-st commented Jul 22, 2024

Updated Example page since they changed they documentation structure and previous links were outdated already. a9770b6


Notice it has changed it color to violet when you hover over the Switch Icon.

Yes, I mentioned it in the first message.

Tailwind integrations requires to define theme variables, and some of them overwrite ours (works for other projects with tailwind)

I believe it should be fixed on their side, since they define their color names like primary/secondary/success/warning/error etc, which will conflict with every project which use the same names.

But for now, I used their colors since it's onchainkit extension 🙂 , and without it their styles will be broken

@rin-st
Copy link
Member Author

rin-st commented Jul 22, 2024

Created an issue coinbase/onchainkit#852

@rin-st
Copy link
Member Author

rin-st commented Jul 25, 2024

Also I am not sure whats the best way to handle this but at some places our SE-2 colors are changed for example :

It is fixed in onchainkit 0.26.3

Copy link
Contributor

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

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

Tysm @rin-st, this is really nice! and works great!

@technophile-04 technophile-04 merged commit a8fc61d into onchainkit Jul 25, 2024
@technophile-04 technophile-04 deleted the onchainkit-init branch July 25, 2024 15:25
@carletex
Copy link
Member

Thanks all!! <3

Let's create a PR to main adding this extension + a PR in create-eth to "publish" the curated extension.

@rin-st
Copy link
Member Author

rin-st commented Jul 25, 2024

Oh, forgot about it

Let's create a PR to main adding this extension

#15

  • a PR in create-eth to "publish" the curated extension

It's already added https://github.com/scaffold-eth/create-eth/pull/74/files#diff-7025c91537448e2ff14397d6f1455b06e2e6da125f21beeb860a474a003a65a2R16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants