Closed Bug 1432927 Opened 2 years ago Closed Last year

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

Categories

(Firefox :: WebPayments UI, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: MattN, Assigned: prathiksha)

References

(Blocks 1 open bug)

Details

(Whiteboard: [webpayments])

Attachments

(2 files, 1 obsolete file)

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
Priority: P3 → P2
Whiteboard: [webpayments]
Product: Toolkit → Firefox
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Priority: P2 → P1
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": "+15555551212",
> +        "email": "test@example.com",
> +      };

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
Attachment #8976342 - Flags: review?(MattN+bmo)
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
Attachment #8976342 - Flags: review?(MattN+bmo)
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 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
Attachment #8976342 - Flags: review?(MattN+bmo) → review+
Attachment #8976342 - Attachment is obsolete: true
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
Attachment #8976755 - Flags: review?(MattN+bmo) → review+
Pushed by mozilla@noorenberghe.ca:
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
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 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
Attachment #8976764 - Flags: review?(MattN+bmo) → review+
I had a fix ready to land before this got backed out which is unfortunate…
Pushed by mozilla@noorenberghe.ca:
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
https://hg.mozilla.org/mozilla-central/rev/d53f1330f08c
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
You need to log in before you can comment on or make changes to this bug.