Re: [w3c/payment-handler] Define requestPermission() behavior. (#180)

romandev commented on this pull request.

@rsolomakhin, @dlongley 

Thank you for review 👍 

I addressed your comments.

>        interface PaymentManager {
         [SameObject] readonly attribute PaymentInstruments instruments;
         [SameObject] readonly attribute PaymentWallets wallets;
-                              Promise<boolean>   requestPermission();
+        [Exposed=Window] static Promise<PermisionState> requestPermission();

Done.

FYI, I found a history of Notification.requestPermission().
https://lists.w3.org/Archives/Public/public-whatwg-archive/2014Jul/0178.html

> @@ -697,6 +716,14 @@
             When called, this method executes the following steps:
           </p>
           <ol>
+            <li>Let <var>permission</var> be the result of running
+              <a data-cite="!permissions#dfn-retrieve-the-permission-state">retrieve
+              the permission state algorithm</a> of the permission associated
+              with <a>payment handler</a>.
+            </li>
+            <li>If <var>permission</var> is not "granted", then return a
+            <a>Promise</a> rejected with a <a>SecurityError</a>.

Done.

> @@ -953,6 +980,14 @@
             When called, this method executes the following steps:
           </p>
           <ol>
+            <li>Let <var>permission</var> be the result of running
+              <a data-cite="!permissions#dfn-retrieve-the-permission-state">retrieve
+              the permission state algorithm</a> of the permission associated
+              with <a>payment handler</a>.
+            </li>
+            <li>If <var>permission</var> is not "granted", then return a
+            <a>Promise</a> rejected with a <a>SecurityError</a>.

Done.

>            </p>
+          <ol>
+            <li>Let <var>p</var> be a new promise.
+            </li>
+            <li>Run the following steps in parallel:
+              <ol>
+                <li>Let <var>permission</var> be the result of running
+                <a data-cite="!permissions#dfn-retrieve-the-permission-state">
+                  retrieve the permission state algorithm</a> of the permission
+                  associated with <a>payment handler</a>.

Done.

>            </p>
+          <ol>
+            <li>Let <var>p</var> be a new promise.
+            </li>
+            <li>Run the following steps in parallel:
+              <ol>
+                <li>Let <var>permission</var> be the result of running
+                <a data-cite="!permissions#dfn-retrieve-the-permission-state">
+                  retrieve the permission state algorithm</a> of the permission
+                  associated with <a>payment handler</a>.
+                </li>
+                <li>If <var>permission</var> is "prompt", ask the user whether
+                allowing adding new payment instruments for the <a data-cite=
+                "!HTML#current-settings-object">current settings object</a>'s
+                origin is acceptable. If it is, set <var>permission</var> to
+                "granted", and "denied" otherwise.

In the implementation, I defined PaymentHandlerStatus as 'default' instead of 'prompt'.
Because the Notification spec[1] is doing like that.
But I awared that we can use PermissionState in Permission API spec[2] instead of our own definition.

[1] https://notifications.spec.whatwg.org/#enumdef-notificationpermission
[2] https://www.w3.org/TR/permissions/#idl-def-PermissionState

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/payment-handler/pull/180#pullrequestreview-45487648

Received on Wednesday, 21 June 2017 16:55:42 UTC