1432927 - Show a payment card input form before the summary view for users without a saved payment card

Status: RESOLVED FIXED

(Firefox :: WebPayments UI, enhancement, P1)

Description

[Matthew N. [:MattN]]

We want to optimize the flow for new users of PaymentRequest to ensure they have a smooth experience. If the user has no saved payment cards in storage (e.g. from Form Autofill), then we don't want to present the user with an empty payment card dropdown, instead we should take the user directly to the "add payment card" form so the flow is somewhat more like a wizard.

E.g. from a UX proposal:
FTU (bug 1432912) => Address Form (bug 1432921) => Add Credit Card Form => Summary

Comment 1

[:prathiksha]

Attachment: Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

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

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8976342 [details]
Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

https://reviewboard.mozilla.org/r/244520/#review250546

This is on the right track, just some things to improve.

::: browser/components/payments/res/containers/address-form.js:153
(Diff revision 1)
>    }
>  
>    saveRecord() {
>      let record = this.formHandler.buildFormObject();
>      let {
> -      page,
> +      page, savedBasicCards,

One property per line please so it's easier to track when a property is added in mercurial blame. I thought eslint had a rule for this…

::: browser/components/payments/res/containers/address-form.js:170
(Diff revision 1)
>          page: {
> -          id: "payment-summary",
> +          id: "basic-card-page",
> +          onboardingWizard: true,
>          },

Similar to bug 1461886, you should pass other relevant properties like the `title`.

::: browser/components/payments/res/containers/basic-card-form.js:99
(Diff revision 1)
> +    this.cancelButton.textContent = this.dataset.cancelButtonLabel;
>      this.backButton.textContent = this.dataset.backButtonLabel;
>      this.saveButton.textContent = this.dataset.saveButtonLabel;
>      this.persistCheckbox.label = this.dataset.persistCheckboxLabel;
>  
> +    this.backButton.hidden = !!page.onboardingWizard;

Please include a comment with a bug number of where it will be fixed indicating that this is just a temporary solution.

::: browser/components/payments/res/paymentRequest.js:134
(Diff revision 1)
> +      state.page = {
> +        id: "basic-card-page",
> +        onboardingWizard: true,
> +        onboardingWithBasicCardPage: true,
> +      };

Like bug 1461886, please pass the `title` and other relevant properties.

::: browser/components/payments/res/paymentRequest.js:137
(Diff revision 1)
>        };
> +    } else if (Object.keys(detail.savedBasicCards).length == 0) {
> +      state.page = {
> +        id: "basic-card-page",
> +        onboardingWizard: true,
> +        onboardingWithBasicCardPage: true,

`onboardingWithBasicCardPage` is not used and should be removed.

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:50
(Diff revision 1)
> +      let address = {
> +        "given-name": "Jared",
> +        "family-name": "Wein",
> +        "organization": "Mozilla",
> +        "street-address": "404 Internet Lane",
> +        "address-level2": "Firefoxity City",
> +        "address-level1": "CA",
> +        "postal-code": "31337",
> +        "country": "US",
> +        "tel": "[phone]",
> +        "email": "[e-mail]",
> +      };

Please re-use one of the PTU addresses rather than duplicating it here by passing it as an argument to the content task

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:63
(Diff revision 1)
> +      for (let [key, val] of Object.entries(address)) {
> +        let field = content.document.getElementById(key);
> +        if (!field) {
> +          ok(false, `${key} field not found`);
> +        }
> +        field.value = val;
> +        ok(!field.disabled, `Field #${key} shouldn't be disabled`);
> +      }
> +      content.document.querySelector("address-form .save-button").click();

Can you ensure that the cancel button is visible?

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:80
(Diff revision 1)
> +      let card = {
> +        "cc-number": "4111111111111111",
> +        "cc-name": "J. Smith",
> +        "cc-exp-month": 11,
> +        "cc-exp-year": "2018",
> +      };

Same for the credit card.

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:94
(Diff revision 1)
> +      await ContentTaskUtils.waitForCondition(() =>
> +        content.document.getElementById("cancel"), "Payment summary page is now rendered");

This isn't actually checking that the summary page is rendered since that cancel button is always in the DOM. You would need to check the visibility.

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:232
(Diff revision 1)
> +    info("Check if the total header is visible on the basic card page during on-boarding");
> +    let isHeaderVisible =
> +      spawnPaymentDialogTask(frame, PTU.DialogContentTasks.isElementVisible, "header");
> +    ok(isHeaderVisible, "Total Header is visible on the basic card page during on-boarding");

Also check that it has textContent

Comment 3

[:prathiksha]

Comment on attachment 8976342 [details]
Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

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

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8976342 [details]
Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

https://reviewboard.mozilla.org/r/244520/#review250868

::: browser/components/payments/res/containers/basic-card-form.js:93
(Diff revision 2)
>        page,
>        savedAddresses,
>        selectedShippingAddress,
>      } = state;
>  
> -    this.pageTitle.textContent = page.title;
> +    this.pageTitle.textContent = page.title || this.dataset.addBasicCardTitle;

As we discussed in-person, this needs to be done inside the `if (editing) {` block below and in the `else` associated with it so that we can choose the add or edit title as appropriate.

::: browser/components/payments/res/paymentRequest.xhtml:138
(Diff revision 2)
>          <order-details></order-details>
>        </section>
>  
>        <basic-card-form id="basic-card-page"
>                         class="page"
> +                       data-add-basic-card-title="&basicCard.addPage.title;"

Add the edit title here too

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8976342 [details]
Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

https://reviewboard.mozilla.org/r/244520/#review250874

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:10
(Diff revision 2)
> +add_task(async function test_onboarding_wizard_without_saved_addresses_and_saved_cards() {
>    await BrowserTestUtils.withNewTab({

Please remember to file a bug to show the billing address page (and title) if there are no cards or addresses.

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:68
(Diff revision 2)
> +      await ContentTaskUtils.waitForCondition(() =>
> +        content.document.querySelector("basic-card-form .save-button"),
> +                                              "Basic card page is now rendered");

This doesn't check that it's rendered, only that it exists in the DOM which could be true for other reasons. Can you use 
```js
PTU.DialogContentTasks.isElementVisible("basic-card-form .save-button")
``` 
instead?

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:151
(Diff revision 2)
> -            content.document.querySelector("address-form .back-button"),
> +            content.document.querySelector("address-form .save-button"),
>                                                    "Address page is rendered");

Save here about using isElementVisible

Comment 6

[:prathiksha]

Comment on attachment 8976342 [details]
Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

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

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 8976342 [details]
Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

https://reviewboard.mozilla.org/r/244520/#review250890

::: browser/components/payments/res/containers/basic-card-form.js:92
(Diff revision 3)
>  
> -    this.pageTitle.textContent = page.title;
> +    this.cancelButton.textContent = this.dataset.cancelButtonLabel;
>      this.backButton.textContent = this.dataset.backButtonLabel;

You should also delete the highlighted lines that used to set this: https://dxr.mozilla.org/mozilla-central/rev/1800b8895c08bc0c60302775dc0a4b5ea4deb310/browser/components/payments/res/containers/payment-method-picker.js#141,148

and

https://dxr.mozilla.org/mozilla-central/rev/1800b8895c08bc0c60302775dc0a4b5ea4deb310/browser/components/payments/res/paymentRequest.xhtml#107-108

Comment 8

[:prathiksha]

Comment on attachment 8976342 [details]
Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

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

Comment 9

[:prathiksha]

Attachment: Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

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

Comment 10

[Matthew N. [:MattN]]

Comment on attachment 8976755 [details]
Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

https://reviewboard.mozilla.org/r/244854/#review250912

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3aa4ee6077c5
Show a payment card input form before the summary view for users without a saved payment card. r=MattN

Comment 12

[Natalia Csoregi [:nataliaCs]]

Backed out for payments/test/mochitest/test_basic_card_form.html

Backout: https://hg.mozilla.org/integration/autoland/rev/ac730cb87faa452ebcda15300546e1f6ab7f401b

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3aa4ee6077c50b6408e8d09414fa9a13939bb18f&group_state=expanded

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=179112250&repo=autoland&lineNumber=2778

[task 2018-05-18T01:53:22.463Z] 01:53:22     INFO - TEST-START | browser/components/payments/test/mochitest/test_basic_card_form.html
[task 2018-05-18T01:53:23.341Z] 01:53:23     INFO - TEST-INFO | started process screentopng
[task 2018-05-18T01:53:23.851Z] 01:53:23     INFO - TEST-INFO | screentopng: exit 0
[task 2018-05-18T01:53:23.851Z] 01:53:23     INFO - Buffered messages logged at 01:53:23
[task 2018-05-18T01:53:23.851Z] 01:53:23     INFO - AddTask.js | Entering test test_initialState
[task 2018-05-18T01:53:23.851Z] 01:53:23     INFO - TEST-PASS | browser/components/payments/test/mochitest/test_basic_card_form.html | Check initial page 
[task 2018-05-18T01:53:23.852Z] 01:53:23     INFO - TEST-PASS | browser/components/payments/test/mochitest/test_basic_card_form.html | Check initial page after appending 
[task 2018-05-18T01:53:23.853Z] 01:53:23     INFO - AddTask.js | Leaving test test_initialState
[task 2018-05-18T01:53:23.855Z] 01:53:23     INFO - AddTask.js | Entering test test_backButton
[task 2018-05-18T01:53:23.856Z] 01:53:23     INFO - Buffered messages finished
[task 2018-05-18T01:53:23.858Z] 01:53:23     INFO - TEST-UNEXPECTED-FAIL | browser/components/payments/test/mochitest/test_basic_card_form.html | Check title - got "", expected "Sample page title 2"
[task 2018-05-18T01:53:23.859Z] 01:53:23     INFO -     SimpleTest.is@SimpleTest/SimpleTest.js:312:5
[task 2018-05-18T01:53:23.859Z] 01:53:23     INFO -     test_backButton@browser/components/payments/test/mochitest/test_basic_card_form.html:80:3
[task 2018-05-18T01:53:23.860Z] 01:53:23     INFO -     async*add_task/</<@SimpleTest/AddTask.js:57:34
[task 2018-05-18T01:53:23.861Z] 01:53:23     INFO -     async*add_task/<@SimpleTest/AddTask.js:31:10
[task 2018-05-18T01:53:23.862Z] 01:53:23     INFO -     setTimeout handler*SimpleTest_setTimeoutShim@SimpleTest/SimpleTest.js:676:12
[task 2018-05-18T01:53:23.863Z] 01:53:23     INFO -     add_task@SimpleTest/AddTask.js:30:7
[task 2018-05-18T01:53:23.863Z] 01:53:23     INFO -     @browser/components/payments/test/mochitest/test_basic_card_form.html:55:1
[task 2018-05-18T01:53:23.864Z] 01:53:23     INFO - TEST-PASS | browser/components/payments/test/mochitest/test_basic_card_form.html | Check label

Comment 13

[Matthew N. [:MattN]]

Attachment: Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

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

Comment 14

[Matthew N. [:MattN]]

Comment on attachment 8976764 [details]
Bug 1432927 - Show a payment card input form before the summary view for users without a saved payment card.

https://reviewboard.mozilla.org/r/244880/#review250926

Comment 15

[Matthew N. [:MattN]]

I had a fix ready to land before this got backed out which is unfortunate…

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d53f1330f08c
Show a payment card input form before the summary view for users without a saved payment card. r=MattN

Comment 17

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/d53f1330f08c