getAlby / nostr-wallet-connect-next

Nostr Wallet Connect (NIP-47) application to allow apps to connect to your node
https://nwc.getalby.com
Apache License 2.0
7 stars 3 forks source link

feat: loading state for shutdown button #389

Closed pavanjoshi914 closed 1 month ago

pavanjoshi914 commented 1 month ago

image

I avoided adding new page or new text elements for shutdown. cause for http mode its very fast. we we still loading states in few situations as mentioned in issue

Fixes getAlby/hub#83

im-adithya commented 1 month ago

The spinner is shown along with the Power Button for me, I think it's a problem with LoadingButton component itself https://github.com/getAlby/nostr-wallet-connect-next/blob/a2b07a539a063dfc97b8ecbcf23208165678fc38/frontend/src/components/ui/loading-button.tsx#L57-L62

Shouldn't it be loading ? <Loader2> : children

Screenshot

Screenshot 2024-06-06 at 10 08 30 AM

Also, I think it's hard to catch it, it's barely visible. I think we should show something in the Popup itself, but Shadcn popups close automatically once you click a button. Best we have a separate screen for this imo.

pavanjoshi914 commented 1 month ago

its fine cause we have many buttons that shows loader icon along with text in the button that is why component is structured like this. but yeah here it's not that good. i updated code for shutdown button itself to show either loader or power icon according to state

rolznz commented 1 month ago

Looks good!

By the way, it's easy to reproduce the slow shutdown by waiting on the liquidity page until a sync starts (from the go logs) and then go to settings and shutdown.

The user can still navigate when the node is shutting down which I think is not a good thing. Maybe we can further improve it in a follow-up PR, but I guess it's not so important right now.