Closed
Bug 1432927
Opened 7 years ago
Closed 7 years ago
Show a payment card input form before the summary view for users without a saved payment card
Categories
(Firefox :: WebPayments UI, enhancement, P1)
Firefox
WebPayments UI
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
Updated•7 years ago
|
Priority: P3 → P2
Whiteboard: [webpayments]
Updated•7 years ago
|
Product: Toolkit → Firefox
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Updated•7 years ago
|
Priority: P2 → P1
Comment hidden (mozreview-request) |
Reporter | ||
Comment 2•7 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Reporter | ||
Comment 4•7 years ago
|
||
mozreview-review |
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)
Reporter | ||
Comment 5•7 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Reporter | ||
Comment 7•7 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8976342 -
Attachment is obsolete: true
Reporter | ||
Comment 10•7 years ago
|
||
mozreview-review |
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+
Comment 11•7 years ago
|
||
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
Comment 12•7 years ago
|
||
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 hidden (mozreview-request) |
Reporter | ||
Comment 14•7 years ago
|
||
mozreview-review |
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+
Reporter | ||
Comment 15•7 years ago
|
||
I had a fix ready to land before this got backed out which is unfortunate…
Comment 16•7 years ago
|
||
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
Comment 17•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
You need to log in
before you can comment on or make changes to this bug.
Description
•