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

Hotfix - clean input after staking and updated title text #78

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

jeremy-babylonlabs
Copy link
Contributor

  • Improved staking clear input logic
  • updated staking and FP title text

@jeremy-babylonlabs jeremy-babylonlabs changed the title Hotfix - clean input after staking Hotfix - clean input after staking and updated title text Aug 15, 2024
Copy link
Contributor

@gbarkhatov gbarkhatov left a comment

Choose a reason for hiding this comment

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

@jeremy-babylonlabs Do we need to count the resets as state?

@jrwbabylonlab
Copy link
Collaborator

@jeremy-babylonlabs wouldn't this be easier with way less changes? #80

But will need ur guys help on testing this tho. it works for me

@vitsalis
Copy link
Member

I don't understand why are we counting resets

@jrwbabylonlab
Copy link
Collaborator

jrwbabylonlab commented Aug 16, 2024

I don't understand why are we counting resets

Because there are re-rending happening where the parent and child components (staking <-> stakingAmount) is affecting each other with their states and props being passed around. for example, setStakingAmountSat is changing the parent component's state which has a lot of dependencies on it, one of it is the state reset and it's being passed down into the child component.

Jeremy's solution is to a hack that try to fix another hack. It works, but u will see the counter actually incremented a lot (due to re-rendering issue in our code).

I proposed another solution #80 which will at least cut the reset dependency from setStakingAmountSat from the child component. But we are a long way to go from making the 3 components (staking, stakingFee, stakingAmount) to a more stable code. Right now, if u touch a single line of code in any of the 3 components, it affect a bunch of logic across whole staking components.

@jeremy-babylonlabs jeremy-babylonlabs force-pushed the hotfix/clean-input-after-staking branch from c93fa11 to b5db28d Compare August 16, 2024 06:07
@jeremy-babylonlabs
Copy link
Contributor Author

using a reset counter can make sure the states rerenders in the lifecycle. but agreed and going with @jrwbabylonlab's approach for now.

Copy link
Contributor

@gbarkhatov gbarkhatov left a comment

Choose a reason for hiding this comment

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

@jeremy-babylonlabs I suggest we do add strong to steps, so it's more emphasized for the end user (for both steps)

      <p>
        <strong>Step-1:</strong> Select a finaltiy provider
      </p> 

Other than that PR does what it should - resets the amount

@jeremy-babylonlabs jeremy-babylonlabs merged commit 61b2e05 into main Aug 16, 2024
1 check passed
@jeremy-babylonlabs jeremy-babylonlabs deleted the hotfix/clean-input-after-staking branch August 16, 2024 09:09
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.

4 participants