search

scio-labs / use-inkathon

Typesafe React hooks and utility functions that simplify the process of working with Substrate-based networks and ink! smart contracts.
https://inkathon.xyz
GNU General Public License v3.0
49 stars 15 forks source link

NightlyConnect pops up when no wallet installed #67

Closed niklasp closed 2 months ago

niklasp commented 3 months ago

When there are no wallets installed i would not expect to see a nightly connect popup per default.

There is a little bug in the code, as isWalletInstalled always returns true for nightly

https://github.com/scio-labs/use-inkathon/blob/8bb66bc462ba443b45c4b5771a27076ba5df4746/src/wallets.ts#L160

https://github.com/scio-labs/use-inkathon/blob/8bb66bc462ba443b45c4b5771a27076ba5df4746/src/provider.tsx#L185-L197

wallets will never be [] and so nightlyWallet will always be the preferredWallet that is tried to enable, also if no wallets are installed.

What do you think about tackling this in a feature-add-workaround where instead of using allSubstrateWallets add a supportedWallets value to the provider that defaults to allSubstrateWallets and working with that instead in the provider. I am asking because I had to limit the supported wallets in my dapps and it would solve the issue for all not adding nightlyWallet to the supportedWallets

niklasp commented 3 months ago

My problem is that i cannot use useInkathon the way it is now as I do not want to support NightlyConnect and see no way to workaround except changing the library. Happy to submit a PR if you agree or come up with other ideas on how to handle.

wottpal commented 2 months ago

This is fixed in @scio-labs/use-inkathon@0.9.0 which no longer automatically opens Nightly Connect on the initial/auto-connect.

Additionally, it contains your PR #68 which I've merged. Though, I would recommend only using it when there is a good reason for it (Nightly Connect is now disabled on init so this shouldn't be one). Why? IMO it leads towards developers artificially excluding users by not-enabling certain wallets. And also when new wallets get added to the eco/repo they need to manually upgrade their dApps to be compatible which I would advice against as the default.