carbon icon indicating copy to clipboard operation
carbon copied to clipboard

Ship useNotification hook

Open tay1orjones opened this issue 4 years ago • 9 comments
trafficstars

We offer a variety of notification styles but unfortunately leave much of the animation and logical round display notifications to the end-user. We would like to codify this behavior/logic into a single hook, useNotification, that would encapsulate this behavior and make it easier for product teams to display/render notifications in their UI. This may look like:

function MyComponent() {
  const send = useNotification();

  function sendNotification() {
    send({ title: 'This is the notification title' });
  }

  return <button type="button" onClick={sendNotification}>Send test notification</button>;
}

Packages impacted

  • carbon-components-react

tay1orjones avatar Apr 14 '21 20:04 tay1orjones

A few additional opportunities to look into with this:

  1. Improve the "escape to close" functionality proposed in https://github.com/carbon-design-system/carbon/pull/8560

    • Regarding multiple/stacked notifications, this hook could assist in keeping track of how many notifications have been rendered, and how to close them in order of appearance using esc.
  2. Keyboard navigation improvements

    • It would also be beneficial if focus could be handled to move from one notifcation to the next and also when closed via esc or the close button. The hook could potentially inform tab order.
  3. Notification Panel interop

    • Could this hook can assist with the development of the notification panel pattern by sending notification data to a central store when a notification is sent via the hook?

tay1orjones avatar May 11 '21 20:05 tay1orjones

Here’s a rough pass on how multiple inline notifications could stack and be dismissed. I kept it simple for now to make sure we like the overall mechanics.

@mjabbink @jeanservaas @tay1orjones Let me know if you have any feedback I should take into the next pass. Thanks!

https://user-images.githubusercontent.com/70543333/120850559-752c4300-c52c-11eb-9606-de8e26c54cbf.mp4

johnbister avatar Jun 04 '21 19:06 johnbister

I’m hoping to get some design feedback before moving forward. Let me know if you have any thoughts @jeanservaas @kingtraceyj

We might want to limit how many notifications are visible in the stack at a time. We could accomplish this by some kind of gradient on the maximum oldest notification being displayed. As newer notifications are dismissed, older ones become visible.

https://user-images.githubusercontent.com/70543333/121598410-a73f1880-c9f6-11eb-88a9-a0e1e51e84f9.mp4

Also, for any notifications that timeout by default (might be more relevant to the toast notifications) we could show the user that this notification will dismiss automatically by using the thick stroke on the left as a timer.

https://user-images.githubusercontent.com/70543333/121598485-b58d3480-c9f6-11eb-8742-52f01d9eda46.mp4

johnbister avatar Jun 10 '21 21:06 johnbister

I mentioned before but I did not her a response. The toolbar and button above the accordion rows also be the same height as the accordion rows? That is at least what the primary intent should be.

mjabbink avatar Jun 11 '21 01:06 mjabbink

Thanks Mike. Yes, the data table should have the 48px toolbar. This was just from an example I found in sketch to help explore how multiple notifications interact. I can make the toolbar change if it's too distracting, but I'm focused on tackling the notification issue.

johnbister avatar Jun 11 '21 14:06 johnbister

@johnbister

I like the updates you've done. I think the fade gradient can work and wonder what @jeanservaas thinks on the details of the gradient?

I like the addition of a taller notification in the stack to see what that looks like

I also think timeout bar is cool.

mjabbink avatar Jun 11 '21 16:06 mjabbink

Just a quick update:

We removed closeOnEscape from InlineNotification and ToastNotification. The workaround that was added so that all notifications weren't closed on escape had to be removed due to accessibility concerns with the close button. Due to this, the below notes definitely need to be addressed in Toast and Inline so we can bring back the closeOnEscape prop!

  1. Improve the "escape to close" functionality proposed in refactor(notification): improve accessibility #8560

    • Regarding multiple/stacked notifications, this hook could assist in keeping track of how many notifications have been rendered, and how to close them in order of appearance using esc.
  2. Keyboard navigation improvements

    • It would also be beneficial if focus could be handled to move from one notifcation to the next and also when closed via esc or the close button. The hook could potentially inform tab order.

abbeyhrt avatar Mar 18 '22 22:03 abbeyhrt

An additional requirement for this is that it would ideally respect the user's prefers-reduced-motion setting. We might need to consider how the animations would or would not be different or there at all when a user prefers reduced motion.

tay1orjones avatar Aug 04 '22 18:08 tay1orjones

Component from MAS Core team presented using the spec from carbon

https://user-images.githubusercontent.com/22034430/182949650-e364702a-9b64-4bd4-a357-6ab6905934dd.mov

Initial code implementation can be seen here

cgirani avatar Aug 04 '22 20:08 cgirani

Any thoughts of when this will be available for use, even in a pre-release version? Note that I cannot see the code. Thank you for the amazing work!

dstran avatar Aug 18 '22 15:08 dstran

Hi @dstran I'm working on this, should have some initial code for carbon team to review soon

cgirani avatar Aug 25 '22 13:08 cgirani