commercetools / merchant-center-application-kit

Tools and components for developing Merchant Center Customizations 🛠
https://docs.commercetools.com/merchant-center-customizations
MIT License
67 stars 27 forks source link

Toast Notification position #3537

Open antoniolodias opened 5 months ago

antoniolodias commented 5 months ago

Describe the issue

Toast notification seems to be attached to page top instead of screen top, so if you scroll down and click something that gives some kind of notification, this won't be seen

To Reproduce

1. If you look before PR merge: https://mc-16889.mc-preview.europe-west1.gcp.escemo.com/blt-nail/orders/new/order-details otherwise: https://mc.europe-west1.gcp.escemo.com/blt-nail/orders/new/order-details

  1. set a small screen
  2. Pick USD currency
  3. Pick customer SUPPORT-25993@test.com
  4. Search for SKU 1200109660
  5. Click “Add” to add product to cart
  6. Scroll all the way down
  7. Click “Next”
  8. On shipping method page, click "Next"
  9. Error: nothing happens
  10. Click a few more times and you will notice toast notifications coming from the top

Expected behaviour

Toast notification should be always visible regardless of your scroll position

recording

https://github.com/commercetools/merchant-center-application-kit/assets/37903776/38711ab3-09f8-4be7-b5a2-fd8e2152f24a

kark commented 5 months ago

Hi 👋, Thank you for reporting this issue. To clarify, our notification system offers three types of notifications: global, page, and side notifications.

According to our documentation:

Notifications are grouped by so-called domains. A domain specifies where the notification should be displayed on the page (for instance page and side).

Therefore, this choice of positioning of the side-domain notifications container in the page (instead of fixed positioning against the viewport) appears to be intentional.

Please mind about the bits:

Be mindful of your users when dispatching notifications. Excessive and confusing notifications can lead to a poor user experience. Generally, error notifications should be displayed in the page domain rather than the side domain.

I’m tagging @FilPob to see if we need to revisit any aspects of this.

antoniolodias commented 5 months ago

Hi there @kark 👋 Thanks for looking into this!

The Global type might work, thank you for pointing that out, but at the same time I don't understand why would we want to trigger a side toast notification that is not visible.

kark commented 5 months ago

Hey @antoniolodias 👋,

I don't understand why would we want to trigger a side toast notification that is not visible

Thanks for pointing that out, that's a fair point. We definitely don't want that to happen. But we need to look into the tradeoffs of orienting side and page notifications against the viewport. For instance, users with smaller viewports won't be able to see bottom notifications because they wouldn't have the option to scroll, unlike they are able to do now.

image

We'll investigate this further.

Thanks for bringing it up!

antoniolodias commented 5 months ago

Thank you @kark

I thought the solution could be setting the notifications' container with position fixed instead of relative and allow scroll if needed. But I can be over simplifying it... The UIUX team might have some guidance as well. Thank you again for looking into this!!