1468153 - PaymentRequest dialog <button> strings

Status: VERIFIED FIXED

(Firefox :: WebPayments UI, enhancement, P1)

Description

[Matthew N. [:MattN]]

My understanding is that we're supposed to use Photon styles[1] but those are usually only used in-content currently and are implemented in the in-content stylesheets.

Should we import all in-content styles (think prefs, error pages, etc.) or move buttons styles to their own stylesheet to share? Note that we don't use Photon button styles in any other dialog e.g. Master Password, window close confirmation, etc. Perhaps we shouldn't be using Photon buttons and instead use native styles to match other dialogs? Using the in-content stylesheet also affects the background colour of the dialog I think.

[1] https://design.firefox.com/photon/components/buttons.html

Comment 1

[Eric Pang [:epang] UX]

(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #0)
> My understanding is that we're supposed to use Photon styles[1] but those
> are usually only used in-content currently and are implemented in the
> in-content stylesheets.
> 
> Should we import all in-content styles (think prefs, error pages, etc.) or
> move buttons styles to their own stylesheet to share? Note that we don't use
> Photon button styles in any other dialog e.g. Master Password, window close
> confirmation, etc. Perhaps we shouldn't be using Photon buttons and instead
> use native styles to match other dialogs? Using the in-content stylesheet
> also affects the background colour of the dialog I think.
> 
> [1] https://design.firefox.com/photon/components/buttons.html

Hey Matt, think we already covered this last week in person, but thought I would respond so it's here too.
We should follow the in-context styles (like prefs). 

The reason behind this is because the web payments panel should feel like it's part of the chrome and not the platform it's on.
For now it's odd since it's in a modal (on windows) but in the future we should have it more integrated to feel like part of the browser.

Comment 2

[Matthew N. [:MattN]]

Oops, thanks. This ended up getting implemented in bug 1466720 so I will morph this bug to cover the updated button strings.

Comment 3

[Matthew N. [:MattN]]

We can use this bug to handle the button ordering like we discussed the other day. Can you clear the needinfo once you figure out what ordering we should use for the various states:
* onboarding wizard
* payment summary page
* add/edit screens after coming from the summary page

Consider that in Firefox the button order generally follows the platform conventions so, for example, on Windows the primary/affirmative button is normally on the left[1] when there is two buttons (both right-aligned) whereas it's on the right for macOS.

[1] https://docs.microsoft.com/en-us/windows/desktop/uxguide/win-dialog-box

Comment 4

[Matthew N. [:MattN]]

I'll move the ordering to a separate bug since it can be done in M4.

Comment 5

[Eric Pang [:epang] UX]

Attachment: Screens - Buttons.png

Here's the ixd for the buttons.
If needed the visuals can be found here:
https://mozilla.invisionapp.com/d/main#/console/13422080/305451650/preview

Comment 6

[Eric Pang [:epang] UX]

Hey Matt, did you open a new bug for ordering?  I posted the spec here, but can move it over if needed. Thanks!

Comment 7

[Matthew N. [:MattN]]

Hey Eric, we moved the ordering to bug 1478176 as we felt that wasn't needed for user testing.

Comment 8

[Matthew N. [:MattN]]

QA can verify that all the strings match Eric's spec.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Eric Pang [:epang] UX from comment #5)
> If needed the visuals can be found here:
> https://mozilla.invisionapp.com/d/main#/console/13422080/305451650/preview

Hey Eric, the link above doesn't work for me. Can you try to re-share it?

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Bug 1468153 - Update the edit strings (Add/Update) and the failure strings to use the hostname.

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

Comment 11

[Eric Pang [:epang] UX]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> (In reply to Eric Pang [:epang] UX from comment #5)
> > If needed the visuals can be found here:
> > https://mozilla.invisionapp.com/d/main#/console/13422080/305451650/preview
> 
> Hey Eric, the link above doesn't work for me. Can you try to re-share it?

give this a try, I'll also add a screenshot.  The link is best though you can click to see the spacing spec;
https://mozilla.invisionapp.com/share/S5FXRY0D8TZ#/305451650_9-1_UI_Components

Comment 12

[Eric Pang [:epang] UX]

Attachment: Buttons Ordering - Visuals

Comment 13

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8996030 [details]
Bug 1468153 - Update the edit strings (Add/Update) and the failure strings to use the hostname.

https://reviewboard.mozilla.org/r/260300/#review267662

Looks great. Thanks for hooking up the error page strings and hostname stuff.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/946f0a8fe89e
Update the edit strings (Add/Update) and the failure strings to use the hostname. r=sfoster

Comment 15

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/946f0a8fe89e

Comment 16

[Timea Cernea [:tbabos][inactive]]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0

Add/Update button:

Verified fixed, the buttons are correctly displayed and functional in the Add/Edit screens (Windows 10 x64, Ubuntu 16.04, Mac OS 10.13)
Worth mentioning that if "Edit" is clicked while there is no shipping address selected, in the Edit Shipping Adress screen, the "Add" button appears instead of "Update". I believe that this will be solved once with Bug 1463547.

Failure strings:

The Failure screen (paymnet fail) has the "Check your credit card has not expired." suggestion that should be corrected to "Check if your credit card has not expired." or "Check the expiration date on your credit card.". 
Otherwise, it is verified fixed on all OS.
 
Waiting for confimration from Eric regarding the mentioned string before any bug status changes.

Comment 17

[Timea Cernea [:tbabos][inactive]]

Actually, based on spec doc it should be "Check that your credit card is not expired".

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Timea Babos from comment #17)
> Actually, based on spec doc it should be "Check that your credit card is not
> expired".

Can you please file a new bug for this and needinfo me?

Comment 19

[Timea Cernea [:tbabos][inactive]]

Here it is: https://bugzilla.mozilla.org/show_bug.cgi?id=1480707

Since I filed a new bug for it, I will close this issue as Verified - Fixed.