-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Closes #20301: Add "Dismiss all" action to notifications dropdown #20612
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
base: main
Are you sure you want to change the base?
Closes #20301: Add "Dismiss all" action to notifications dropdown #20612
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pheus This works well and after a lot of thought about whether "Clear" was the right terminology (as opposed to "Mark Read" or "Dismiss"), I think probably "Clear" is clear enough, though maybe "Dismiss" is a little more consistent. The issue I have is that our existing terminology isn't very consistent — we use "Delete Selected" for the bulk delete button, and "dismiss" for the URL path, and then we muddle things up with the "read" path for when you click on the notification to view the object it pertains to, which makes it no longer "new" (a dirty badge), but doesn't remove it from your list of notifications, but also doesn't have any differentiator between it and an "unread" notification (well, it's shown in a slightly dimmed gray). "Clear" is a new terminology that has no precedent, but then most other terminology we have is implicit at best. So I'm okay with "Clear" because it obviously does "the right thing" as far as what the user is most likely to want.
I did notice one funny corner case: if you are viewing the Notifications page and you use the "Clear All" button to dismiss all unread notifications, those notifications stay in the list (because the action was done through HTMX and does not repaint the page). Then if you click on the trash can icon for any of the now-deleted notifications, or use the "Delete Selected" button, you get a 404 error.
Could you please give that some thought and see if there's a simple way to avoid that behavior, maybe by forcing a refresh if you're viewing the Notifications page? Or maybe the right answer is it isn't worth trying to prevent that, depending on how much effort it involves.
Thanks!
|
Thanks for the thoughtful review! Terminology Corner case Thanks again! |
|
Quick design question about the dropdown UX: The bell dropdown shows up to 10 unread items, but the new “Dismiss all” currently clears all unread notifications for the user. To avoid surprises, what’s your preference?
Happy to implement whichever aligns best with NetBox’s UX. Thanks! |
|
I would prefer option A. Seems like the most common use case IMO. |
Introduce a view to allow users to dismiss all unread notifications with a single action. Update the notifications' template to include a "Dismiss all" button for enhanced usability. This addition streamlines notification management and improves the user experience. Fixes netbox-community#20301
d8627e7 to
8eaff9d
Compare
| from django.db.models import Count, Q | ||
| from django.http import HttpResponseBadRequest, HttpResponseForbidden, HttpResponse | ||
| from django.http import HttpResponseBadRequest, HttpResponseForbidden, HttpResponse, Http404 | ||
| from django.shortcuts import get_object_or_404, redirect, render |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: This is an unrelated fix. Http404 is used, but not imported.
| return render(request, 'htmx/notifications.html', { | ||
| 'notifications': request.user.notifications.unread(), | ||
| 'notifications': request.user.notifications.unread()[:10], | ||
| 'total_count': request.user.notifications.count(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During manual testing, I've noticed that the dropdown menu does not actually limit the notification.


Fixes: #20301
Add a “Dismiss all” action to the bell dropdown that clears all unread notifications for the current user via a GET endpoint. The dropdown continues to show up to the most recent 10 unread items, and now includes a clear, i18n’d confirmation message that shows the total number of unread notifications affected.
Summary of changes
New view:
NotificationDismissAllViewregistered with@register_model_view(Notification, name='dismiss_all', path='dismiss-all', detail=False); deletes unread notifications forrequest.user.HX-Redirectto repaint the page; otherwise return the existing dropdown partial.Dropdown partial context: Now passes only the top 10 unread notifications plus counts:
notifications = request.user.notifications.unread()[:10],total_count = request.user.notifications.count(),unread_count = request.user.notifications.unread().count(). :contentReference[oaicite:1]{index=1}Template update (
htmx/notifications.html):Adds a “Dismiss all” button in the dropdown header when there are unread items, with a pluralized confirmation that shows the total unread count (e.g., “Dismiss 37 unread notifications?”).
HTMX utility helpers:
Introduce
htmx_current_url()andhtmx_maybe_redirect_current_page()to detect the current page (viaHX-Current-URL) and issue anHX-Redirectwhen appropriate. Both are now used by the single‑itemdismissview and the newdismiss-allview to avoid stale rows on the Notifications page. :contentReference[oaicite:3]{index=3}Screenshots
Screenshots (notifications present)
Screenshots (no notifications)
Notes for reviewers
dismiss-all).