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

Add support for SEP-0007 payment requests #1179

Closed
wants to merge 23 commits into from

Conversation

ebma
Copy link
Member

@ebma ebma commented Dec 2, 2020

  • Add transactionRequest context (to parse and store incoming SEP-0007 requests)
  • Add <TransactionRequestHandler> that checks the uri stored in the transactionRequest context and shows dialogs according to its content
  • Add <VerifyTrustedServiceDialog> that is shown for incoming SEP-0007 requests that have an origin_domain but are not in the list of trusted services. The user can then decide to trust this domain and it will be added to the list of trusted services.
  • Add <PaymentAccountSelectionDialog> that is shown for incoming SEP-0007 pay requests. It shows details of the pay request and allows the user to select an account to use for this payment. Only accounts that hold a trustline for the specified asset will be shown in the list. A warning will be shown if the request is not signed. Selecting an account will show the payment dialog for the selected account.
  • Add preselectedParams as prop to the <PaymentForm> to be able to pre-fill the respective input fields with values of the pay request. Pre-filled input fields will be disabled so that the user cannot change the value.
  • Enable trusted services by default (Remove the feature flag)
  • Save settings to localStorage in web build (makes it easier to test trusted services but is useful in general)

Closes #323.

Preview of the <PaymentAccountSelectionDialog> if no origin_domain and no signature are provided:
Screenshot 2020-12-02 at 15 22 31

Flow when origin_domain of request is already a trusted service:
pay-demo1

Flow when origin_domain of request is not a trusted service:
trusted-service-demo

@ebma ebma added the enhancement New feature or request label Dec 2, 2020
@andywer
Copy link
Contributor

andywer commented Dec 3, 2020

Great job, @ebma! 👍

I proposed a wording change and we need to re-arrange the headlines in the view a little bit – looks somewhat unbalanced right now. Otherwise really cool.

We should also think about whether it makes sense to show the "trust this origin" dialog before the user has had a chance to see the actual request. Happy to fix this as a separate issue, though. Don't think it should be blocking.

onClose: () => void
}

function PaymentAccountSelectionDialog(props: PaymentAccountSelectionDialogProps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing that we can either refactor right now or we need to do it as soon as we start working on tx type requests:

It should probably rather be called TransactionRequestReviewDialog or similar and we should pass that SEP-7 URI content rendered as a grid as children prop, so we can re-use this component for the tx type URIs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I suggest we postpone it until we do the tx type requests because it might be easier to restructure once we know how we handle/show tx requests.

@andywer
Copy link
Contributor

andywer commented Dec 3, 2020

Pushed one commit with a few UI adjustments. Btw, why did you prevent the max-width to be set? I thought it looked rather strange on a desktop screen. Maybe you had your reasons, but I changed that as part of my commit.

@ebma
Copy link
Member Author

ebma commented Dec 7, 2020

The noMaxWidth stems from the other dialogs that are used in the account settings. I did not pay attention on the max-width property when reusing code from those dialogs (for the sake of consistency) so it's a good thing that you noticed and changed it. 👍
I'd also propose to tackle the issue with showing the 'trust this origin' dialog later.

@ebma ebma force-pushed the feature/323-support-sep7-payment-requests branch from 4e5e279 to e133f24 Compare January 20, 2021 14:50
@ebma
Copy link
Member Author

ebma commented Jan 20, 2021

I changed it so that the <PaymentForm> is now shown inline instead of the user being redirected to the payment dialog.

The payment form will not render without an account. That's why the first account in the account selection list is preselected by default.
Screenshot 2021-01-20 at 16 08 22

If no accounts are available for the network specified in the tx-request another dialog is shown instead:
Screenshot 2021-01-20 at 15 49 54

@ebma ebma force-pushed the feature/323-support-sep7-payment-requests branch from e133f24 to abc70ab Compare January 20, 2021 15:10
@ebma
Copy link
Member Author

ebma commented Jan 25, 2021

Superseded by #1202.

@ebma ebma closed this Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support SEP-0007 payment requests
2 participants