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

Refactor/show utxos #805

Closed
wants to merge 36 commits into from

Conversation

barrytra
Copy link
Contributor

In the show Utxos file, Instead of passing a single parameter isFrozen to toggle function, passing whole utxo data is better. As utxo's other data is need in fidelity bond section.
I am not able to review the changes due to some backend issue :(

amitx13 and others added 30 commits June 2, 2024 12:10
}
},
[frozenUtxos, unFrozenUtxos, setUnFrozenUtxos, setFrozenUtxos],
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think shortening this piece of code would give the same results. Not sure though, please go through this.

@@ -160,7 +160,7 @@ const UtxoRow = memo(
type="checkbox"
checked={checked}
onChange={() => {
onToggle && onToggle(utxoIndex, isFrozen)
Copy link
Contributor

Choose a reason for hiding this comment

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

what will happen to isFrozen that we're not getting from above, how would you handle frozen utxo and non-frozen utxo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

utxo will contain all the properties of the particularly selected utxo. So to access isFrozen property we can use utxo.isFrozen. Basically we are trying to access whole utxo instead of just one of its property isFrozen.

@theborakompanioni
Copy link
Collaborator

theborakompanioni commented Aug 6, 2024

Would also be okay if utxo is the only argument in the callback (removing utxoIndex). But can be done in #802.. what do you think @barrytra ?

Addendum: The index argument does not work as you expect @barrytra

@barrytra
Copy link
Contributor Author

barrytra commented Aug 7, 2024

Would also be okay if utxo is the only argument in the callback (removing utxoIndex). But can be done in #802.. what do you think @barrytra ?

Addendum: The index argument does not work as you expect @barrytra

I thought of doing that. But utxoIndex will be used to segregate frozen and unfrozen utxos which is done in handleUtxoCheckedState function (on line 298). Or I can think of another way without using utxoIndex

@theborakompanioni
Copy link
Collaborator

This PR is superseded by #815 and can be closed, right @barrytra ?

@barrytra
Copy link
Contributor Author

This PR is superseded by #815 and can be closed, right @barrytra ?

yes it can be closed :)

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