Feat: adds PaymentItemType enum and PaymentItem.type #666

marcoscaceres · 2018-01-17T03:02:17Z

Tests: Tests for PaymentItem.type member web-platform-tests/wpt#9099
dev docs:

Adds optional "type" member PaymentItem and introduces PaymentItemType enum.

I'm thinking we should introduce "discount" or similar when we add coupon support.

@domenic, I'm wondering if there would be any advantage to having a default value for the type member (e.g., "other")? Thought I'd ask just in case.

Preview | Diff

mnoorenberghe

From an implementation perspective, this will nicely solve our use case. 👍

mnoorenberghe · 2018-01-17T03:20:57Z

index.html

+          <dfn>type</dfn> member
+        </dt>
+        <dd>
+          A <a>PaymentItemType</a> enum value. A the value of <a>type</a> to


I think you have a mistake at "A the value of"

adrianhopebailie · 2018-01-17T09:56:45Z

index.html

+      </h2>
+      <pre class="idl">
+        enum PaymentItemType {
+          "tax"


Instead of other, could we have lineItem as the default and include in the enum?

I'm not sure we need that default at all.

adrianhopebailie · 2018-01-17T09:58:06Z

index.html

+      </pre>
+      <p>
+        The <a>PaymentItemType</a> serves to categorize a <a>PaymentItem</a>
+        into particular types.


Should we have some indication of why this feature exists? i.e. Browsers MAY use this to influence their UI

+1 - it's always good to say why. :-) I'm not sure if we have user experience data to back it up, but grouping line items into categories (say, national and local taxes) could help users understand what they're being charged.

@adrianhopebailie, @stpeter, above, I state that the user agent MAY use this to, "for example, visually grouping types together or otherwise distinguishing them from other types (or from items that have no associated type)."

Is that sufficient?

Yes, that seems good enough for now.

marcoscaceres · 2018-01-19T03:52:30Z

PR for tests sent: web-platform-tests/wpt#9099

marcoscaceres · 2018-01-19T05:22:36Z

To make this work with #669, will probably need to make .type nullable.

domenic

Nit on IDL style, where we shouldn't use nullable dictionary members generally, instead just allowing them to be omitted.

Happy to leave naming concerns in others' hands, although I did weigh in a bit myself.

domenic · 2018-01-19T22:38:19Z

index.html

@@ -1853,6 +1854,7 @@ <h2>
          required DOMString label;
          required PaymentCurrencyAmount amount;
          boolean pending = false;
+          PaymentItemType? type;


No need for this to be null. Just have them not supply it if they don't want to set it (or, set it to undefined).

domenic · 2018-01-19T22:38:34Z

index.html

+          <dfn>type</dfn> member
+        </dt>
+        <dd data-tests="PaymentItem/type_member.https.html">
+          A <a>PaymentItemType</a> enum value or null. A user agent MAY use the


So here I would say "If supplied, a PaymentItemType enum value"

domenic · 2018-01-19T22:40:03Z

index.html

@@ -1885,6 +1887,40 @@ <h2>
          shipping option. <a>User agents</a> MAY indicate pending fields in
          the user interface for the payment request.
        </dd>
+        <dt>
+          <dfn>type</dfn> member


Since there is no type for the "normal" payment items, I wonder if this should be named "specialType" or something? It's probably fine as just type though.

I'd like to just keep it as type.

domenic · 2018-01-19T22:40:56Z

index.html

+        <dd>
+          Indicates that the corresponding <a>PaymentItem</a> represents a form
+          of tax (e.g., sales tax, goods and services tax, value added tax,
+          etc.).


Would it be useful to say something like "(The developer can use the label member to be more specific.)"? I guess in that case I'd move the examples outside of the parentheses so there aren't too many parentheses.

Yeah, but I think the developer bit would probably be better in type member above, so will add it there. I'll remove the parens.

domenic · 2018-01-19T22:44:45Z

@domenic, I'm wondering if there would be any advantage to having a default value for the type member (e.g., "other")? Thought I'd ask just in case.

Since these are never reflected back to the web developer, I don't see any concrete advantage to a default. I guess it would allow people to be explicit and write type: isTaxable(x) ? "tax" : "other" instead of type: isTaxable(x) ? "tax" : undefined. But even them coming up with the right name seems hard, so I wouldn't bother.

marcoscaceres · 2018-01-22T00:56:34Z

@zkoch, @adrianba, @aestes, need your ok on this as implementers.

We are planning to support this in Gecko.

zkoch · 2018-01-30T21:11:52Z

@rsolomakhin thoughts? seems good to me and easy to support.

rsolomakhin · 2018-01-30T21:13:36Z

@zkoch: 👍 looks good to me as well.

adrianba · 2018-02-15T23:37:49Z

We previously had a type of display items and decided to remove it from v1 because it was hard to get agreement on exactly how it should work. I don't support rushing this in at this point. It hasn't been a feature we've seen high priority requests for.

marcoscaceres · 2018-02-16T05:13:32Z

We (Firefox) are willing to implement it, so maybe we can add it as a "Feature at Risk" to see if any merchants will use it.

* closes #163

stpeter · 2018-02-21T23:39:47Z

+1 @marcoscaceres LGTM

marcoscaceres · 2018-02-21T23:52:31Z

@ianbjacobs, could you please send a request to republish the spec.

ianbjacobs · 2018-02-22T14:50:59Z

@marcoscaceres, request sent.
Ian

marcoscaceres added proposal - needs discussion needs tests labels Jan 17, 2018

marcoscaceres mentioned this pull request Jan 17, 2018

Add type to PaymentItem #163

Closed

mnoorenberghe reviewed Jan 17, 2018

View reviewed changes

mnoorenberghe approved these changes Jan 17, 2018

View reviewed changes

adrianhopebailie reviewed Jan 17, 2018

View reviewed changes

marcoscaceres mentioned this pull request Jan 19, 2018

Tests for PaymentItem.type member web-platform-tests/wpt#9099

Merged

marcoscaceres removed the needs tests label Jan 19, 2018

marcoscaceres force-pushed the displayitem_type branch from 6a3c1ac to 82c630b Compare January 19, 2018 05:28

domenic requested changes Jan 19, 2018

View reviewed changes

domenic approved these changes Jan 22, 2018

View reviewed changes

marcoscaceres removed the proposal - needs discussion label Jan 22, 2018

marcoscaceres requested review from zkoch, aestes and adrianba January 29, 2018 01:16

marcoscaceres mentioned this pull request Jan 29, 2018

need primary editor WICG/cq-usecases#49

Closed

marcoscaceres mentioned this pull request Feb 16, 2018

Temperature check: preferred editing workflows? WICG/cq-usecases#66

Closed

marcoscaceres added 5 commits February 21, 2018 10:58

Feat: adds PaymentItemType enum and PaymentItem.type

4b98e53

* closes #163

Make PaymentItem.type nullable

d7bb87f

tests: link to PaymentItem/type_member.https.html

d0a11f2

Revert nullable type

7876653

review feedback

2c16356

Mark as at risk

a5dbf13

marcoscaceres force-pushed the displayitem_type branch from f60430b to a5dbf13 Compare February 21, 2018 23:30

stpeter approved these changes Feb 21, 2018

View reviewed changes

marcoscaceres merged commit d891049 into gh-pages Feb 21, 2018

marcoscaceres deleted the displayitem_type branch February 21, 2018 23:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: adds PaymentItemType enum and PaymentItem.type #666

Feat: adds PaymentItemType enum and PaymentItem.type #666

marcoscaceres commented Jan 17, 2018 •

edited by pr-preview bot

mnoorenberghe left a comment

mnoorenberghe Jan 17, 2018

adrianhopebailie Jan 17, 2018

marcoscaceres Jan 18, 2018

adrianhopebailie Jan 17, 2018

stpeter Jan 17, 2018

marcoscaceres Jan 18, 2018

stpeter Jan 18, 2018

marcoscaceres commented Jan 19, 2018

marcoscaceres commented Jan 19, 2018

domenic left a comment

domenic Jan 19, 2018

domenic Jan 19, 2018

domenic Jan 19, 2018

marcoscaceres Jan 22, 2018

domenic Jan 19, 2018

marcoscaceres Jan 22, 2018

domenic commented Jan 19, 2018

marcoscaceres commented Jan 22, 2018

zkoch commented Jan 30, 2018

rsolomakhin commented Jan 30, 2018

adrianba commented Feb 15, 2018

marcoscaceres commented Feb 16, 2018

stpeter commented Feb 21, 2018

marcoscaceres commented Feb 21, 2018

ianbjacobs commented Feb 22, 2018

Feat: adds PaymentItemType enum and PaymentItem.type #666

Feat: adds PaymentItemType enum and PaymentItem.type #666

Conversation

marcoscaceres commented Jan 17, 2018 • edited by pr-preview bot

mnoorenberghe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoscaceres commented Jan 19, 2018

marcoscaceres commented Jan 19, 2018

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Jan 19, 2018

marcoscaceres commented Jan 22, 2018

zkoch commented Jan 30, 2018

rsolomakhin commented Jan 30, 2018

adrianba commented Feb 15, 2018

marcoscaceres commented Feb 16, 2018

stpeter commented Feb 21, 2018

marcoscaceres commented Feb 21, 2018

ianbjacobs commented Feb 22, 2018

marcoscaceres commented Jan 17, 2018 •

edited by pr-preview bot