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.

Sign up for GitHub

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

Dont block UI if .updateWith() not called (closes #589) #591

Merged
merged 18 commits into from
Sep 8, 2017
Merged

Dont block UI if .updateWith() not called (closes #589) #591

Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e4788ec
dont block UI if .updateWith() not called (closes #589)
marcoscaceres Aug 22, 2017
19e7e70
Add [[didUpdate]] internal slot
marcoscaceres Aug 22, 2017
395bbc2
fixed typo
marcoscaceres Aug 22, 2017
f39ef2a
Simplify wording
marcoscaceres Aug 22, 2017
dd079a4
editorial: fix up updateWith() examples
marcoscaceres Aug 23, 2017
1da6e34
prettier code
marcoscaceres Aug 23, 2017
f5cef17
Make example a little neater
domenic Aug 24, 2017
c1141d1
fix: create and dispatch event correctly
marcoscaceres Aug 25, 2017
12efe20
editorial: add .updateWith() good/bad example
marcoscaceres Aug 25, 2017
eb4bc06
Simplify examples a little bit more
marcoscaceres Aug 25, 2017
fe39adc
make punctuation consistent
marcoscaceres Aug 25, 2017
a997192
fixup up wording
marcoscaceres Aug 25, 2017
660b5c7
fix whitespace in example
marcoscaceres Aug 25, 2017
3b7f6f5
Address second round of feedback from @domenic
marcoscaceres Aug 28, 2017
94dd1e4
fix markup
marcoscaceres Aug 31, 2017
8c50403
fix markup
marcoscaceres Aug 31, 2017
06934db
Keep WaitForUpdate set to true as guard
marcoscaceres Sep 1, 2017
0e8e780
Remove updatedUI from example
marcoscaceres Sep 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 86 additions & 35 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ <h3>
}
} catch (err) {
// Something went wrong...
response.complete("unknown");
await response.complete("fail");
}
}
doPaymentRequest();
Expand All @@ -419,18 +419,11 @@ <h3>
</p>
<pre class="example" title="Registering event handlers">
const request = new PaymentRequest(methodData, details, options);
request.onshippingaddresschange = async ev =&gt; {
// Can we ship here?
try {
const json = request.shippingAddress.toJSON();
const { shippingOptions, total } = await calculateShipping(json);
const newDetails = {...details, ...{ shippingOptions, total }};
ev.updateWith(newDetails);
} catch (err) {
ev.updateWith({ error: `Sorry! we can't ship to your address.` });
}
// Async update to details
request.onshippingaddresschange = ev =&gt; {
ev.updateWith(checkShipping(request));
};
// update the total
// Sync update to the total
request.onshippingoptionchange = ev =&gt; {
const shippingOption = shippingOptions.find(
option =&gt; option.id === request.id
Expand All @@ -442,6 +435,18 @@ <h3>
};
ev.updateWith({ ...details, total: newTotal });
};
async function checkShipping(request) {
try {
const json = request.shippingAddress.toJSON();

await ensureCanShipTo(json);
const { shippingOptions, total } = await calculateShipping(json);

return { ...details, shippingOptions, total };
} catch (err) {
return { ...details, error: `Sorry! we can't ship to your address.` };
}
}
</pre>
</section>
<section data-link-for="PaymentResponse">
Expand Down Expand Up @@ -2387,12 +2392,6 @@ <h2>
The <a>PaymentRequestUpdateEvent</a> enables developers to update the
details of the payment request in response to a user interaction.
</p>
<p>
If a developer wants to update the payment request, then they need to
call <a>updateWith()</a> and provide a <a>PaymentDetailsUpdate</a>
dictionary, or a promise for one, containing changed values that the
<a>user agent</a> SHOULD present to the user.
</p>
<p>
The <a>PaymentRequestUpdateEvent</a> constructor MUST set the
internal slot <a>[[\waitForUpdate]]</a> to false.
Expand All @@ -2401,13 +2400,59 @@ <h2>
<h2>
<dfn data-lt="updateWith(detailsPromise)">updateWith()</dfn> method
</h2>
<p class="note">
If a developer wants to update the payment request, then they need
to call <a>updateWith()</a> and provide a
<a>PaymentDetailsUpdate</a> dictionary, or a promise for one,
containing changed values that the <a>user agent</a> presents to
the user.
</p>
<aside class="note">
<p>
If a developer wants to update the payment request, then they
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm just noticing that this repeats text from slightly above. Let's delete the above text, as it feels pretty out of place anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

deleted the one above.

need to call <a>updateWith()</a> and provide a
<a>PaymentDetailsUpdate</a> dictionary, or a promise for one,
containing changed values that the <a>user agent</a> presents to
the user.
</p>
<p>
To prevent the user interface from blocking (and to reflect
changes made by the end-user through the UI), developers need to
immediately call <a>updateWith()</a>.
</p>
<pre class="example" title="how to use `updateWith()` correctly.">
// ❌ Bad - this won't work!
request.onshippingaddresschange = async ev =&gt; {
// await goes to next tick, and updateWith() was not
// called before that, so [[updatedUI]] gets set to true.
const details = await getNewDetails(oldDetails);
// 💥 Too late, throws "InvalidStateError".
ev.updateWith(details);
};

// ✅ Good - UI will wait.
request.onshippingaddresschange = ev =&gt; {
// Calling updateWith() with a promise is ok 👍
const promiseForNewDetails = getNewDetails(oldDetails);
ev.updateWith(promiseForNewDetails);
};
</pre>
<p>
Additionally, <a>[[\waitForUpdate]]</a> prevents reuse of
<a>PaymentRequestUpdateEvent</a>.
</p>
<pre class="example" title="can only call `updateWith()` once">
// ❌ Bad - calling updateWith() doesn't work!
request.addEventListener("shippingaddresschange", ev =&gt; {
ev.updateWith(details); // this is ok.
// 💥 [[waitForUpdate]] is true, throws an "InvalidStateError".
ev.updateWith(otherDetails);
});

// ❌ Bad - this won't work either!
request.addEventListener("shippingaddresschange", async ev =&gt; {
const p = Promise.resolve({ ...details });
ev.updateWith(p);
await p;
// 💥 the update has now completed, so [[updatedUI]] gets set to
// true; this throws "InvalidStateError"
ev.updateWith({ ...newDetails });
});
</pre>
</aside>
<p>
The <a>updateWith(detailsPromise)</a> method MUST act as follows:
</p>
Expand All @@ -2422,7 +2467,8 @@ <h2>
throw</a> an "<a>InvalidStateError</a>" <a>DOMException</a>.
</li>
<li>Let <var>request</var> be the value of <var>event</var>'s
<a data-cite="DOM#dom-event-target">target</a> attribute.
<code><a data-cite="DOM#dom-event-target">target</a></code>
attribute.
</li>
<li>If <var>request</var>.<a>[[\state]]</a> is not
"<a>interactive</a>", then <a>throw</a> an
Expand All @@ -2444,11 +2490,6 @@ <h2>
required by the change. Developers MUST settle the
<var>detailsPromise</var> to indicate that the payment request is
valid again.
<p>
The <a>user agent</a> SHOULD disable any part of the user
interface that could cause another update event to be fired.
Only one update may be processed at a time.
</p>
</li>
<li>
<p>
Expand Down Expand Up @@ -2717,8 +2758,6 @@ <h2>
<li>In either case, run the following steps, after either the upon
rejection or upon fulfillment steps have concluded:
<ol>
<li>Set <var>event</var>.<a>[[\waitForUpdate]]</a> to false.
</li>
<li>Set <var>request</var>.<a>[[\updating]]</a> to false.
</li>
<li>The <a>user agent</a> SHOULD update the user interface
Expand Down Expand Up @@ -2896,9 +2935,21 @@ <h2>
further action. The <a>user agent</a> user interface SHOULD ensure
that this never occurs.
</li>
<li>Let <var>event</var> be the result of <a data-cite=
"!DOM#concept-event-create">creating an event</a> using
<a>PaymentRequestUpdateEvent</a>.
</li>
<li>Initialize <var>event</var>'s <code><a data-cite=
"!DOM#dom-event-type">type</a></code> attribute to <var>name</var>.
</li>
<li>
<a>Fire an event</a> named <var>name</var> at <var>request</var>
using <a>PaymentRequestUpdateEvent</a>.
<a data-cite="!DOM#concept-event-dispatch">Dispatch</a>
<var>event</var> at <var>request</var>.
</li>
<li data-link-for="PaymentRequestUpdateEvent">If
<var>event</var>.<a>[[\waitForUpdate]]</a> is true, disable any part
of the user interface that could cause another update event to be
fired.
</li>
</ol>
</section>
Expand Down