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

Bejzik Updates #1 #239

Merged
merged 24 commits into from
Dec 14, 2022
Merged

Bejzik Updates #1 #239

merged 24 commits into from
Dec 14, 2022

Conversation

b8zeek
Copy link
Contributor

@b8zeek b8zeek commented Dec 13, 2022

Summary

First in a row of the upcoming updates series in order to improve Nimi's structure, stability, readability, maintainability and scalability.

Updates

I am starting with the root component, App. Next is done:

  • removed HashRouter
  • fixed URL
  • updated routing, now we use createBrowserRouter as per documentation by RRD v6
  • updated providers setup
  • providers outsourced to a separate folder
  • updated Wagmi setup as per documentation
  • adding support for new wallets: Argent, Trust Wallet, Omni, imToken and Ledger Live
  • updated useRainbow hook
  • updated useENSMetadata hook
  • removed broken Fathom functionality which was presenting errors in console
  • created wrapper for "user logged in" routes.

🍀

@vercel
Copy link

vercel bot commented Dec 13, 2022

@bejzik8 is attempting to deploy a commit to the nimi-app Team on Vercel.

To accomplish this, @bejzik8 needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

@b8zeek b8zeek requested review from Mi-Lan and adamazad and removed request for Mi-Lan December 13, 2022 03:39
Copy link
Member

@adamazad adamazad left a comment

Choose a reason for hiding this comment

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

Everything looks better, but we still want to keep Nimi Connect in the source.

package.json Outdated Show resolved Hide resolved
src/providers/LoggedInWrapper.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@Mi-Lan Mi-Lan left a comment

Choose a reason for hiding this comment

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

Great work on the cleanup! Looks much better now left some small comments

src/hooks/useENSMetadata.ts Outdated Show resolved Hide resolved
src/hooks/useENSMetadata.ts Show resolved Hide resolved
</HashRouter>
</ThemeProvider>
</ReduxStoreProvider>
<ReduxProvider>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can remove the redux provider since we don't use it...And all the redux related stuff

Copy link
Contributor Author

@b8zeek b8zeek Dec 13, 2022

Choose a reason for hiding this comment

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

Are we safe to remove Redux, @adamazad? I haven't got to that part yet...

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes remove it

Copy link
Member

@adamazad adamazad Dec 14, 2022

Choose a reason for hiding this comment

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

Are we safe to remove Redux, @adamazad? I haven't got to that part yet...

Yes, we don't need it at the moment.

@b8zeek b8zeek requested a review from Mi-Lan December 13, 2022 16:13
@vercel
Copy link

vercel bot commented Dec 13, 2022

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

Name Status Preview Updated
nimi-io-app-development ✅ Ready (Inspect) Visit Preview Dec 13, 2022 at 5:11PM (UTC)
nimi-io-app-production ✅ Ready (Inspect) Visit Preview Dec 13, 2022 at 5:11PM (UTC)

@Mi-Lan Mi-Lan merged commit 9b893cf into development Dec 14, 2022
@adamazad adamazad deleted the bejzik-updates branch January 4, 2023 12:28
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