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

feature: Modal loading while staking #147

Merged
merged 7 commits into from
Sep 16, 2024
Merged

Conversation

gbarkhatov
Copy link
Contributor

@gbarkhatov gbarkhatov commented Sep 14, 2024

Adds loading state while signing the TX.
Problematic part - pradel/react-responsive-modal#504
Currently, react-responsive-modal cannot dynamically update the ESC button behavior. I.e. we cannot easily:

  • create a modal
  • add a support fort this modal to be closed via the ESC button (default behavior)
  • then disable the ESC key because our prop changed

So PreviewModal is not closable via ESC by default

In terms of our general approach. Since per design we don't have to differentiate between wallet is signing and wallet is broadcasting to the blockchain the whole "disabling" process will be enabled until the whole signing + broadcasting finishes. It is reset though once closed or error happens. For example, if we reject from the mobile

Video: https://youtu.be/o7giBbUmWso


Update - even though it's not inside the Design, I assume we want the loading state for unbonding and withdrawing micro modals. Logic is the same - once we are pending action from the wallet (sign unbonding / sign withdrawal + broadcasting) the loading indicator is shown and we cannot close the modal

This change also works with the Keystone

Closes #146

@gbarkhatov gbarkhatov force-pushed the feature/wallet-loading branch from 2cacee5 to 0ef462e Compare September 15, 2024 11:37
@gbarkhatov
Copy link
Contributor Author

@jrwbabylonlab change variable naming and resolved dev branch conflicts

@jeremy-babylonlabs
Copy link
Contributor

I think we can put isAwaitingWalletResponse in context providers to avoid props drilling?

another notes on UX
Screenshot 2024-09-16 at 8 32 56 AM

on mobile view we will need to scroll all the way down to the bottom to see the loading state, maybe its better if we stick the loading div on the bottom to make things more obvious.

@gbarkhatov
Copy link
Contributor Author

@jeremy-babylonlabs we will need to scroll down to hit the "stake" button anyways

Copy link
Member

@vitsalis vitsalis left a comment

Choose a reason for hiding this comment

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

small comments

src/app/components/Modals/GeneralModal.tsx Outdated Show resolved Hide resolved
src/app/components/Delegations/Delegations.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@jeremy-babylonlabs jeremy-babylonlabs left a comment

Choose a reason for hiding this comment

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

we can discuss more on the ux later, lgtm now

@gbarkhatov gbarkhatov merged commit 76c60ca into dev Sep 16, 2024
1 check passed
@gbarkhatov gbarkhatov deleted the feature/wallet-loading branch September 16, 2024 09:23
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.

Show loading screen while waiting for the tx creation and broadcast
4 participants