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

Migrate elements to usage via CDN #317

Merged
merged 15 commits into from
Sep 16, 2024

Conversation

siddharthborderwala
Copy link
Contributor

  • Fixes the elements crash
  • Migrate to using elements via UMD - read about differences and benefits here
  • Use elements UMD types package for TS support

@siddharthborderwala siddharthborderwala changed the title Migrate elements to UMD version Migrate elements to usage via CDN Aug 17, 2024
@siddharthborderwala siddharthborderwala marked this pull request as draft August 17, 2024 06:01
@siddharthborderwala siddharthborderwala marked this pull request as ready for review August 17, 2024 06:15
@siddharthborderwala
Copy link
Contributor Author

@baktun14 I have migrated elements to usage via CDN instead of using as an NPM package. It fixes the crash that's happening due to a Nextjs SWC minification issue.

Some of the other benefits of the UMD build are

  • Bundle size of your dApp decreases
  • No build/bundling for elements needed

@baktun14
Copy link
Contributor

baktun14 commented Aug 21, 2024

The leap wallet styling overrides the app styling

Console:
image
Console with leap:
image

@siddharthborderwala
Copy link
Contributor Author

siddharthborderwala commented Aug 22, 2024

@baktun14 thank you for making a note of this. It's a bug with the stylesheet for elements version 1.4.1. We're releasing elements version 1.4.2 today/tomorrow which will have the fix for this. Will update then.

@siddharthborderwala
Copy link
Contributor Author

Hi @baktun14 I have fixed the issue with the stylesheets. It won't affect the rest of the app.

@baktun14
Copy link
Contributor

Hi @baktun14 I have fixed the issue with the stylesheets. It won't affect the rest of the app.

The styling issues seem to have been fixed. 👍

Comment on lines 28 to 29
<link rel="stylesheet" href="https://unpkg.com/@leapwallet/[email protected]/dist/style.css" />
<script defer src="https://unpkg.com/@leapwallet/[email protected]/dist/umd/main.js" />
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a big fan of importing this globally when we only need it in the get started page.

Copy link
Contributor

Choose a reason for hiding this comment

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

@siddharthborderwala Would it possible to import these dynamically?

Copy link
Contributor

Choose a reason for hiding this comment

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

Any update on this? @siddharthborderwala

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@baktun14 will update this and push changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@baktun14 I've made the required changes. Now the elements related CSS and JS will be loaded only on the /get-started page.

@baktun14
Copy link
Contributor

@siddharthborderwala Hey, I rebased and fixed some issues, but for some reason the @leapwallet/elements package is still a dependency of the types packages and it messed up our CI...

https://github.com/akash-network/console/actions/runs/10888482586/job/30213077526?pr=317

@baktun14 baktun14 merged commit ab68c65 into akash-network:main Sep 16, 2024
5 checks passed
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