Hide the "edit" link in the pickers when no options are selected

VERIFIED FIXED in Firefox 63

Status

()

defect
P1
normal
VERIFIED FIXED
11 months ago
9 months ago

People

(Reporter: MattN, Assigned: prathiksha)

Tracking

(Blocks 1 bug)

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

Firefox Tracking Flags

(firefox63 verified)

Details

(Whiteboard: [webpayments] [user-testing])

Attachments

(1 attachment)

See the top left of
https://mozilla.invisionapp.com/share/SDFY4PA4EQ7#/screens/292978667

In practice with the FTU this would only happen if the user deletes the address/card from prefs while the dialog is open.
Blocks: 1446577
No longer depends on: 1463545
Priority: P2 → P3
Whiteboard: [webpayments] → [webpayments-reserve]
This case will also happen when there is no onboarding after bug 1443735. Eric said today that we should hide "Edit" (and therefore the separator I assume) when no shipping address is selected by default.
Blocks: 1443735
Summary: Show an "add" link and better fallback text inside pickers/<rich-select> when there are no options → Hide the "edit" link and show better fallback text inside pickers/<rich-select> when there are no options
Whiteboard: [webpayments-reserve] → [webpayments-reserve] [user-testing]
Priority: P3 → P2
Whiteboard: [webpayments-reserve] [user-testing] → [webpayments] [user-testing]
(Assignee)

Updated

9 months ago
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Priority: P2 → P1
(Assignee)

Comment 2

9 months ago
Filed Bug 1478822 to keep track of setting the fallback text in pickers when no option is selected.
Summary: Hide the "edit" link and show better fallback text inside pickers/<rich-select> when there are no options → Hide the "edit" link in the pickers when no options are selected
Comment hidden (mozreview-request)
(Reporter)

Comment 4

9 months ago
mozreview-review
Comment on attachment 8995358 [details]
Bug 1463547 - Hide the edit link in the pickers when no options are selected.

https://reviewboard.mozilla.org/r/259814/#review266828

Please add a check of the visibility for the address picker in test_initialState in test_payment_dialog.html and a more-specific test for a picker in test_empty in test_address_picker.html.

::: browser/components/payments/res/containers/payment-method-picker.js:69
(Diff revision 1)
>      if (selectedPaymentCardGUID && selectedPaymentCardGUID !== this.dropdown.value) {
>        throw new Error(`The option ${selectedPaymentCardGUID} ` +
>                        `does not exist in the payment method picker`);
>      }
> +
> +    super.render(state);

Please add these calls to all of the other subclasses too just so they don't get missed in future `render` improvements.

::: browser/components/payments/res/containers/rich-picker.js:53
(Diff revision 1)
>        this.labelElement.textContent = newValue;
>      }
>    }
> +
> +  render(state) {
> +    this.editLink.hidden = !this.dropdown.popupBox.value;

Nit: `!this.dropdown.value` (get rid of `popupBox`) since the picker shouldn't rely on the popupBox implementation detail
Attachment #8995358 - Flags: review?(MattN+bmo)
Comment hidden (mozreview-request)
(Assignee)

Comment 6

9 months ago
mozreview-review-reply
Comment on attachment 8995358 [details]
Bug 1463547 - Hide the edit link in the pickers when no options are selected.

https://reviewboard.mozilla.org/r/259814/#review266828

> Please add these calls to all of the other subclasses too just so they don't get missed in future `render` improvements.

As we discussed, I did not add this line to the render function of shipping-option-picker.js as it breaks the visibility of the edit link for that picker.
(Reporter)

Comment 7

9 months ago
mozreview-review
Comment on attachment 8995358 [details]
Bug 1463547 - Hide the edit link in the pickers when no options are selected.

https://reviewboard.mozilla.org/r/259814/#review266838

Thanks!

::: browser/components/payments/test/mochitest/test_address_picker.html:36
(Diff revision 2)
>  /** Test the address-picker component **/
>  
>  import "../../res/containers/address-picker.js";
>  
>  let picker1 = document.getElementById("picker1");
> +let editLink = picker1.querySelector(".edit-link");

Nit: You could do `picker1.editLink` instead.
Attachment #8995358 - Flags: review?(MattN+bmo) → review+
Comment hidden (mozreview-request)
Flags: qe-verify+
QA Contact: hani.yacoub
Whiteboard: [webpayments] [user-testing] → [webpayments-reserve] [user-testing]
Whiteboard: [webpayments-reserve] [user-testing] → [webpayments] [user-testing]
Can this land?
Flags: needinfo?(MattN+bmo)
The try push looked Green so landing now.
Flags: needinfo?(MattN+bmo)

Comment 11

9 months ago
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s eff0f0de010c46a461a131421e0b8ad28997fc3a -d 8b2d475dff2c: rebasing 476496:eff0f0de010c "Bug 1463547 - Hide the edit link in the pickers when no options are selected. r=MattN" (tip)
merging browser/components/payments/res/containers/address-picker.js
merging browser/components/payments/res/containers/payment-method-picker.js
merging browser/components/payments/res/containers/rich-picker.js
warning: conflicts while merging browser/components/payments/res/containers/rich-picker.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 12

9 months ago
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7893e7f4bb5
Hide the edit link in the pickers when no options are selected. r=MattN

Comment 13

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b7893e7f4bb5
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63

Comment 14

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

Verified - Fixed on the latest Nightly 63.0a1 (2018-08-06) on Windows 10 x64, Ubuntu 16.04, Mac OS 10.13, Windows 7 x64.
The "Edit" link is hidden if there is no option selected for the address.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.