Closed Bug 1469464 Opened 7 years ago Closed 7 years ago

Update PaymentRequest header and footer match the visual specs

Categories

(Firefox :: WebPayments UI, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: MattN, Assigned: MattN)

References

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 files, 1 obsolete file)

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 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 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 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 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 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+
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.
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 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]
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 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 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+
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
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: