Skip to content

Introduce Send pages for singlesig, single input/output send #445

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

Merged
merged 1 commit into from
Mar 25, 2025

Conversation

johnny9
Copy link
Contributor

@johnny9 johnny9 commented Feb 20, 2025

These changes implement the first iteration of the Send coins flow for our Desktop Wallet. A QML model, SendRecipient, is defined to hold the values entered into the Send form which will then be used to construct the actual transaction. The resulting transaction will be held in another new QML model, WalletQmlModelTransaction, and that will be held by the WalletModel and used to render the confirmation page, SendReview.qml. This is designed to closely mirror the objects used in the current Qt SendCoinsDialog modules.

@johnny9
Copy link
Contributor Author

johnny9 commented Feb 20, 2025

Current:

Screencast.from.2025-02-20.10-18-37.mp4

Goal:

basic_send_flow_prototype.mp4

There are a few immediate things I will be working on adjusting within the context of this PR to match the proposed designs. Primarily I will focus on the layout of the pages to match as well as the transition animations.

@johnny9 johnny9 changed the title qml: Introduce Send pages for singlesig, sigle input/output send Introduce Send pages for singlesig, sigle input/output send Feb 20, 2025
Copy link
Contributor

@MarnixCroes MarnixCroes left a comment

Choose a reason for hiding this comment

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

24d1888

  • It happens many times that clicking the Review button doesn't work.
    Nothing happens, it doesn't go to SendReview page.

And then after adding or removing a zero it works. (always below balance)

  • The address is cleared after send is confirmed and navigated back to Send page, the Amount and Note should be cleared too?

}
}

ContinueButton {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe
make the button only click-able / highlighted on hover when there is a valid input?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do. I will be coming back to this as a Validation PR.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

To fix the build error:

RCC: Error in 'qml/bitcoin_qml.qrc': Cannot find file 'controls/LabeledTextInput.qml'

I suggest (UPDATED):

--- a/src/Makefile.qt.include
+++ b/src/Makefile.qt.include
@@ -353,6 +353,7 @@ QML_RES_ICONS = \
   qml/res/icons/cross.png \
   qml/res/icons/error.png \
   qml/res/icons/export.png \
+  qml/res/icons/flip-vertical.png \
   qml/res/icons/gear.png \
   qml/res/icons/gear-outline.png \
   qml/res/icons/hidden.png \
@@ -396,6 +397,7 @@ QML_RES_QML = \
   qml/controls/ContinueButton.qml \
   qml/controls/CoreText.qml \
   qml/controls/CoreTextField.qml \
+  qml/controls/LabeledTextInput.qml \
   qml/controls/ExternalLink.qml \
   qml/controls/FocusBorder.qml \
   qml/controls/Header.qml \

Why not keep files in buildsystem scripts sorted?

@hebasto
Copy link
Member

hebasto commented Mar 5, 2025

@johnny9

Want to update the PR and fix the CI?

@johnny9
Copy link
Contributor Author

johnny9 commented Mar 10, 2025

Update:

  • Fixed missing component in Makefile
  • Added missing bitcoinamount.h include in sendrecipient.h

@hebasto
Copy link
Member

hebasto commented Mar 17, 2025

Needs rebase.

@johnny9
Copy link
Contributor Author

johnny9 commented Mar 19, 2025

Update from 9a863b0 to 55b946a

  • Rebased with main
  • Updated changes using clang-format-diff.py
  • Added missing helper function in bitcoinamount.h

@johnny9
Copy link
Contributor Author

johnny9 commented Mar 19, 2025

Update from 55b946a to 727aa52

  • Fix lint issue with duplicate include in walletqmlmodel.h

@johnny9 johnny9 changed the title Introduce Send pages for singlesig, sigle input/output send Introduce Send pages for singlesig, single input/output send Mar 20, 2025
@johnny9
Copy link
Contributor Author

johnny9 commented Mar 25, 2025

24d1888

  • It happens many times that clicking the Review button doesn't work.
    Nothing happens, it doesn't go to SendReview page.

And then after adding or removing a zero it works. (always below balance)

  • The address is cleared after send is confirmed and navigated back to Send page, the Amount and Note should be cleared too?

Both of these are good to notice and will be addressed

The review button not working is due mainly to invalid inputs and are out of scope of the current basic implementation. This will be addressed as a validation PR. This will include error messaging.

The form not clearing is good to note and I make an issue to address this when I come back around to validation.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 727aa52, I have skimmed through the code and it looks OK.

@hebasto hebasto merged commit 2e7bef0 into bitcoin-core:main Mar 25, 2025
8 of 9 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.

3 participants