wip: allow show() to take optional promise #668
Conversation
index.html
Outdated
| with <var>exception</var>. | ||
| </li> | ||
| <li>Abort the algorithm. | ||
| <li>Return from the method and, <a>in parallel</a>, run the |
There was a problem hiding this comment.
The stuff deleted here is just moved and given its own definition below ("update a PaymentRequest's details algorithm").
marcoscaceres
changed the title
| <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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Updated to match method signature.
index.html
Outdated
| <li>Set <var>request</var>.<a>[[\updating]]</a> to true. | ||
| </li> | ||
| <li> | ||
| <a>In parallel</a>, disable the user interface user interface |
There was a problem hiding this comment.
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.)
index.html
Outdated
| 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
You are correct, I screwed this up.
index.html
Outdated
| with <var>exception</var>. | ||
| </li> | ||
| <li>Abort the algorithm. | ||
| <li>Return from the method and, <a>in parallel</a>, run the |
There was a problem hiding this comment.
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).
|
@domenic, hopefully I understood correctly and you wanted me to drop the "in parallels", right? You are correct that the state machine should handle the signaling to the UI thread while allowing all this to run on the main thread (and just waiting for the Giving it another read... to see if it makes sense. |
|
Going to split this into two PR. One editorial one, for the update algo, and then the next for the API change. |
2 participants
Closes #645.
Changes
show()to take an optional promise that resolves with aPaymentDetailsUpdate:We can then reuse the machinery to perform an update, but splitting it out into its own algorithm
Preview | Diff