1463545 - Replace grid layout of <rich-option> subclasses with new UI spec

Status: VERIFIED FIXED

(Firefox :: WebPayments UI, enhancement, P1)

Description

[Matthew N. [:MattN]]

We no longer use a grid layout so we won't have holes in the UI for missing fields.

https://mozilla.invisionapp.com/share/SDFY4PA4EQ7#/screens/292978667

Comment 1

[Matthew N. [:MattN]]

We should also ensure we never show "null" for a field if it's missing in storage.

Comment 2

[Matthew N. [:MattN]]

It would be good to also update layout of the "Edit | Add" links (ordering, localized bar separator, and position) in a separate commit of this bug.

Comment 3

[:prathiksha]

Hi Eric. What information do we want to show in the closed and open state of the payment method picker specifically and in what order? Currently, we don't show the card type in closed state according to the spec here(https://mozilla.invisionapp.com/share/SDFY4PA4EQ7#/screens/304878910) but we show it in the open state. Should we keep it this way? :)

Comment 4

[:prathiksha]

(In reply to :prathiksha from comment #3)
> Hi Eric. What information do we want to show in the closed and open state of
> the payment method picker specifically and in what order? Currently, we
> don't show the card type in closed state according to the spec
> here(https://mozilla.invisionapp.com/share/SDFY4PA4EQ7#/screens/304878910)
> but we show it in the open state. Should we keep it this way? :)

Some other questions I have:

what label should we show in the open state <option> element of the contact information picker?

How do we handle missing fields in both closed state <rich-option> and open state <option> element labels in all the pickers?

Comment 5

[Eric Pang [:epang] UX]

(In reply to :prathiksha from comment #3)
> Hi Eric. What information do we want to show in the closed and open state of
> the payment method picker specifically and in what order? Currently, we
> don't show the card type in closed state according to the spec
> here(https://mozilla.invisionapp.com/share/SDFY4PA4EQ7#/screens/304878910)
> but we show it in the open state. Should we keep it this way? :)

Hi Prathiksha,

You can see both states on this page of the spec (i recently updated it)
https://mozilla.invisionapp.com/d/main#/console/13422080/304878939/preview

We are hoping to show the card image in the close state.  Ideally we would show the image in the open state as well, but since we can't we've used text.

Also, I used a this symbol ❘ for the division lines as per Jared suggestion.

Comment 6

[Eric Pang [:epang] UX]

(In reply to :prathiksha from comment #4)
> (In reply to :prathiksha from comment #3)
> > Hi Eric. What information do we want to show in the closed and open state of
> > the payment method picker specifically and in what order? Currently, we
> > don't show the card type in closed state according to the spec
> > here(https://mozilla.invisionapp.com/share/SDFY4PA4EQ7#/screens/304878910)
> > but we show it in the open state. Should we keep it this way? :)
> 
> Some other questions I have:
> 
> what label should we show in the open state <option> element of the contact
> information picker?
> 
> How do we handle missing fields in both closed state <rich-option> and open
> state <option> element labels in all the pickers?

let me check with Jacqueline on this and get back to you!

ni on myself as a reminder

Comment 7

[Eric Pang [:epang] UX]

Hi Prathiksha!

Sorry for the delayed response.
Here's what we should do for missing fields.

ixd:
https://mozilla.invisionapp.com/share/ABFIO5F9NPZ#/275361836_Screens_-_Shipping_Address_Dropdown

Visual
https://e-pang.github.io/Webpayments/#artboard0

I also pinged Brian to get his help with copy.  For now we can use Missing as a placeholder though.  We'll see what he says :).

Let me know if anything else is needed.
Thanks!

Comment 8

[:prathiksha]

(In reply to Eric Pang [:epang] UX from comment #7)

Ok, thanks Eric! :)

Comment 9

[:prathiksha]

Attachment: Bug 1463545 - Replace grid layout of <address-option> with a new two line design. r=sfoster

Replace grid layout of <rich-option> subclasses with new UI spec.

Comment 10

[Matthew N. [:MattN]]

Hey Prathiksha,

Since this bug wasn't clearly defined and after looking at your patch I think it would be best for me to implement a different approach that supports the "Missing …" text so we don't have to change approaches for that. Simply using .join(",") doesn't really work for that since we need to colour the missing fields a different colour.

There are some other small-to-medium sized better-defined bugs now that you could take if you're interested:
Bug 1490805 - Add the CVV security code field to the add card form and make it required in all places
Bug 1491020 - Down arrow from the drop downs on the summary page are missing
Bug 1491065 - First Time Use: Label Button Text
Bug 1470184 - Update the Preferences text and layout for Web Payments

Comment 11

[Matthew N. [:MattN]]

Attachment: Bug 1463545 - Copy stylesheets from the payment dialog into mochitests. r=jaws

Comment 13

[Matthew N. [:MattN]]

Attachment: Bug 1463545 - Overlay the selected <rich-option> on top of the native <select>. r=sfoster

* We get the shared colours (including hover) and dropmarker.
* Makes it harder to regress the clickable area of the <rich-select> since the problem will be visible.
* Hides the text for the closed state of the <select> so the <rich-option> text can be presented.

Depends on D6233

Comment 14

[Matthew N. [:MattN]]

Attachment: Bug 1463545 - Update the <basic-card-option> layout to match the UI spec. r=sfoster

Depends on D6330

Comment 15

[Matthew N. [:MattN]]

Attachment: Bug 1463545 - Use text-overflow:ellipsis; on <shipping-option>. r=sfoster

Depends on D5186

Comment 16

[Matthew N. [:MattN]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=70cd15728e7649e195bbbaade0b8dc01bba1e791

Comment 17

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

Comment on attachment 9010152 [details]
Bug 1463545 - Copy stylesheets from the payment dialog into mochitests. r=jaws

Jared Wein [:jaws] (please needinfo? me) has approved the revision.

Comment 19

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

Comment on attachment 9010448 [details]
Bug 1463545 - Overlay the selected <rich-option> on top of the native <select>. r=sfoster

Sam Foster [:sfoster] has approved the revision.

Comment 20

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

Comment on attachment 9010450 [details]
Bug 1463545 - Update the <basic-card-option> layout to match the UI spec. r=sfoster

Sam Foster [:sfoster] has approved the revision.

Comment 21

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

Comment on attachment 9010452 [details]
Bug 1463545 - Use text-overflow:ellipsis; on <shipping-option>. r=sfoster

Sam Foster [:sfoster] has approved the revision.

Comment 22

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

Comment on attachment 9007016 [details]
Bug 1463545 - Replace grid layout of <address-option> with a new two line design. r=sfoster

Sam Foster [:sfoster] has approved the revision.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7106b3f8d717
Copy stylesheets from the payment dialog into mochitests. r=jaws
https://hg.mozilla.org/integration/autoland/rev/913329feffe4
Overlay the selected <rich-option> on top of the native <select>. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/b6cc6d47b5be
Update the <basic-card-option> layout to match the UI spec. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/1f44117fee2e
Replace grid layout of <address-option> with a new two line design. r=sfoster
https://hg.mozilla.org/integration/autoland/rev/216c54f4650e
Use text-overflow:ellipsis; on <shipping-option>. r=sfoster

Comment 24

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/7106b3f8d717
https://hg.mozilla.org/mozilla-central/rev/913329feffe4
https://hg.mozilla.org/mozilla-central/rev/b6cc6d47b5be
https://hg.mozilla.org/mozilla-central/rev/1f44117fee2e
https://hg.mozilla.org/mozilla-central/rev/216c54f4650e

Comment 25

[Christoph Kerschbaumer [:ckerschb]]

Hey MattN,

I am refactoring SOP console error messages over within [1]. It seems that the refactoring of test_basic_card_option.html within this bug, in particular basic-card-option.js causes some SOP errors. Before 1490874 those were not reported as errors to the console, hence test_basic_card_option.html succeeded even though it shouldn't have. Here is the corresponding TRY run [3], can you please take a look?

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1490874#c14
[2] https://hg.mozilla.org/mozilla-central/rev/b6cc6d47b5be#l2.69
[3] https://treeherder.mozilla.org/#/jobs?repo=try&revision=02e9588f7bf7fe207ec180522582c2e25dc55994

Comment 26

[Matthew N. [:MattN]]

I replied in bug 1490874 comment 17.

Comment 27

[Hani Yacoub, Desktop QA]

I verified the following:

- Down arrow from the drop downs on the summary page are displayed.

- "null" is never shown for a field if it's missing in storage.

- Information displayed in a closed and open state in shipping address, shipping option and in contact information are matching the specs.
But the information displayed in payment method in the closed and open state aren't matching the spec, there is a missing line to separate information in payment method field.

- Ordering, localized bar separator and position of the "Edit | Add" links in billing address section aren't matching the specs.


I'm not sure if all the mentioned above were handled in this bug or not, or if there is anything else that I should verify in this bug. Matt could you please take a look at this?

Comment 28

[Matthew N. [:MattN]]

(In reply to Hani Yacoub from comment #27)
> I verified the following:
> 
> - Down arrow from the drop downs on the summary page are displayed.
> 
> - "null" is never shown for a field if it's missing in storage.

Is this also true for ones not added to permanent storage, i.e. ones where the user unchecked the save checkbox?

> - Information displayed in a closed and open state in shipping address,
> shipping option and in contact information are matching the specs.
> But the information displayed in payment method in the closed and open state
> aren't matching the spec, there is a missing line to separate information in
> payment method field.

I think you may be looking at an old spec. The separator lines were changed to instead have 3 spaces between items.

> - Ordering, localized bar separator and position of the "Edit | Add" links
> in billing address section aren't matching the specs.

That should be fixed by bug 1480717.

Comment 29

[Hani Yacoub, Desktop QA]

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #28)
> (In reply to Hani Yacoub from comment #27)
> > I verified the following:
> > 
> > - Down arrow from the drop downs on the summary page are displayed.
> > 
> > - "null" is never shown for a field if it's missing in storage.
> 
> Is this also true for ones not added to permanent storage, i.e. ones where
> the user unchecked the save checkbox?

I thought that I supposed to verify if any field in the summary page like "Contact Information" for example is missing information, a null drop down shouldn't be displayed. (please see contact info.png)
But I might be wrong about that.

Comment 30

[Hani Yacoub, Desktop QA]

Attachment: contact info.png

Comment 31

[Matthew N. [:MattN]]

No, I think you misunderstood. I'm saying to test with a temporary card and temporary address meaning that you uncheck the checkbox when adding it. You shouldn't see the string "null" or "undefined" in the UI anywhere anymore.

How were you able to get "None Selected" for the contact info picker? Was that by deleting records from Preferences while the dialog was open? If not, please file a bug with STR on that as you should have seen the FTU IIRC.

Comment 33

[Hani Yacoub, Desktop QA]

I tested with a temporary card and temporary address and I didn't see any "null" or "undefined" string in the UI.

I was able to get "None Selected" for the contact info picker, if for example the information needed is the email and that info wasn't required for the delivery address.
Can by accessed from this test page(https://rsolomakhin.github.io/pr/email/). 
should I proceed by filling a bug for that?

Another issue I noticed here that the down arrow and "edit" link are displayed even if the drop down in empty.

Comment 34

[Matthew N. [:MattN]]

(In reply to Hani Yacoub from comment #33)
> I tested with a temporary card and temporary address and I didn't see any
> "null" or "undefined" string in the UI.
> 
> I was able to get "None Selected" for the contact info picker, if for
> example the information needed is the email and that info wasn't required
> for the delivery address.
> Can by accessed from this test
> page(https://rsolomakhin.github.io/pr/email/). 
> should I proceed by filling a bug for that?

Ah, no, that's expected in that situation… I just wasn't think about the case where only an email was requested.

> Another issue I noticed here that the down arrow and "edit" link are
> displayed even if the drop down in empty.

* The down arrow is expected for now but I added it to the scope of bug 1478822.
* The edit link shouldn't appear if no option is selected, except on the billing address picker (will be fixed by bug 1482689) so if you can repro that then file a bug with STR please.

Thanks

Comment 35

[Hani Yacoub, Desktop QA]

I'll mark this as verified fixed.