Update PaymentRequest header and footer match the visual specs

RESOLVED FIXED in Firefox 63

Status

()

enhancement
P1
normal
RESOLVED FIXED
Last year
11 months ago

People

(Reporter: MattN, Assigned: MattN)

Tracking

Trunk
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

(Whiteboard: [webpayments])

User Story

* 63: Footer button consistent position
* Scrollable area above the footer
* Footer grey background
* 63: Remove View Cart / View All Items
* Remove unused "Total" label element
* 63: Add "Pay to"
* Add currency code to header
* total colouring
* Remove "Your Payment" heading

Attachments

(5 attachments, 1 obsolete attachment)

Various updates discussed during the SF All Hands to match the visual specs.
User Story: (updated)
While making the footer position correct, I may as well make the middle area scrollable.
User Story: (updated)
Priority: P2 → P1
Depends on: 1470203
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 17

Last year
mozreview-review
Comment on attachment 8986087 [details]
Bug 1469464 - Consistent PaymentRequest footer positioning with <payment-request-page>.

https://reviewboard.mozilla.org/r/251542/#review263034

I got test failures at this point in the patch series. I'm going through in order, so if those are addressed in a later patch maybe they should be folded together? It looks great otherwise. Should just be some updating of references so I'll call it a nit.
Attachment #8986087 - Flags: review?(sfoster) → review+

Comment 18

Last year
mozreview-review
Comment on attachment 8986085 [details]
Bug 1469464 - Always hide View All, remove summary heading, add 'Pay to'.

https://reviewboard.mozilla.org/r/251538/#review263036

Looks good
Attachment #8986085 - Flags: review?(sfoster) → review+

Comment 19

Last year
mozreview-review
Comment on attachment 8986086 [details]
Bug 1469464 - Display the currency code with the total.

https://reviewboard.mozilla.org/r/251540/#review263038

Big improvement
Attachment #8986086 - Flags: review?(sfoster) → review+

Comment 20

Last year
mozreview-review
Comment on attachment 8990581 [details]
Bug 1469464 - Fix default shippingAddressErrors value for debugging.

https://reviewboard.mozilla.org/r/255646/#review263040
Attachment #8990581 - Flags: review?(sfoster) → review+

Comment 21

Last year
mozreview-review
Comment on attachment 8990858 [details]
Bug 1469464 - PaymentRequest footer styling.

https://reviewboard.mozilla.org/r/255856/#review263044

::: browser/components/payments/res/containers/address-form.css:77
(Diff revision 1)
>  :-moz-any(input, textarea, select):not(:focus) + .error-text,
>  :-moz-any(input, textarea, select):valid + .error-text {
>    display: none;
>  }
> +
> +address-form > footer > .cancel-button {

Isnt this (eventually) going to follow platform UI conventions?
Attachment #8990858 - Flags: review?(sfoster) → review+
Assignee

Comment 22

Last year
mozreview-review-reply
Comment on attachment 8986087 [details]
Bug 1469464 - Consistent PaymentRequest footer positioning with <payment-request-page>.

https://reviewboard.mozilla.org/r/251542/#review263034

Oops, I think I renamed that variable near the end and forget to re-run the tests.
Assignee

Comment 23

Last year
mozreview-review-reply
Comment on attachment 8990858 [details]
Bug 1469464 - PaymentRequest footer styling.

https://reviewboard.mozilla.org/r/255856/#review263044

> Isnt this (eventually) going to follow platform UI conventions?

Yeah, I suspect so (I mentioned it to UX the other day) but UX is susposed to get back to us on the button ordering. I'll include that in bug 1468153 and mention the bug number.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 29

Last year
mozreview-review
Comment on attachment 8986085 [details]
Bug 1469464 - Always hide View All, remove summary heading, add 'Pay to'.

https://reviewboard.mozilla.org/r/251538/#review263324


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/components/payments/test/mochitest/test_payment_dialog.html:97
(Diff revision 5)
>  add_task(async function test_viewAllButtonVisibility() {
>    await setup();
>  
>    let button = el1._viewAllButton;
>    ok(button.hidden, "Button is initially hidden when there are no items to show");
> +  ok(isHidden(button), "Button should be visibly hidden since bug 1469464")

Error: Missing semicolon. [eslint: semi]

Comment 30

Last year
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/4b86bde79c91
Always hide View All, remove summary heading, add 'Pay to'. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/9453814beada
Display the currency code with the total. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/90654d5df81a
Fix default shippingAddressErrors value for debugging. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/58c1a79fb661
Consistent PaymentRequest footer positioning with <payment-request-page>. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/59c418929764
PaymentRequest footer styling. r=sfoster

Comment 31

Last year
mozreview-review
Comment on attachment 8986087 [details]
Bug 1469464 - Consistent PaymentRequest footer positioning with <payment-request-page>.

https://reviewboard.mozilla.org/r/251542/#review263326


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/components/payments/test/mochitest/test_payment_dialog.html:97
(Diff revision 5)
>  add_task(async function test_viewAllButtonVisibility() {
>    await setup();
>  
>    let button = el1._viewAllButton;
>    ok(button.hidden, "Button is initially hidden when there are no items to show");
>    ok(isHidden(button), "Button should be visibly hidden since bug 1469464")

Error: Missing semicolon. [eslint: semi]
Comment hidden (mozreview-request)
Assignee

Comment 33

Last year
mozreview-review
Comment on attachment 8991520 [details]
Bug 1469464 - Fix eslint missing semicolon in test_payment_dialog.html.

https://reviewboard.mozilla.org/r/256432/#review263328
Attachment #8991520 - Flags: review?(MattN+bmo) → review+

Comment 34

Last year
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/f9d4885e8956
Fix eslint missing semicolon in test_payment_dialog.html. r=MattN
Backed out 6 changesets (bug 1469464) for browser-chrome failures at browser/base/content/test/static/browser_parsable_css.js 

Backout: https://hg.mozilla.org/integration/autoland/rev/135aaab3fef515f55cd323f2b2248d9c9aa61c88

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=59c4189297640ec9c2bdbb2d3bb7b617cb74e6e5

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

0:17:28     INFO - Ignored error "Expected media feature name but found ‘-moz-is-glyph’." on resource://gre/res/svg.css because of whitelist item {"sourceName":"/\\b(autocomplete-item|svg)\\.css$/","errorMessage":"/Expected media feature name but found \\u2018-moz.*/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Expected media feature name but found ‘-moz-is-resource-document’." on resource://gre/res/svg.css because of whitelist item {"sourceName":"/\\b(autocomplete-item|svg)\\.css$/","errorMessage":"/Expected media feature name but found \\u2018-moz.*/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown pseudo-class or pseudo-element ‘-moz-svg-foreign-content’.  Ruleset ignored due to bad selector." on resource://gre/res/svg.css because of whitelist item {"sourceName":"/\\b(contenteditable|EditorOverride|svg|forms|html|mathml|ua)\\.css$/i","errorMessage":"/Unknown pseudo-class.*-moz-/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown pseudo-class or pseudo-element ‘-moz-svg-text’.  Ruleset ignored due to bad selector." on resource://gre/res/svg.css because of whitelist item {"sourceName":"/\\b(contenteditable|EditorOverride|svg|forms|html|mathml|ua)\\.css$/i","errorMessage":"/Unknown pseudo-class.*-moz-/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown pseudo-class or pseudo-element ‘-moz-svg-marker-anon-child’.  Ruleset ignored due to bad selector." on resource://gre/res/svg.css because of whitelist item {"sourceName":"/\\b(contenteditable|EditorOverride|svg|forms|html|mathml|ua)\\.css$/i","errorMessage":"/Unknown pseudo-class.*-moz-/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Buffered messages finished
00:17:28     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | Got error message for resource://payments/paymentRequest.css: Expected declaration but found ‘"disabled-overlay"’.  Skipped to next declaration. - 
00:17:28     INFO - Stack trace:
00:17:28     INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_parsable_css.js:messageIsCSSError:243
00:17:28     INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_parsable_css.js:checkAllTheCSS:442
00:17:28     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1103
00:17:28     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1094
00:17:28     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:996
00:17:28     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795
00:17:28     INFO - Ignored error "Unknown property ‘user-select’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown property ‘appearance’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown property ‘user-select’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown property ‘user-select’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown property ‘user-select’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown property ‘user-select’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown property ‘user-select’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown property ‘user-select’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown property ‘user-select’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Unknown property ‘user-select’.  Declaration dropped." on resource://pdf.js/web/viewer.css because of whitelist item {"sourceName":"/web\\/viewer\\.css$/i","errorMessage":"/Unknown property.*(appearance|user-select)/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Ignored error "Expected media feature name but found ‘-moz-windows-default-theme’." on jar:file:///Users/cltbld/tasks/task_1531379587/build/application/Firefox%20Nightly.app/Contents/Resources/browser/features/formautofill@mozilla.org.xpi!/chrome/skin/windows/autocomplete-item.css because of whitelist item {"sourceName":"/\\b(autocomplete-item|svg)\\.css$/","errorMessage":"/Expected media feature name but found \\u2018-moz.*/i","isFromDevTools":false,"used":true}
00:17:28     INFO - Not taking screenshot here: see the one that was previously logged
00:17:28     INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | All the styles (205) loaded without errors. - Got 1, expected 0
00:17:28     INFO - Stack trace:
00:17:28     INFO - chrome://mochikit/content/browser-test.js:test_is:1305
00:17:28     INFO - chrome://mochitests/content/browser/browser/base/content/test/static/browser_parsable_css.js:checkAllTheCSS:443
00:17:28     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1103
00:17:28     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1094
00:17:28     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:996
00:17:28     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795
00:17:28     INFO - Leaving test bound checkAllTheCSS
00:17:28     INFO - GECKO(1993) | MEMORY STAT | vsize 4778MB | residentFast 780MB | heapAllocated 413MB
Flags: needinfo?(MattN+bmo)
Flags: needinfo?(MattN+bmo)
Attachment #8991520 - Attachment is obsolete: true
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 38

Last year
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/825671e0c5cc
Always hide View All, remove summary heading, add 'Pay to'. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/5ffc9a512843
Display the currency code with the total. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/d6174a81ff89
Fix default shippingAddressErrors value for debugging. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/e6f0e3fb69d6
Consistent PaymentRequest footer positioning with <payment-request-page>. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/8da83fe09309
PaymentRequest footer styling. r=sfoster

Updated

11 months ago
Duplicate of this bug: 1435111
Duplicate of this bug: 1429193
You need to log in before you can comment on or make changes to this bug.