Update complete() method to take a string and clarify its purpose

[adrianba]

The argument to complete is used to influence the user experience as the
user interface for the payment request is dismissed. Some sites will not
be able to provide an indication of success (perhaps because they
require additional flow steps after the payment request before accepting
an order). Calling complete() with no arguments uses the default value
where success or failure are not indicated.

[halindrome]

Have you filed a ReSpec bug on this?

[adrianba]

Have you filed a ReSpec bug on this?

This was my mistake. Fixed now.

[msporny]

+1 to merge, this is better than the boolean in the spec right now. I feel a bit uneasy about the string value of "" (and the list of potential values growing in the future) and how a browser would affect the UI flow based on that value, but expect the browser vendors to give proper feedback on this in time. From a developers perspective, what you pass to the function is more clear now by just reading the code... I know what .complete("success") and .complete("fail") mean.

What happens when someone does .complete("failure")?

[dlongley]

I prefer .succeed() and .fail() or something along those lines. Passing a string isn't all that much better than a boolean (which is bad), IMO. Could consider passing a Promise to be resolved/rejected.

[msporny]

To be clear, I prefer .succeed() and .fail() too, but baby steps toward that. I'd rather have this PR in the spec than what's in the spec right now.

[adrianhopebailie]

LGTM

[adrianba]

@msporny wrote:

I feel a bit uneasy about the string value of ""

Yeah, I'm not totally wedded to this - the reason I chose "" is because it is falsy in JavaScript.

[adrianba]

@dlongley wrote:

I prefer .succeed() and .fail() or something along those lines.

Would those methods take arguments? What if you don't have a success or a failure - you just don't know yet. Would you propose a third method? If we have three methods then at some point we might have four or more methods?

[adrianba]

@msporny wrote:

and the list of potential values growing in the future

Good point. There is a downside in that adding new values doesn't indicate whether they are successful values or failing values. For example, what if we wanted a new UX for "timeout" that was distinct from a general failure animation.

[dlongley]

Could do: complete({status: <enum>}); like complete({status: 'success'});

If we want to support the most extensibility.

[jnormore]

👍 to extensibility with status @dlongley

My thoughts behind this are that eventually we could be convinced to add more error handling functionality (like an error message supplied by the merchant).

[adrianba]

Merged as 5bcadcc per telcon.