-
Notifications
You must be signed in to change notification settings - Fork 135
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 type to PaymentItem #163
Comments
I don't hate this idea. I wonder if there are already terms defined at schema.org that we could just co-opt? |
I wonder what the purpose of this is? What is the requirement you're trying to address? My understanding of the |
From the call today, my understanding was that he was looking to be able to tag items with clear typing so that the UI could be more informed. A bare string is not as useful as a defined type (e.g., tax; shipping; discount). A UI could use this information to call out important line items or address regulatory requirements (in some jurisdictions VAT must be clearly distinguished from other fees). |
I think we should wait before adding this. Since it would be for display only, it would presumably be optional to process (just like label). I think we should get some experience with implementations before adding something like this. It doesn't feel essential to a minimum viable feature (and I think, as a consequence, that we probably wouldn't process it in our initial implementation). |
That recommendation aligns with how I have seen similar cases progress (or not progress) with other specs for the platform. That is, in cases where we have gone ahead and added something like this to a spec at this point in its lifecycle—before it was clear that browser implementors were on board with implementing it per spec—we have often seen it never end up getting implemented interoperably per spec (and so eventually just dropped from the spec). This seems like one of those kinds of refinements that instead emerge (if at all) only after the initial implementations have been deployed and we start to get real-world feedback from web developers. |
Hmm, I can see what you are saying about waiting, but I believe the problem is unavoidable. @halindrome nailed my point, which is that there are certain expectations people have when viewing a display of what they are paying for. Without strong typing of what the items mean, any implementor is left to just figure it out and that is certain to fail. Wouldn't it be really confusing if your tax item was rendered first, then an actual good, then a discount, then another good? What a weird and clunky checkout experience. I don't have any solid examples off the top of my head other than taxes, but I believe there are strong reasons why payment apps would want to know the breakdown of the payment as well. Since we have constructed the PaymentRequest prior to engaging with the payment app, our only hope for providing itemization compatibility between the apps is to provide an option to strongly type them. |
Or just not use the feature. Since there are no requirements on the feature's behavior it is essentially useless. I know, I know - that's just my opinion. |
Isn't displayItems an ordered list? Is the concern here user agents won't respect the ordering? |
Regardless of the type of displayItems, there are no requirements on its handling. Your example is one potential concern. Another would be displaying only some of the items (because of space limitations, for example). I have no idea. I am just being pathological because that's how I explore edge cases in standards. |
+1 to @nickjshearer 's point. If the browser respects the ordering then I don't know what this solves? |
Okay. Then would you be open to language that requires that? As far as I know there is no requirement for this now. There are no requirements on the UI at all. "If the payment mediator exposes the displayItems to the user, it MUST expose all of them in the order in which they were supplied." Or something to that effect? |
I am documenting this sort of statement; see: Ian |
@halindrome said:
+1 but I think we'd need to accommodate a size restriction or acceptable behaviour if the list is too big to show. @ianbjacobs while this pertains to the UI the browser will show the payer as part of the app selection process it doesn't really impact the payment app stuff you are documenting I don't think. |
So then are we saying that there is a single ordering that is standard across all markets? I honestly don't know the answer here, but unless every market has the same expectations about the ordering of items we would need orderings to be re-orderable based on the billing address or something like that. Or...better markup of item type so browsers can do the regionally-appropriate thing. |
The ordering decision is made by the website. It submits the list of items in the request. In that context I'd say that the website needs to be aware of any local requirements or norms in the same way as it must know what language to use, there is no requirement on the user agent to know this or do any ordering. I suggest we can close this issue. |
Reopening again, as this has come up as an issue in our (Mozilla's) design. In our design, we want to be able to separate and order @adrianba, @zkoch, @michelle, would appreciate your input. cc @mnoorenberghe. |
What sorts of types would be included? From reading the thread here, it sounds like the display of tax line items (and variance per customer region) is the primary issue. |
Yeah, primary motivator is a tax line. |
@marcoscaceres do you not believe this to be true? I think it's fair to reduce the scope of the API and say that all local regulatory requirements & l10n generally should be handled by the merchant. The merchant has to calculate tax amount and localize display items themselves anyways, so having the merchant order the display items themselves doesn't seem unreasonable. |
Agree that the design as is addressed the minimum use case. However, there might still be an expectation because of local conventions to more clearly call out things like tax (our design literally has a tax line at the moment). Places like Canada and the US are good examples because some merchants don’t show tax in the sale price, it varies from place to place, etc. |
I'm OK with this. |
@marcoscaceres I think it might help to see examples, but I have some doubt that the browser can get this right currently. The tricky part is that the formatting isn't just locale-specific. The formatting of EU VAT on a receipt, for example, may depend not just on the current locale but also the kind of payment being made and the VAT status and home country of the merchant and the customer. And other information that the merchant will need to have but that doesn't yet have a home in PRAPI. |
@marcoscaceres is this requirement more about a way to group the displayItems into list items and sub-total items as opposed to having special types for displayItems? Could we get away with a flag on a displayItem that indicates it should be part of the sub-total group? I worry we are getting into territory where we are inferring UI behaviour/layout |
Yes, the main issue is about having the tax line not be part of the scrollable list of other line items (there could be hundreds).
The merchant can try to be helpful and always sort tax to the bottom of their list of items but that doesn't really help if there are too many items that the item list becomes scrollable. In the image above, the section above the horizontal rule and below "Order Details" will be scrollable. |
The ideal solution is probably too big a change at this stage (making dictionary PaymentItem {
required DOMString label;
required PaymentCurrencyAmount amount;
boolean pending = false;
boolean subTotal = false;
}; If |
Perhaps the ideal non-breaking change is adding const paymentRequest = new PaymentRequest(
[{supportedMethods: 'basic-card'}],
{
subtotal: {
label: 'Sub Total',
amount: {
currency: 'USD',
value: '6.00',
},
},
shipping: {
label: 'Shipping',
amount: {
currency: 'USD',
value: '3.00',
},
},
tax: {
label: 'Tax',
amount: {
currency: 'USD',
value: '1.00',
},
},
total: {
label: 'Total',
amount: {
currency: 'USD',
value: '10.00',
},
},
displayItems: aVeryLongListHere,
}); |
No, it's more about classifying display items by type and letting the UA do whatever it wants with that information.
I'm not a big fan of this because it makes an assumption about how the data would be displayed. I'd rather say "x is 'tax'", and then the UA can be like "oh, that's 'tax', I'll put that with the tax thingies down the bottom".
Me too :) Hence just saying this is "tax" and letting the UA figure it out. |
@rsolomakhin, respectfully, that seems to add a fair bit of redundancy. For example, "shipping" is already derived from the selected shipping option. It also limits the display of things (e.g., tax) to a single item, but there might be multiple taxes being applied (e.g., a "GST" and some random state tax). That's why I think just having the type specifically on the |
Sounds good, @marcoscaceres. |
So
Is type a string or an enum? |
We have an enum in |
Defining that could be fun 😄 Is there a reference of "invoice line item types" or similar we can use? |
@adrianhopebailie I was going to start with "tax", but we can build it up from there. |
Hi folks involved in this thread. I've sent a proposal: Which you can preview: Would like to hear your thoughts before continuing. |
Closed via 4b98e53 |
Before I spend the time to put out a PR on this, I want to get some initial feedback. I think there are many kinds of PaymentItems that could be interesting/impactful to the payment app. I still like calling out the total as a separate item, but I would like to propose we go a step further and give all PaymentItems a type.
The PaymentItemType enum would provide a way to assign specific meaning/semantics for common use cases, e.g. taxes, discounts, etc. We would control this enum and make additions to it as needed. The enum would have a fallback value that means the PaymentItem is purely for display and has no significant meaning beyond the merchant's purposes.
What do you think? With moderate approval I will code it up.
The text was updated successfully, but these errors were encountered: