Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide useNotification hook #8405

Open
1 task
tay1orjones opened this issue Apr 14, 2021 · 11 comments
Open
1 task

Provide useNotification hook #8405

tay1orjones opened this issue Apr 14, 2021 · 11 comments
Labels
needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. planning: umbrella Umbrella issues, surfaced in Projects views proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 type: enhancement 💡

Comments

@tay1orjones
Copy link
Member

tay1orjones commented Apr 14, 2021

We offer a variety of notification styles but unfortunately leave much of the animation and logic around 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/react

Acceptance Criteria

#8405 (comment)

@tay1orjones tay1orjones changed the title useNotification hook Ship useNotification hook Apr 14, 2021
@tay1orjones tay1orjones removed the Epic label Apr 14, 2021
@tay1orjones
Copy link
Member Author

A few additional opportunities to look into with this:

  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.
  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?

@johnbister
Copy link
Contributor

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!

notification-stack.mp4

@johnbister
Copy link
Contributor

johnbister commented Jun 10, 2021

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.

STACK.HIDDEN.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.

NOTIFICATION.TIMEOUT.mp4

@mjabbink
Copy link

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.

@johnbister
Copy link
Contributor

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.

@mjabbink
Copy link

@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.

@tay1orjones tay1orjones added this to the 2022 Q2 milestone Mar 15, 2022
@tay1orjones tay1orjones added the planning: umbrella Umbrella issues, surfaced in Projects views label Mar 15, 2022
@abbeyhrt
Copy link
Contributor

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.

@tay1orjones tay1orjones removed this from the 2022 Q2 milestone Jun 17, 2022
@tay1orjones
Copy link
Member Author

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.

@cgirani
Copy link

cgirani commented Aug 4, 2022

Component from MAS Core team presented using the spec from carbon

ToastNotification.mov

Initial code implementation can be seen here

@dstran
Copy link

dstran commented Aug 18, 2022

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!

@cgirani
Copy link

cgirani commented Aug 25, 2022

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

@sstrubberg sstrubberg added type: enhancement 💡 proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. labels Jan 10, 2023
@tay1orjones tay1orjones changed the title Ship useNotification hook Provide useNotification hook Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. planning: umbrella Umbrella issues, surfaced in Projects views proposal: accepted This request has gone through triaging and we are accepting PR's against it. role: dev 🤖 type: enhancement 💡
Projects
Status: 📋 Backlog
Status: Later 🧊
Development

No branches or pull requests

7 participants