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

VERIFIED FIXED in Firefox 64

Status

()

enhancement
P1
normal
VERIFIED FIXED
Last year
9 months ago

People

(Reporter: MattN, Assigned: MattN)

Tracking

(Depends on 3 bugs, Blocks 1 bug)

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

Firefox Tracking Flags

(firefox64 verified)

Details

(Whiteboard: [webpayments])

User Story

* Make the internal layout of all RichOption subclasses match the visual spec.

Attachments

(6 attachments)

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
We should also ensure we never show "null" for a field if it's missing in storage.
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Priority: P2 → P1
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.
User Story: (updated)
User Story: (updated)
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? :)
Flags: needinfo?(epang)
(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?
(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.
Flags: needinfo?(epang)
(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
Flags: needinfo?(epang)
Whiteboard: [webpayments] → [webpayments-reserve]
Flags: qe-verify?
Whiteboard: [webpayments-reserve] → [webpayments]
Flags: qe-verify? → qe-verify+
QA Contact: hani.yacoub
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!
Flags: needinfo?(epang) → needinfo?(prathikshaprasadsuman)
(In reply to Eric Pang [:epang] UX from comment #7)

Ok, thanks Eric! :)
Flags: needinfo?(prathikshaprasadsuman)
Whiteboard: [webpayments] → [webpayments-reserve]
Replace grid layout of <rich-option> subclasses with new UI spec.
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
Assignee: prathikshaprasadsuman → MattN+bmo
Attachment #9007016 - Attachment description: Bug 1463545 - Replace grid layout of <rich-option> subclasses with new UI spec. r?MattN → Bug 1463545 - Replace grid layout of <rich-option> subclasses with new UI design
Whiteboard: [webpayments-reserve] → [webpayments]
Duplicate of this bug: 1491020
* 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
Attachment #9007016 - Attachment description: Bug 1463545 - Replace grid layout of <rich-option> subclasses with new UI design → Bug 1463545 - Replace grid layout of <address-option> with a new two line design. r=sfoster
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.
Attachment #9010152 - Flags: review+
Duplicate of this bug: 1492140
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.
Attachment #9010448 - Flags: review+
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.
Attachment #9010450 - Flags: review+
Comment on attachment 9010452 [details]
Bug 1463545 - Use text-overflow:ellipsis; on <shipping-option>. r=sfoster

Sam Foster [:sfoster] has approved the revision.
Attachment #9010452 - Flags: review+
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.
Attachment #9007016 - Flags: review+
Pushed by mozilla@noorenberghe.ca:
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
Depends on: 1493216
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
Flags: needinfo?(MattN+bmo)
I replied in bug 1490874 comment 17.
Flags: needinfo?(MattN+bmo)
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?
Flags: needinfo?(MattN+bmo)
(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.
Flags: needinfo?(MattN+bmo)
(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.
Flags: needinfo?(MattN+bmo)
Posted image contact info.png
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.
Flags: needinfo?(MattN+bmo)
Duplicate of this bug: 1494449
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.
Flags: needinfo?(MattN+bmo)
(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
Flags: needinfo?(MattN+bmo)
I'll mark this as verified fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.