Re: [w3c/payment-request] fine-grained errors reporting for PaymentAddress (#712)

domenic approved this pull request.

Mostly LGTM, although I agree with others who are concerned about the redundancy. Will leave it to you to make the call though.

> @@ -2719,6 +2761,152 @@ <h2>
           </dd>
         </dl>
       </section>
+      <section data-dfn-for="AddressErrorFields" data-link-for=
+      "AddressErrorFields">
+        <h2>
+          <dfn>AddressErrorFields</dfn> dictionary
+        </h2>
+        <pre class="idl">
+          dictionary AddressErrorFields {
+            DOMString addressLineError;

I tend to agree this is redundant. I think you have chosen a code style with a combination of object literal shorthand, destructuring, and redundant variable declarations that makes it extra confusing. But I don't think that's a good reason to verbose-ify the API, personally. I would instead suggest writing your code in a more conventional way, such as

```js
function validateAddress(address){
   return {
    postalCode: !isValidPostalCode(address.postalCode) ? "Invalid zip code" : undefined
  };
}

function collectErrors(response) {
   return {
    shippingAddress: validateAddress(response.shippingAddress)
  };
}
```

(basically, don't declare variables you're only going to use once.

> +            DOMString countryError;
+            DOMString dependentLocalityError;
+            DOMString languageCodeError;
+            DOMString organizationError;
+            DOMString phoneError;
+            DOMString postalCodeError;
+            DOMString recipientError;
+            DOMString regionError;
+            DOMString regionCodeError;
+            DOMString sortingCodeError;
+          };
+        </pre>
+        <p>
+          The members of the <a>AddressErrorFields</a> dictionary represent
+          validation errors with specific parts of a <a>physical address</a>.
+          Each dictionary member has a dual function: firstly, it denotes that

I'd say "firstly, its presence"

> +          end user.
+        </p>
+        <pre class="example">
+          request.onshippingaddresschange = ev =&gt; {
+            ev.updateWith(validateAddress(request.shippingAddress));
+          };
+          function validateAddress(shippingAddress) {
+            const error = "Can't ship to this address.";
+            const shippingAddressErrors = {
+              cityError: "FarmVille is not a real place.",
+              postalCodeError: "Unknown postal code for your country.",
+            };
+            // Empty shippingOptions implies that we can't ship
+            // to this address.
+            const shippingOptions = [];
+            return { error, shippingAddressErrors, shippingOptions };

Empty shippingOptions should not be necessary, since not-present gets converted to empty in the processing model, I am pretty sure.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/payment-request/pull/712#pullrequestreview-126175638