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

Add support for ShippingOption.selected. #179

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
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
30 changes: 30 additions & 0 deletions specs/paymentrequest.html
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,13 @@ <h2>PaymentRequest constructor</h2>
length of 1, then set <a><code>shippingOption</code></a> to the <code>id</code> of
the only <a><code>ShippingOption</code></a> in the sequence.
</li>
<li>
If <code>details</code> contains a <code>shippingOptions</code> sequence with a
length greater than 1, and if any <a><code>ShippingOption</code></a> in the sequence
has the <code>selected</code> field set to <code>true</code>, then set
<a><code>shippingOption</code></a> to the <code>id</code> of the last <a><code>ShippingOption</code></a>
in the sequence with <code>selected</code> set to <code>true</code>.
</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be hesitant to make this a conformance issue, though. It may be perfectly reasonable for the user agent to use locally-configured preferences to override the "selected" flag; e.g., "remember my choice from last time" or "always default to the cheapest shipping option." So I would far prefer to see this specified as a hint rather than a normative behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A user-agent could still do that just not without firing the change event.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, that's fine. I just want such behavior to be technically conformant. The conformance section of this document denotes everything except a named list of items as normative, and this step -- which specifies that the shippingOption is set a certain way -- doesn't fall into any of the named non-normative categories. That makes performing such action mandatory for conformance purposes, and I think that's too strict.

<li>Set the value <em>request</em>@[[\updating]] to <em>false</em>.</li>
<li>Return <em>request</em>.</li>
</ol>
Expand Down Expand Up @@ -682,9 +689,21 @@ <h2>PaymentDetails dictionary</h2>
<p>If the sequence only contains one item, then this is the shipping option that
will be used and <a><code>shippingOption</code></a> will be set to the <code>id</code>
of this option without running the <a>shipping option changed algorithm</a>.</p>
<p>If an item in the sequence has the <code>selected</code> field set to <code>true</code>,
then this is the shipping option that will be used by default and <a><code>shippingOption</code></a>
will be set to the <code>id</code> of this option without running the <a>shipping option changed
algorithm</a>. Authors SHOULD NOT set <code>selected</code> to <code>true</code> on more than
one item. If more than one item in the sequence has <code>selected</code> set to <code>true</code>,
then <a>user agents</a> MUST select the last one in the sequence.</p>
Copy link
Member

Choose a reason for hiding this comment

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

There are two ways to read this line, so it's slightly unclear if the UAs should select the last one in the sequence of shipping options, or the last one in the sequence /with the selected flag set to true/. I think you mean the latter.

<p>The <code>shippingOptions</code> field is only used if the <a><code>PaymentRequest</code></a> was
constructed with <a><code>PaymentOptions</code></a> <code>requestShipping</code>
set to <code>true</code>.</p>
<p class="note">
If the sequence contains only one item or if the sequence has an item with the <code>selected</code>
field set to <code>true</code>, then authors SHOULD ensure that the <code>total</code> field includes
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this mean that developers will have to write different code paths for total calculation based on whether or not they have a default shipping option for the customer? Wouldn't it be easier if the browser just fired the "shippingoptionchange" event when the UI appears if the developer sets the selected field? The general question here is what would result in an easier implementation for the developer?

the cost of the shipping option. This is because no <a><code>shippingoptionchange</code></a> event
will be fired for this option unless the user selects an alternative option first.
</p>
</dd>
</dl>
</section>
Expand Down Expand Up @@ -814,6 +833,7 @@ <h2>ShippingOption interface</h2>
required string id;
required string label;
required CurrencyAmount amount;
boolean selected = false;
};
</pre>
<p>
Expand All @@ -835,6 +855,9 @@ <h2>ShippingOption interface</h2>
<dd>
A <a><code>CurrencyAmount</code></a> containing the monetary amount for the item.
</dd>
<dt><code>selected</code></dt>
<dd>This is set to <code>true</code> to indicate that this is the default selected <a>ShippingOption</a>
in a sequence. <a>User agents</a> SHOULD display this option by default in the user interface.</dd>
</dl>
</section>

Expand Down Expand Up @@ -1033,6 +1056,13 @@ <h2>PaymentRequestUpdateEvent</h2>
length of 1, then set <em>newOption</em> to the <code>id</code> of the only
<a><code>ShippingOption</code></a> in the sequence.
</li>
<li>
If <code>details</code> contains a <code>shippingOptions</code> sequence with a
length greater than 1, and if any <a><code>ShippingOption</code></a> in the sequence
has the <code>selected</code> field set to <code>true</code>, then set
<em>newOption</em> to the <code>id</code> of the last <a><code>ShippingOption</code></a>
in the sequence with <code>selected</code> set to <code>true</code>.
</li>
<li>
Set the value of <a><code>shippingOption</code></a> on <em>target</em> to
<em>newOption</em>.
Expand Down