-
Notifications
You must be signed in to change notification settings - Fork 5
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
Feature: add custom network #274
Conversation
bogos
commented
Nov 13, 2023
•
edited
Loading
edited
- Add custom network modal selector
- Validate selected network
…rd into feature/add-custom-network
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…rd into feature/add-custom-network
The code in the previous commit is functional but appears messy and lacks clarity due to asynchronous operations within useEffect. Despite its smelly nature, it successfully initializes the API and manages state. Consider refactoring for improved readability when possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great changes @bogos 🤝.
I will merge and leave you the commentary on what needs to be improved.
|
||
useEffect(() => { | ||
if (networkConnected) { | ||
if (!fetchApi) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 This logic is not stable, what happen if fetchApi
returns after useEffect
is executed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic works as expected. There are some minor issues to fix in the UI
} | ||
} | ||
|
||
const formData = { | ||
name: useFormInput<string>('test', [notEmpty]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here in the form data the placeholder for the name seems like an address