Closed Bug 1459253 Opened Last year Closed Last year

Hide the 'View All Items' button when there are no display items

Categories

(Firefox :: WebPayments UI, enhancement, P1, minor)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: MattN, Assigned: prathiksha)

References

Details

(Whiteboard: [webpayments])

Attachments

(1 file)

When there are no display items (or additional display items?) we should hide the button to show the order details view as it will provide no value to the user, only waste their time to click. UX agreed to this in the past.
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Priority: P2 → P1
Comment on attachment 8974210 [details]
Bug 1459253 - Hide the 'View All Items' button when there are no display items.

https://reviewboard.mozilla.org/r/242504/#review248390

Great start!

::: commit-message-59005:1
(Diff revision 1)
> +Bug 1459253 - Hide the 'View All Items' button when there are no display items. r?MattN

Please add a test to browser/components/payments/test/mochitest/test_payment_dialog.html

::: browser/components/payments/res/containers/payment-dialog.js:132
(Diff revision 1)
> +  /**
> +   * Determine if a display item should belong in the footer list.
> +   * This uses the proposed "type" property, tracked at:
> +   * https://github.com/w3c/payment-request/issues/163
> +   *
> +   * @param {object} item - Data representing a PaymentItem
> +   * @returns {boolean}
> +   */
> +  static isFooterItem(item) {
> +    return item.type == "tax";
> +  }

I don't think you should be separating between main and footer items in this file as it's not necessary for determining how many items there are.

::: browser/components/payments/res/containers/payment-dialog.js:272
(Diff revision 1)
>      let paymentDetails = request.paymentDetails;
>      this._hostNameEl.textContent = request.topLevelPrincipal.URI.displayHost;
>  
> +    let items = this._getMainListItems(state);
> +    let additionalItems = this._getAdditionalDisplayItems(state);
> +    this._viewAllButton.hidden =  !items.length && !additionalItems.length;

Nit: there is an extra space before the first `!`
Attachment #8974210 - Flags: review?(MattN+bmo)
Comment on attachment 8974210 [details]
Bug 1459253 - Hide the 'View All Items' button when there are no display items.

https://reviewboard.mozilla.org/r/242504/#review248702

Otherwise this looks good.

::: browser/components/payments/res/containers/payment-dialog.js:113
(Diff revision 3)
> +  _getMainListItems(state) {
> +    let request = state.request;
> +    let items = request.paymentDetails.displayItems;
> +    return items;
> +  }

I think you can inline this in `render` and don't need a helper for it:

```js
let {displayItems} = request.paymentDetails;
let additionalItems = this._getAdditionalDisplayItems(state);
this._viewAllButton.hidden = !displayItems.length && !additionalItems.length;
```
Attachment #8974210 - Flags: review?(MattN+bmo)
Comment on attachment 8974210 [details]
Bug 1459253 - Hide the 'View All Items' button when there are no display items.

https://reviewboard.mozilla.org/r/242504/#review248738

Thanks!
Attachment #8974210 - Flags: review?(MattN+bmo) → review+
Pushed by prathikshaprasadsuman@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/db808e8d92b4
Hide the 'View All Items' button when there are no display items. r=MattN
https://hg.mozilla.org/mozilla-central/rev/db808e8d92b4
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
You need to log in before you can comment on or make changes to this bug.