diff --git a/app/assets/stylesheets/cards.css b/app/assets/stylesheets/cards.css index 2591332d9..19ab9740c 100644 --- a/app/assets/stylesheets/cards.css +++ b/app/assets/stylesheets/cards.css @@ -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; } } } } diff --git a/app/assets/stylesheets/notifications.css b/app/assets/stylesheets/notifications.css index 878eb25c5..0cf434da8 100644 --- a/app/assets/stylesheets/notifications.css +++ b/app/assets/stylesheets/notifications.css @@ -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; } } diff --git a/app/assets/stylesheets/panels.css b/app/assets/stylesheets/panels.css index a82dc1665..93e60b58e 100644 --- a/app/assets/stylesheets/panels.css +++ b/app/assets/stylesheets/panels.css @@ -15,6 +15,10 @@ } } + .panel--wide { + --panel-size: 60ch; + } + .panel--centered { --panel-border-size: 0; --panel-size: 100%; diff --git a/app/assets/stylesheets/trays.css b/app/assets/stylesheets/trays.css index 2352cb2b4..bb2538a7f 100644 --- a/app/assets/stylesheets/trays.css +++ b/app/assets/stylesheets/trays.css @@ -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 */ diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 4180655f5..29348493a 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -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 diff --git a/app/javascript/controllers/notifications_tray_controller.js b/app/javascript/controllers/notifications_tray_controller.js new file mode 100644 index 000000000..12077425b --- /dev/null +++ b/app/javascript/controllers/notifications_tray_controller.js @@ -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 + } +} diff --git a/app/views/notifications/_tray.html.erb b/app/views/notifications/_tray.html.erb index 274d81622..e3fa911ba 100644 --- a/app/views/notifications/_tray.html.erb +++ b/app/views/notifications/_tray.html.erb @@ -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" } %>
diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index 38ac87f52..2fa4e0078 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -13,7 +13,7 @@
-
+
<% if @unread.any? %>

New for you

@@ -30,7 +30,7 @@
-
+

Previously seen

diff --git a/app/views/notifications/trays/show.html.erb b/app/views/notifications/trays/show.html.erb index f61c52d51..a4700253b 100644 --- a/app/views/notifications/trays/show.html.erb +++ b/app/views/notifications/trays/show.html.erb @@ -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 %> + <% end %> diff --git a/test/system/smoke_test.rb b/test/system/smoke_test.rb index d899aa450..92cb287fd 100644 --- a/test/system/smoke_test.rb +++ b/test/system/smoke_test.rb @@ -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)}"