BuidlGuidl / burnerwallet

https://burner.buidlguidl.com
MIT License
0 stars 1 forks source link

feat: Add Intro Modal #58

Closed ChangoMan closed 4 weeks ago

ChangoMan commented 1 month ago

Description

The modal version of https://github.com/BuidlGuidl/burnerwallet/pull/57. I haven't added any spinners yet after clicking the button, still thinking about that part.

Screenshot 2024-06-01 at 11 57 45 AM Screenshot 2024-06-01 at 11 55 47 AM
vercel[bot] commented 1 month ago

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

Name Status Preview Comments Updated (UTC)
burnerwallet-nextjs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 6, 2024 9:05am
carletex commented 1 month ago

Thanks for this @ChangoMan !!

I definitely prefer this approach!! (with some tweaks) Yes, the landing page can hold more info, but this feels more natural (the transition of closing the modal doesn't feel the same as navigating to another page). If we want to display more info, we can always link to a page with some docs. (from the modal or the settings drawer). Intuitive experience > a lot of info displayed.**

We didn't have any "welcoming experience" before, so I feel this is a good first iteration. Going for a full landing page feels like too big of a jump.

**Tweaks that I'd make:


In any case, If I'm the only one who likes this option, happy to go with #58 haha. If we are divided we can ask Austin and some other people.

damianmarti commented 1 month ago

This looks good and simple!!

What if we move the landing page to another URL, like /info, for now?

I think that like Pablo commented on the Landing Page PR, this will be useful for people with no context about the Burner Wallet and it will be good content to be indexed by the search engines, so the users can find the site when looking for a wallet.

ChangoMan commented 1 month ago

Sound like we like the modal! I'll trim down the text, let me know if it looks better on your phone. I'll also mess with the animation and I can add the landing page at /info easily.

Should we wait until the wallet connect PR gets merged? This PR will alter the structure quite a bit and might mess up that PR.

damianmarti commented 1 month ago

Sound like we like the modal! I'll trim down the text, let me know if it looks better on your phone. I'll also mess with the animation and I can add the landing page at /info easily.

Should we wait until the wallet connect PR gets merged? This PR will alter the structure quite a bit and might mess up that PR.

Maybe if @Pabl0cks think the WC is working well enough to be merged, we can merge it, and add the known issues as new issues to fix them (I'm working on fixing these issues, but these are not easy ones :-( )

Pabl0cks commented 1 month ago

Nice job @ChangoMan!! I really like the modal, the slim version of it looks nice on my mobile and is pretty concise, I like it.

When we merge the extra warning when the user receives real funds, we'll have a double message about doing a pk backup, which I think is good. And there we can be more explicit on the risk on not doing the backup (losing funds).

What if we move the landing page to another URL, like /info, for now?

I think that like Pablo commented on the Landing Page PR, this will be useful for people with no context about the Burner Wallet and it will be good content to be indexed by the search engines, so the users can find the site when looking for a wallet.

Now I'm totally up for this modal too! ♥ But yeah we could have a /info or /how-works page explaining all that info.

Probably won't be enough to get any SEO traffic, unless we start getting nice backlinks to it, but it won't hurt to have it, and can help some users that want to know how their burner works.

Pabl0cks commented 1 month ago

Should we wait until the wallet connect PR gets merged? This PR will alter the structure quite a bit and might mess up that PR.

Tonight I can do the last tests and we merge it? We'll have to continue working and testing on it, and next iteration of WC we can improve the UI/UX a bit too, but since is a bit hidden feature I guess we could merge the first iteration as it is 🙏

carletex commented 1 month ago

This is looking great! Now it fits nicely on my phone.

But we can tackle these two in different PRs. This looks good for a merge!

Regarding merging #46 , up to Pablo and Damu. Per Pablo's last comment it seems that it's ready! IMO If it's hidden, doesn't need to be super polished.

Thanks all!

damianmarti commented 1 month ago

@ChangoMan #46 is merged now. There are some conflicts to be resolved now. After resolving I can do a final review.

ChangoMan commented 4 weeks ago

@damianmarti Merge conflicts should be resolved!