w3c / payment-request

Payment Request API
https://www.w3.org/TR/payment-request/
Other
488 stars 135 forks source link

add paymentmethodchange event #695

Closed marcoscaceres closed 6 years ago

marcoscaceres commented 6 years ago

closes #662

The following tasks have been completed:

Implementation commitment:


Preview | Diff

marcoscaceres commented 6 years ago

Only indirectly and in a backwards compatibile way. I hope to have a draft ready for review tomorrow.

rsolomakhin commented 6 years ago

No rush! Just checking.

marcoscaceres commented 6 years ago

Basic card integration https://github.com/w3c/payment-method-basic-card/pull/53

marcoscaceres commented 6 years ago

Sent tests for review https://github.com/w3c/web-platform-tests/pull/10912

marcoscaceres commented 6 years ago

Realized that when other events fire (e.g., onshippingaddresschange) or generally things happen on the user interaction task source (e.g., user aborts, request.abort()), the methodDetails needs to be reset to null... same with methodName.

marcoscaceres commented 6 years ago

Hmmm... having spec'ed this out, now I don't know if I like this design anymore 😓 Doesn't feel right that the only thing that changes .methodName and .methodData is this event.

Maybe this should switch back to having special event for this, as per https://github.com/w3c/payment-request/issues/662#issuecomment-359038775

The name paymentmethodchange is also super ambiguous tho, because it's really a "Instrument change" of a payment method. So, I I'm kind thinking "instrumentchange" is more appropriate as an event name.

I think I'd like to change this to:

PaymentMethodChangeEvent : PaymentRequestUpdateEvent {
   readonly attribute DOMString methodName;
   readonly attribute object? methodDetails;
}

So that:

request.oninstrumentchange = ev => {
  const { type: cardType } = ev.methodDetails;
  if (ev.methodName === "https://apple.com/apple-pay") {
    switch (cardType) {
      case "store":
        // do Apple Pay specific handling for store card...
        break;
    }
  }
  // finally...
  ev.updateWith(newStuff);
};

@aestes wdyt?

marcoscaceres commented 6 years ago

Updated this to match https://github.com/w3c/payment-request/pull/695#issuecomment-387978220

marcoscaceres commented 6 years ago

Updated WPTests to match updated spec.

marcoscaceres commented 6 years ago

@domenic, this one too should now be good to go (🤞).

marcoscaceres commented 6 years ago

@domenic, I think I've addressed all the feedback 🤞

marcoscaceres commented 6 years ago

Added link to Firefox tracking bug.

marcoscaceres commented 6 years ago

Added developer documentation. Please review. https://developer.mozilla.org/en-US/docs/Web/API/PaymentRequest/onpaymentmethodchange

marcoscaceres commented 6 years ago

PaymentMethodChangeEvent is now in Firefox Nightly behind a pref. We don't currently have any plans to fire it for Basic Card, but other payment handlers could theoretically make use of it.