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

frontend: create coin control sub-component #2872

Merged

Conversation

NicolaLS
Copy link
Contributor

After: #2870 and #2866

Create a sub-component for the coin control (toggle and utxo selection dialog) to make the send.tsx component smaller and easier to understand.

@NicolaLS NicolaLS marked this pull request as draft August 27, 2024 18:05
@NicolaLS NicolaLS force-pushed the coin-control-sub-component branch 2 times, most recently from 8fb03ee to ec4142d Compare September 2, 2024 15:56
@NicolaLS
Copy link
Contributor Author

NicolaLS commented Sep 2, 2024

(rebased)

@NicolaLS NicolaLS force-pushed the coin-control-sub-component branch 2 times, most recently from e4c3c44 to ff0dfd4 Compare September 2, 2024 15:59
@NicolaLS
Copy link
Contributor Author

NicolaLS commented Sep 2, 2024

rebased / also amended the commit message to explain the change from #2870

@NicolaLS NicolaLS marked this pull request as ready for review September 2, 2024 16:01
Copy link
Collaborator

@thisconnect thisconnect left a comment

Choose a reason for hiding this comment

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

nice thank you 👍

tested LGTM with some nits.

private toggleCoinControl = () => {
this.setState(({ activeCoinControl }) => {
if (activeCoinControl) {
this.selectedUTXOs = {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

note for myself: I was quickly confused why this is gone, but it looks like toggleCoinControl was only used to open the dialog and activeCoinControl never true.

@@ -601,7 +564,7 @@ class Send extends Component<Props, State> {
{t('send.button')}
</Button>
<BackButton
disabled={activeCoinControl || activeScanQR}
disabled={activeScanQR}
Copy link
Collaborator

Choose a reason for hiding this comment

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

ack 👍

Create a sub-component for the coin control toggle and dialog (utxo
selection) to make the send.tsx component smaller and easier to
understand.

We can remove `activeCoinControl` from the back button disable property
because a) the use can already navigate back using ESC, and b) the back
button is not clickable while there is an active overlay.
@NicolaLS
Copy link
Contributor Author

NicolaLS commented Sep 9, 2024

@thisconnect thanks! (first push is rebase, second addressed the change requests). PTAL.

Copy link
Collaborator

@thisconnect thisconnect left a comment

Choose a reason for hiding this comment

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

LGTM

@thisconnect thisconnect merged commit 9315fcc into BitBoxSwiss:master Sep 9, 2024
7 checks passed
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.

2 participants