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

feat: notification component #553

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

kyr0
Copy link

@kyr0 kyr0 commented May 29, 2024

Hi @lucaslarroche

as discussed here: #552 I've built a Notification component for Pico.

Feel free to integrate it, if you think it's useful. It definitely helps the UX of my Pico-based frontend, and as it is fully spec-compliant and accessible, I think it's a nice addition.

Please let me know if you want me to change anything in the implementation.

Demo

Thank you for building Pico!

Best,
Aron

Relevant code quicklinks:

Impl:

@if map.get($modules, "components/notification") {

Settings:

"components/notification": true,

Import:

@use "components/notification"; // dialog[role="alert"]

@myisaak
Copy link

myisaak commented Jun 6, 2024

  1. What if we submit multiple notifications?
  2. Can one add default animation like we have already for the modal component?

@kyr0
Copy link
Author

kyr0 commented Jun 15, 2024

@myisaak

  1. This component is implemented to be a single notification, not a sonner component. Also, from an accessibility point of view, I wonder if "multiple notifications at the same time" is good UX. I understand your point, but a dialog with an alert role is meant to be a singleton, even in the HTML spec. I'd suggest to solve complex application logic questions like that using JavaScript. One could create a stack of notifications to show, and let them show one after another, each as a single notification. This would make sure that you could also "process" it with a screenreader and you have a chance for the attention span of an average person to actually understand multiple notifications in a row. If 3 things are happening at the same time, notifications show up at the same time and would disappear after n seconds, it's likely that people will miss at least one important information.

  2. The current transition that is set has the "default"

  transition: opacity var(--pico-transition);

...which seemed correct to me, but looking at the modal implementation, I guess it should not be set, and instead it should be: https://github.com/picocss/pico/blob/main/scss/components/_modal.scss#L132

@myisaak
Copy link

myisaak commented Jun 19, 2024

  1. Notification != Alert. Notifications should be stacked, alerts not. See: https://notifications.spec.whatwg.org/ vs https://www.w3.org/WAI/ARIA/apg/patterns/alertdialog/
  2. Also, you assume .showModal() over .show(), former sets the focus, obstructing the user's workflow. That shouldn't happen for unimportant notifications, but for alerts it should. E.g. "should you delete this repo?" vs "your login credentials was incorrect".

@kyr0 you had my idea of writing a spec-compliant notification component for picocss. You're headed in an interesting direction, similar to https://github.com/Ghosh/micromodal

Also good to consider:

  1. Notifications already standardized in browsers: https://notifications.spec.whatwg.org/
  2. UX gurus consider modals as obstructive: https://modalzmodalzmodalz.com

@kyr0
Copy link
Author

kyr0 commented Jun 20, 2024

@myisaak As far as I know, the Notification spec is about presenting an overlay over the browser using system notification APIs. I've implemented it a few times. This spec AFAIK hasn't anything to do with Notifications rendered inside of the browser's/tab's website viewport. The behaviour, wether it is replacing or stacking in nature is set by the browser by default and can often be overridden in systems settings (verified in macOS).

I agree with the obstructive nature of modals blocking the UI. I just didn't find a way to have the <dialog> element not behave like that. It is spec'ed that you can use <dialog> with role "alert" which resembles notification. That's what I meant when I referred to the spec.

I think that I didn't go in the direction of https://github.com/Ghosh/micromodal though. They are constructing obstructive modals using elements not meant for the purpose (<div> etc.) and are not using the standard <dialog> element. This implementation also has a JavaScript library dependency, while my implementation simply calls the native, spec'ed Dialog DOM API directly, vanilla, via inline JS, dependency-free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants