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: allow node selection after connection error #217

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Conversation

samsiegart
Copy link
Collaborator

@samsiegart samsiegart commented Dec 27, 2023

refs Agoric/agoric-sdk#8505
refs #211

This adds a "connection settings" button to the chain connection error dialog which allows you to select API and RPC nodes. It saves your selections to local storage, which take priority over whichever nodes are loaded from the network config.

It doesn't provide an entrypoint to the node selector outside of the error dialog, that can be added in a future PR. Also, if you change networks (dev/testnet only, as mainnet doesn't have the network dropdown), it just deletes the saved nodes, instead of saving a map of saved nodes for every network.

Screenshots

image

image

Recording

(Animations look choppy in the gif due to compression, but you can try it in the live demo in the comments yourself)
fixed-provision-bug

Copy link

github-actions bot commented Dec 27, 2023

Network:
Commit: bb477dd
Ref: refs/heads/main
IPFS v1 hash: bafybeiad5ewmnxumo3k5mlrrb6pa7mjcollovvs4t27crhvmzzlsiggrti
CF - DWeb - 4EVERLAND

setSavedApi(api);
setSavedRpc(rpc);
setIsOpen(false);
window.location.reload();
Copy link
Member

Choose a reason for hiding this comment

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

Is a full page reload necessary here? I guess it doesn't matter much, as a majority of app state is dependent on the rpc + api endpoints.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea it's just taking the easy way out. Otherwise we'd have to recreate the chainStorageWatcher and walletConnection, reset the state, and make sure we clean up all casting followers etc.

Copy link
Member

@0xpatrickdev 0xpatrickdev left a comment

Choose a reason for hiding this comment

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

It doesn't provide an entrypoint to the node selector outside of the error dialog, that can be added in a future PR.

It would be nice to see this in the MVP, maybe as the last option in Network Dropdown and/or a url query param (?showConnectionSettings=true) for advanced users. But agree this can come later

@samsiegart samsiegart merged commit bb477dd into main Jan 3, 2024
3 checks passed
@samsiegart samsiegart deleted the node-selector branch January 3, 2024 08:20
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.

2 participants