Skip to content

Connect Receive page to WalletQmlModel and Cleanup Layouts #464

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

johnny9
Copy link
Collaborator

@johnny9 johnny9 commented Jun 3, 2025

This PR is based on top of #462. The relevant commits start at "qml: Extract BitcoinAmountInputField from Send" so select the commits starting from there when reviewing in the "Files Changed" tab. You can shift+click the commits in the "Changes from" drop down to select a range.

This commit introduces the PaymentRequest QObject that backs the Receive form and is used to create new payment requests in the walletdb. These commits basically hook up the Receive page to the wallet to create actual payment requests and addresses.

As a part of connecting the page to the model. I also did a pass on the anchors and replaced them with Row/Column layouts.

Screenshot from 2025-06-02 23-57-07

@johnny9 johnny9 force-pushed the paymentrequests branch from 6d7a4c1 to d512d17 Compare June 3, 2025 04:05
@GBKS
Copy link
Contributor

GBKS commented Jun 5, 2025

Are you looking for UI feedback on this right now? One thing I noticed is that the QR code does not render for me like in your screenshot.

@johnny9
Copy link
Collaborator Author

johnny9 commented Jun 5, 2025

Are you looking for UI feedback on this right now? One thing I noticed is that the QR code does not render for me like in your screenshot.

Mostly just making sure things are working right now. I will be doing a UI pass with address formatting and amount formatting next and will include as much as I can to match the UI with the spec. I am interested to see what you're seeing with the QR rendering. That sounds like something is broken.

@johnny9
Copy link
Collaborator Author

johnny9 commented Jun 5, 2025

Are you looking for UI feedback on this right now? One thing I noticed is that the QR code does not render for me like in your screenshot.

Actually, UI feedback is great as well. I can manage Issues now so i will start collecting any comments about the UI as issues so that I can track them easier and reference and close them with new commits.

@GBKS
Copy link
Contributor

GBKS commented Jun 6, 2025

Here's what that looks like (after pressing Create bitcoin address). I made sure I have the qr dependency installed.

image

For other feedback, will you re-use the amount and label (Note to self) components from the send form that you're working on in #462? If so, then I don't need to give feedback on those right now.

Other than the amount input and address display, here are some notes (many probably super obvious that you already know):

  • The form misses the "Note to self" field (Message is what's being shared, Note to self stays in the app)
  • Layout-wise the form (excluding the QR) should be centered horizontally. So when you switch between send and receive, the form should stay in place.
  • In the design docs and prototype, we have a ... button in the top-right, but I don't think we 100% finalized what goes in there. So good to ignore that for now.

We also have to figure something out with the Share functionality. AI tells me that QML does not allow for direct access to the native share UIs that operating systems provide. There appear to be OS-specific workarounds, but those require bridges. Benefit of a native share UI is that the user gets presented a list of apps they have installed, contacts they frequently communicate with, etc. So we may need to approach this as progressive enhancement. So by default, we show a Copy request button that simply puts the URI in the clipboard. And eventually, primarily on mobile, we can switch it out with the native share UI. What do you make of that?

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