1435101 - Only show shipping (option & address) pickers in the payment request dialog when requestShipping is true

Status: RESOLVED FIXED

(Firefox :: WebPayments UI, enhancement, P1)

Description

[Matthew N. [:MattN]]

The visibility of the shipping option and shipping address pickers should be controlled by `state.request.paymentOptions.requestShipping` in the relevant render methods.

Integration tests should ensure that a default shipping addresses isn't sent if one is in storage.

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1435101 - Only show the shipping options and shipping address if requestShipping is true.

Review commit: https://reviewboard.mozilla.org/r/222046/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222046/

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8952818 [details]
Bug 1435101 - Only show the shipping options and shipping address if requestShipping is true.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222046/diff/1-2/

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8952818 [details]
Bug 1435101 - Only show the shipping options and shipping address if requestShipping is true.

https://reviewboard.mozilla.org/r/222046/#review227952

::: toolkit/components/payments/res/paymentRequest.xhtml:70
(Diff revision 2)
>  
>          <section>
> -          <div><label>&shippingAddressLabel;</label></div>
> -          <address-picker selected-state-key="selectedShippingAddress"></address-picker>
> -          <div><label>&shippingOptionsLabel;</label></div>
> -          <shipping-option-picker></shipping-option-picker>
> +          <div class="shippingRequested"><label>&shippingAddressLabel;</label></div>
> +          <address-picker class="shippingRequested" selected-state-key="selectedShippingAddress"></address-picker>
> +          <div class="shippingRequested"><label>&shippingOptionsLabel;</label></div>
> +          <shipping-option-picker class="shippingRequested"></shipping-option-picker>

In some ways I think the render function of <shipping-option-picker> should set `this.hidden = true;` instead but since the label is separate and would also need to be hidden I guess we can try this.

::: toolkit/components/payments/test/browser/browser_request_shipping.js:4
(Diff revision 2)
> +  let onChanged = TestUtils.topicObserved("formautofill-storage-changed",
> +                                      (subject, data) => data == "add");

Nit: indentation seems off

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8952818 [details]
Bug 1435101 - Only show the shipping options and shipping address if requestShipping is true.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/222046/diff/2-3/

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8952818 [details]
Bug 1435101 - Only show the shipping options and shipping address if requestShipping is true.

https://reviewboard.mozilla.org/r/222046/#review228020

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6c2b9768775d
Only show the shipping options and shipping address if requestShipping is true. r=MattN

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1435101 - Wrap some lines to under 100 characters to make eslint happy.

Review commit: https://reviewboard.mozilla.org/r/222110/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/222110/

Comment 8

[Matthew N. [:MattN]]

Comment on attachment 8952872 [details]
Bug 1435101 - Wrap some lines to under 100 characters to make eslint happy.

https://reviewboard.mozilla.org/r/222110/#review228046

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33318adf48fc
Wrap some lines to under 100 characters to make eslint happy. r=MattN

Comment 10

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/6c2b9768775d
https://hg.mozilla.org/mozilla-central/rev/33318adf48fc