Re: [w3c/payment-request] wip: allow show() to take optional promise (#668)

domenic requested changes on this pull request.

A few issues but overall seems to work!

> @@ -932,11 +932,38 @@ <h2>
           <a>DOMException</a>, and set the <a>user agent</a>'s <a>payment
           request is showing</a> boolean to false.
           </li>
+          <li>Otherwise, present a user interface that will allow the user to

The intro text which says "The show() method must act as follows" should be updated to say "the show(detailsPromise) method". Cf. updateWith()'s definition elsewhere.

> @@ -932,11 +932,38 @@ <h2>
           <a>DOMException</a>, and set the <a>user agent</a>'s <a>payment
           request is showing</a> boolean to false.
           </li>
+          <li>Otherwise, present a user interface that will allow the user to
+          interact with the <var>handlers</var>.
+          </li>
+          <li>If <var>detailsPromise</var> was passed, then:
+            <ol>
+              <li>Set <var>request</var>.<a>[[\updating]]</a> to true.
+              </li>
+              <li>
+                <a>In parallel</a>, disable the user interface user interface

This should not be done in parallel, as you can't update the UI from off the main thread (i.e. in parallel with the main thread). I guess it's my bad for implying the updating should be done in parallel; it shouldn't. (See later comment.)

>            <li>
             <p>
-              Otherwise, present a user interface to allow the user to interact
-              with the <var>handlers</var>. The user agent SHOULD prioritize
-              the preference of the user when presenting payment methods.
+              Present <var>handlers</var> to the user. The user agent SHOULD

I'm confused. Above you displayed a user interface that allowed the user to interact with _handlers_. Now you are presenting _handlers_ to the user. What is the difference between these two? Why do both?

I think what I would do is add a step in the "If _detailsPromise_ was passed, then:" saying "Re-enable the user interface that allows the users to interact with _handlers_." And, I would move all the paragraphs in this last step up to the first "present a user interface" step. Am I on the right track?

> -          </ol>
-          <p>
-            If any of the above steps say to <dfn>abort the update</dfn> with
-            an exception <var>exception</var>, then:
-          </p>
-          <ol>
-            <li>Abort the current user interaction and close down any remaining
-            user interface.
-            </li>
-            <li>Set <var>request</var>.<a>[[\state]]</a> to "<a>closed</a>".
-            </li>
-            <li>Reject the promise <var>request</var>.<a>[[\acceptPromise]]</a>
-            with <var>exception</var>.
-            </li>
-            <li>Abort the algorithm.
+            <li>Return from the method and, <a>in parallel</a>, run the

This shouldn't be done in parallel, it turns out; the algorithm just waits on promises, which should be done on the main thread (like it is done in JS).

-- 
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-request/pull/668#pullrequestreview-90262421