bterkuile/fizzy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #1448 from basecamp/unique-notifications

Browse Source 
Group notifications by card and only show the oldest one in the tray
This commit is contained in:
Jorge Manrubia
2025-11-13 09:42:21 +01:00
committed by GitHub
parent c49a96a659 8f4b6926f5
commit 89ff019360
10 changed files with 128 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/assets/stylesheets/cards.css
+12 -24
View File
@@ -464,43 +464,31 @@
}
.card__notification-unread-indicator {
--btn-background: var(--color-canvas);
--btn-background: var(--color-marker);
--btn-border-color: var(--color-canvas);
--btn-icon-size: 0.75em;
--btn-color: var(--color-ink-inverted);
--btn-icon-size: 0.5em;
--btn-padding: 0;
--btn-size: 1.25em;
--btn-size: 1.6em;
font-size: var(--text-xx-small);
font-weight: 600;
margin: 2px;
position: relative;
z-index: 1;
&:before,
.icon {
transition: opacity 150ms ease;
}
/* The red dot */
&:before {
aspect-ratio: 1;
background-color: var(--color-marker);
border-radius: 50%;
content: "";
inline-size: 50%;
inset: 50% auto auto 50%;
position: absolute;
translate: -50% -50%;
}
/* The X */
.icon {
opacity: 0;
transition: opacity 150ms ease;
}
@media (hover: hover) {
.card:hover & {
--btn-background: transparent;
--btn-background: var(--color-ink-lightest);
--btn-color: var(--color-ink);
&:before { opacity: 0; }
.icon { opacity: 1; }
.badge-count { opacity: 0; }
.icon { opacity: 1; }
}
}
}
app/assets/stylesheets/notifications.css
+5 -7
View File
@@ -29,10 +29,6 @@
column-gap: var(--inline-space-half);
}
.card__notification-unread-indicator {
font-size: var(--text-small);
}
&:has(.card--notification) {
.notifications-list__empty-message {
display: none;
@@ -54,12 +50,14 @@
.card__notification-unread-indicator {
--btn-background: transparent;
--btn-size: 1.8em;
&:before {
opacity: 0;
}
margin: 2px;
.icon {
block-size: 1.7em;
color: var(--color-ink);
inline-size: 1.7em;
opacity: 1;
}
}
app/assets/stylesheets/panels.css
+4
View File
@@ -15,6 +15,10 @@
}
}
.panel--wide {
--panel-size: 60ch;
}
.panel--centered {
--panel-border-size: 0;
--panel-size: 100%;
app/assets/stylesheets/trays.css
-5
View File
@@ -550,11 +550,6 @@
}
}
.card__notification-unread-indicator {
--btn-icon-size: 1.25em;
--btn-size: 2em;
}
/* On mobile… */
@media (max-width: 799px) {
/* …add a border */
app/helpers/notifications_helper.rb
+12 -10
View File
@@ -20,14 +20,17 @@ module NotificationsHelper
end
def notification_tag(notification, &)
tag.div id: dom_id(notification), class: "tray__item tray__item--notification", data: { navigable_list_target: "item" } do
concat(
link_to(notification,
class: [ "card card--notification", { "card--closed": notification.card.closed? }, { "unread": !notification.read? } ],
data: { turbo_frame: "_top", badge_target: "unread", action: "badge#update dialog#close" },
style: { "--card-color:": notification.card.color },
&)
)
tag.div id: dom_id(notification), class: "tray__item tray__item--notification", data: {
navigable_list_target: "item",
notifications_tray_target: "notification",
card_id: notification.card.id,
timestamp: notification.created_at.to_i
} do
link_to(notification,
class: [ "card card--notification", { "card--closed": notification.card.closed? }, { "unread": !notification.read? } ],
data: { turbo_frame: "_top", badge_target: "unread", action: "badge#update dialog#close" },
style: { "--card-color:": notification.card.color },
&)
end
end
@@ -40,7 +43,6 @@ module NotificationsHelper
data: { action: "form#submit:stop badge#update:stop", form_target: "submit" },
form: { data: { controller: "form" } } do
concat(icon_tag("unseen"))
concat(tag.span("Mark as unread", class: "for-screen-reader"))
end
else
button_to notification_reading_path(notification),
@@ -49,7 +51,7 @@ module NotificationsHelper
data: { action: "form#submit:stop badge#update:stop", form_target: "submit" },
form: { data: { controller: "form" } } do
concat(icon_tag("remove"))
concat(tag.span("Mark as read", class: "for-screen-reader"))
concat(tag.span("1", class: "badge-count", data: { group_count: "" }))
end
end
end
app/javascript/controllers/notifications_tray_controller.js
+89
View File
@@ -0,0 +1,89 @@
import { Controller } from "@hotwired/stimulus"
export default class extends Controller {
static targets = [ "notification", "hiddenNotifications" ]
static classes = [ "grouped" ]
connect() {
this.group()
}
group() {
const notificationsByCardId = this.#groupNotificationsByCardId()
for (const cardId in notificationsByCardId) {
const notifications = notificationsByCardId[cardId]
if (notifications.length > 1) {
this.#renderGroup(notifications)
}
}
this.grouped = true
}
notificationTargetConnected(notification) {
if (this.grouped && notification.parentElement !== this.hiddenNotificationsTarget) {
this.#groupNotification(notification)
}
}
#groupNotificationsByCardId() {
const notificationsByCardId = {}
this.notificationTargets.forEach(notification => {
const cardId = notification.dataset.cardId
notificationsByCardId[cardId] ||= []
notificationsByCardId[cardId].push(notification)
})
return notificationsByCardId
}
#groupNotification(notification) {
const groupedNotifications = this.#groupedNotificationsFor(notification)
if (groupedNotifications.length > 1) {
this.#renderGroup(groupedNotifications)
}
}
#groupedNotificationsFor(notification) {
const cardId = notification.dataset.cardId
return this.notificationTargets
.filter(notification => notification.dataset.cardId === cardId)
}
#renderGroup(groupedNotifications) {
groupedNotifications.sort((notification_1, notification_2) => parseInt(notification_1.dataset.timestamp) - parseInt(notification_2.dataset.timestamp))
this.#showAsGrouped(groupedNotifications[0], groupedNotifications.length)
groupedNotifications.slice(1).forEach(notification => this.#hideInGroup(notification))
}
#showAsGrouped(notification, size) {
notification.classList.add(this.groupedClass)
this.#showGroupedNotification(notification)
this.#setGroupCount(notification, size)
}
#hideInGroup(notification) {
this.#hideGroupedNotification(notification)
this.#setGroupCount(notification, "")
}
// We use a hidden container instead of hiding the notifications directly so that the CSS that sort the
// tray element indexes doesn't get messed up with the child positions changing.
#showGroupedNotification(notification) {
if (notification.parentElement === this.hiddenNotificationsTarget) {
this.hiddenNotificationsTarget.parentElement.insertBefore(notification, this.hiddenNotificationsTarget)
}
}
#hideGroupedNotification(notification) {
this.hiddenNotificationsTarget.appendChild(notification)
}
#setGroupCount(notification, count) {
notification.querySelector("[data-group-count]").textContent = count
}
}
app/views/notifications/_tray.html.erb
+1 -1
View File
@@ -7,7 +7,7 @@
data-navigable-list-actionable-items-value="true"
data-navigable-list-reverse-navigation-value="true"
data-action="keydown->navigable-list#navigate dialog:show@document->navigable-list#reset turbo-visit->navigable-list#reset keydown.esc->dialog#close:stop click@document->dialog#closeOnClickOutside">
<%= turbo_frame_tag "notifications", src: tray_notifications_path, refresh: "morph" %>
<%= turbo_frame_tag "notifications", src: tray_notifications_path, refresh: "morph", data: { controller: "notifications-tray", notifications_tray_grouped_class: "notification--grouped", action: "turbo:frame-render->notifications-tray#group" } %>
<div class="tray__item tray__item--hat txt-x-small gap-half">
<div data-navigable-list-target="item" class="full-width">
app/views/notifications/index.html.erb
+2 -2
View File
@@ -13,7 +13,7 @@
<div data-controller="badge navigable-list" data-badge-unread-class="unread" data-action="keydown@document->navigable-list#navigate" data-navigable-list-actionable-items-value="true"
data-navigable-list-focus-on-selection-value="false">
<section class="notifications-list panel center borderless unpad flex flex-column gap-half">
<section class="notifications-list panel panel--wide center borderless unpad flex flex-column gap-half">
<% if @unread.any? %>
<div class="flex align-center justify-space-between margin-block-start margin-block-end-half">
<h2 class="txt-medium txt-uppercase txt-alert">New for you</h2>
@@ -30,7 +30,7 @@
</div>
</section>
<section class="notifications-list notifications-list--read panel center borderless unpad flex flex-column gap-half margin-block-start">
<section class="notifications-list notifications-list--read panel panel--wide center borderless unpad flex flex-column gap-half margin-block-start">
<h2 class="txt-medium margin-block-start-double margin-block-end-half txt-uppercase translucent">Previously seen</h2>
<div id="notifications_list_read" contents>
app/views/notifications/trays/show.html.erb
+2 -1
View File
@@ -1,3 +1,4 @@
<%= turbo_frame_tag "notifications" do %>
<%= turbo_frame_tag "notifications", data: { controller: "notifications-tray", notifications_tray_grouped_class: "notification--grouped" } do %>
<%= render partial: "notifications/notification", collection: @notifications, cached: true %>
<div hidden data-notifications-tray-target="hiddenNotifications"></div>
<% end %>
test/system/smoke_test.rb
+1 -1
View File
@@ -38,7 +38,7 @@ class SmokeTest < ApplicationSystemTestCase
test "dismissing notifications" do
sign_in_as(users(:david))
notif = notifications(:logo_card_david_mention_by_jz)
notif = notifications(:logo_comment_david_mention_by_jz)
assert_selector "div##{dom_id(notif)}"