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

[WIP] showPaymentUI() method #325

Open
wants to merge 1 commit into
base: gh-pages
Choose a base branch
from
Open

Conversation

romandev
Copy link
Member

@romandev romandev commented Sep 24, 2018

closes #299, #300

The following tasks have been completed:

  • web platform tests (link)
  • MDN Docs added (link)

Implementation commitment:

  • Safari (link to issue)
  • Chrome (link to issue)
  • Firefox (link to issue)
  • Edge (public signal)

Preview | Diff

@romandev
Copy link
Member Author

@rsolomakhin, @ianbjacobs PTAL

};

partial interface Navigator {
[SecureContext, SameObject] readonly attribute PaymentHandlerContainer paymentHandler;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If both navigator.paymentHandler and navigator.paymentHandler.data are not promises, then the browser has to initialize these objects very early in page load, which can affect page loading performance. Can we make at least one of these into a promise, so that the browser can lazy-instantiate them after the page has loaded?

Copy link
Contributor

Choose a reason for hiding this comment

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

paymentHandler.data should be a Promise, IMO. This seemed to work well with the Web Payment CG's original API design where the "payment request" could be retrieved via getPendingRequest:

const paymentRequest = await navigator.payment.getPendingRequest();
// ... handle payment request, generate acknowledgement

// acknowledge payment request handled
navigator.payment.acknowledge(acknowledgement);

https://github.com/Spec-Ops/payment-polyfill#getting-a-pending-payment-request

Here it would be:

const data = await navigator.paymentHandler.data;
// ... handle payment request, generate response

// respond to payment request
navigator.paymentHandler.respondWith(response);

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I agree with this. I'll update this soon.

@ianbjacobs
Copy link
Contributor

(I am going to defer to @rsolomakhin for the details!)

</section>
<section>
<h2>
<dfn>respondWith()</dfn> method
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does navigator.paymentHandler.respondWith(response) need its own algorithm?

Copy link
Member Author

Choose a reason for hiding this comment

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

This method simply passes the response data to SW thread. So, I thought that the method's explanation is enough.

Copy link
Member Author

@romandev romandev left a comment

Choose a reason for hiding this comment

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

To help your understanding, I have a plan to implement POC for this idea. Do you think that it's useful?

};

partial interface Navigator {
[SecureContext, SameObject] readonly attribute PaymentHandlerContainer paymentHandler;
Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I agree with this. I'll update this soon.

</section>
<section>
<h2>
<dfn>respondWith()</dfn> method
Copy link
Member Author

Choose a reason for hiding this comment

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

This method simply passes the response data to SW thread. So, I thought that the method's explanation is enough.

@rsolomakhin
Copy link
Collaborator

@romandev : I agree that it might be useful to get implementation experience with this feature. Feel free to build it behind a flag. This can guide our discussions in W3C and we can also talk to the payment handler writers about trying out this part of the API.

@webpayments-specs
Copy link

webpayments-specs commented Sep 27, 2018 via email

@@ -1192,6 +1192,7 @@ <h2>
readonly attribute DOMString instrumentKey;
readonly attribute boolean requestBillingAddress;
Promise&lt;WindowClient?&gt; openWindow(USVString url);
Promise&lt;PaymentHandlerResponse?&gt; showPaymentUI(USVString url, optional object? data = null);

This comment was marked as spam.

@@ -1192,6 +1192,7 @@ <h2>
readonly attribute DOMString instrumentKey;
readonly attribute boolean requestBillingAddress;
Promise&lt;WindowClient?&gt; openWindow(USVString url);
Promise&lt;PaymentHandlerResponse?&gt; showPaymentUI(USVString url, optional object? data = null);

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Normative what happens when users closing the window opened by PaymentRequestEvent.openWindow
6 participants