From cfbb7fc94ab90ac50ff6868c1a14ac61a2431a14 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 13:45:31 -0400 Subject: [PATCH 01/11] Add desktop pop-up notifications (Jelly-style toast) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New `popup_notifications` sub-plugin: an in-browser toast card that appears top-right (below the header search) when a new notification arrives, modelled on the Jelly macOS notifier's look and delivery. Purely additive — it subscribes to the same `/notification/:user_id` MessageBus channel that already drives the bell counter and the notifications dropdown, and renders a card; the bell, dropdown, and read-state are untouched. Turning it off just stops the card. - Desktop only (never mounts on `site.mobileView`). - Opt-in per user, OFF by default: a "Desktop Pop Up Notifications" On/Off dropdown on the account page, stored in the `jtech_popup_notifications_enabled` user custom field; `popup_notifications_default_enabled` (default false) controls the default for users who haven't chosen. - Card layout: acting user's name on top, avatar on the left, topic title bold, then a preview of their message (fetched from the source post). - Click the card → route to the post (same as the dropdown row); click anywhere else, or wait `popup_notifications_timeout_seconds`, to dismiss. Backend is thin: registers the editable boolean custom field and exposes the effective (default-aware) value on the current-user serializer. Tests: - spec/system/popup_notifications_spec.rb — screenshots both states and proves the additive contract: OFF ⇒ the reply still creates the normal `replied` notification and NO toast; ON ⇒ same notification PLUS the toast, click-to-route, and click-outside dismiss. - spec/requests/popup_notifications_pref_spec.rb — the preference is editable, off by default, and default-aware on the serializer. Linting: stree, prettier, eslint, stylelint, ember-template-lint all clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- README.md | 17 ++ .../components/jtech-popup-notification.gjs | 247 ++++++++++++++++++ .../jtech-desktop-popup-notifications.gjs | 69 +++++ .../initializers/jtech-popup-notifications.js | 22 ++ assets/stylesheets/popup-notifications.scss | 94 +++++++ config/locales/client.en.yml | 8 + config/locales/server.en.yml | 16 ++ config/settings.yml | 22 ++ plugin.rb | 3 +- .../requests/popup_notifications_pref_spec.rb | 55 ++++ spec/system/popup_notifications_spec.rb | 119 +++++++++ sub_plugins/popup_notifications.rb | 48 ++++ 12 files changed, 719 insertions(+), 1 deletion(-) create mode 100644 assets/javascripts/discourse/components/jtech-popup-notification.gjs create mode 100644 assets/javascripts/discourse/connectors/user-custom-preferences/jtech-desktop-popup-notifications.gjs create mode 100644 assets/javascripts/discourse/initializers/jtech-popup-notifications.js create mode 100644 assets/stylesheets/popup-notifications.scss create mode 100644 spec/requests/popup_notifications_pref_spec.rb create mode 100644 spec/system/popup_notifications_spec.rb create mode 100644 sub_plugins/popup_notifications.rb diff --git a/README.md b/README.md index ff3884d..1d7c659 100644 --- a/README.md +++ b/README.md @@ -13,9 +13,26 @@ One combined Discourse plugin. Bundles seven previously-separate plugins under a | Dumbcourse | `DiscourseDumbcourse` | `dumbcourse_*` | `dumbcourse_enabled` | | Translator-tweaks | *(patches `DiscourseTranslator`)* | *(none — gated by translator's own settings)* | `translator_enabled` (upstream) | | Smart search | `DiscourseSmartSearch` | `smart_search_*` | `smart_search_enabled` | +| Desktop pop-ups | `DiscoursePopupNotifications` | `popup_notifications_*` | `popup_notifications_enabled` | The bundle is gated by `jtech_enabled`; each sub-plugin is independently gated by its own setting above. +### Desktop pop-up notifications + +A Jelly-style toast card that appears in the top-right corner (just below the header search) when a new notification arrives, modelled on the [Jelly](https://github.com/lubabs770/Jelly) macOS notifier's look and delivery. + +- **Purely additive.** It subscribes to the same `/notification/:user_id` MessageBus channel that already drives the bell counter and the notifications dropdown, and does nothing else — the bell, the dropdown, and read-state are untouched. Turning it off simply stops the card from appearing. +- **Desktop only.** Never mounts on mobile (`site.mobileView`). +- **Opt-in per user, off by default.** Each user turns it on via a **Desktop Pop Up Notifications** On/Off dropdown on their account page (`/u/:username/preferences/account`), stored in the `jtech_popup_notifications_enabled` user custom field. `popup_notifications_default_enabled` (default `false`) controls the default for users who haven't chosen. +- **Card layout:** the acting user's name on top, their avatar on the left, the topic title in bold, then a short preview of their message (fetched from the source post). +- **Interaction:** clicking the card routes to the post (same as clicking the row in the dropdown); clicking anywhere else — or waiting `popup_notifications_timeout_seconds` (default 20) — dismisses it. + +| Setting | Default | Purpose | +| --- | --- | --- | +| `popup_notifications_enabled` | `true` | Master switch. Off ⇒ no card for anyone, per-user preference hidden. | +| `popup_notifications_default_enabled` | `false` | Default for users who haven't set the account-page preference. | +| `popup_notifications_timeout_seconds` | `20` | Seconds the card stays before auto-dismissing. | + ### Mod-categories — staff-event notifications Mod-categories ships a notification fan-out for five staff-event streams in addition to its original topic-level moderator notes. Whenever a moderator performs one of the actions below, every OTHER staff member gets a high-priority bell notification + live MessageBus pop-up alert, AND the event surfaces in the shield-tab user menu alongside topic notes. diff --git a/assets/javascripts/discourse/components/jtech-popup-notification.gjs b/assets/javascripts/discourse/components/jtech-popup-notification.gjs new file mode 100644 index 0000000..14f45d0 --- /dev/null +++ b/assets/javascripts/discourse/components/jtech-popup-notification.gjs @@ -0,0 +1,247 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { on } from "@ember/modifier"; +import { action } from "@ember/object"; +import { cancel } from "@ember/runloop"; +import { service } from "@ember/service"; +import icon from "discourse/helpers/d-icon"; +import { ajax } from "discourse/lib/ajax"; +import { getURLWithCDN } from "discourse/lib/get-url"; +import discourseLater from "discourse/lib/later"; +import DiscourseURL from "discourse/lib/url"; + +// Desktop-only, Jelly-style pop-up "toast". Purely ADDITIVE — it renders a +// card when a new notification is published on the current user's +// `/notification/:id` MessageBus channel (the same channel that already +// drives the bell counter and the notifications dropdown) and does nothing +// else. Core notifications, the bell, the dropdown, and read-state are all +// untouched; turning the feature off simply stops this card from appearing. +// +// Card layout (top → bottom): the acting user's name, their avatar on the +// left, the topic title in bold, then a short preview of their message. +// +// Never mounts on mobile (`site.mobileView`) or for users who have not +// opted in on their account page. +const AVATAR_SIZE = 48; +const EXCERPT_LENGTH = 120; +const STALE_MS = 10000; + +export default class JtechPopupNotification extends Component { + @service currentUser; + @service siteSettings; + @service site; + @service messageBus; + + @tracked toast = null; + + channel = null; + dismissHandle = null; + lastShownId = null; + + constructor() { + super(...arguments); + if ( + !this.currentUser || + this.site.mobileView || + !this.siteSettings.popup_notifications_enabled + ) { + return; + } + this.mountedAt = Date.now(); + this.channel = `/notification/${this.currentUser.id}`; + this.onDocumentClick = this.onDocumentClick.bind(this); + this.messageBus.subscribe(this.channel, this.onMessage); + } + + // Read live so saving the account-page dropdown (which mirrors the value + willDestroy() { + super.willDestroy(...arguments); + this.clear(); + if (this.channel) { + this.messageBus.unsubscribe(this.channel, this.onMessage); + } + } + + // onto currentUser) takes effect without a page reload. + get prefEnabled() { + return !!this.currentUser?.jtech_popup_notifications_enabled; + } + + @action + async onMessage(payload) { + try { + if (!this.prefEnabled) { + return; + } + const notification = payload?.last_notification?.notification; + if (!notification || notification.read) { + return; + } + if (notification.id === this.lastShownId) { + return; + } + // Ignore MessageBus backlog replayed from before this tab mounted. + const createdAt = Date.parse(notification.created_at); + if (createdAt && createdAt < this.mountedAt - STALE_MS) { + return; + } + this.lastShownId = notification.id; + await this.present(notification); + } catch { + // A malformed payload must never break the page. + } + } + + async present(notification) { + const data = notification.data || {}; + const toast = { + username: + data.display_username || + data.username || + data.original_username || + data.mentioned_by_username || + "", + title: notification.fancy_title || data.topic_title || data.title || "", + excerpt: data.excerpt || "", + avatarUrl: null, + url: this.urlFor(notification), + }; + + // Enrich with the acting user's avatar + a preview of their message from + // the source post. Best-effort: the card still shows without it. + try { + const post = await this.fetchPost(notification, data); + if (post) { + if (post.avatar_template) { + toast.avatarUrl = getURLWithCDN( + post.avatar_template.replace("{size}", AVATAR_SIZE) + ); + } + if (!toast.username && post.username) { + toast.username = post.username; + } + if (!toast.excerpt && post.cooked) { + toast.excerpt = this.excerptFrom(post.cooked); + } + } + } catch { + // ignore enrichment failure — show what we have + } + + // The preference may have flipped off during the await. + if (!this.prefEnabled) { + return; + } + this.toast = toast; + this.arm(); + } + + fetchPost(notification, data) { + if (data.original_post_id) { + return ajax(`/posts/${data.original_post_id}.json`); + } + if (notification.topic_id && notification.post_number) { + return ajax( + `/posts/by_number/${notification.topic_id}/${notification.post_number}.json` + ); + } + return null; + } + + excerptFrom(cooked) { + const el = document.createElement("div"); + el.innerHTML = cooked; + const text = (el.textContent || "").replace(/\s+/g, " ").trim(); + return text.length > EXCERPT_LENGTH + ? `${text.slice(0, EXCERPT_LENGTH)}…` + : text; + } + + urlFor(notification) { + const data = notification.data || {}; + if (notification.topic_id && notification.slug) { + const suffix = notification.post_number + ? `/${notification.post_number}` + : ""; + return `/t/${notification.slug}/${notification.topic_id}${suffix}`; + } + if (data.url) { + return data.url; + } + return `/u/${this.currentUser.username}/notifications`; + } + + arm() { + this.clear(); + const secs = + parseInt(this.siteSettings.popup_notifications_timeout_seconds, 10) || 20; + this.dismissHandle = discourseLater(this, this.dismiss, secs * 1000); + document.addEventListener("click", this.onDocumentClick, true); + } + + clear() { + if (this.dismissHandle) { + cancel(this.dismissHandle); + this.dismissHandle = null; + } + if (this.onDocumentClick) { + document.removeEventListener("click", this.onDocumentClick, true); + } + } + + onDocumentClick(event) { + // Clicking anywhere outside the card dismisses it. A click on the card + // itself is handled by `open` (this listener is capture-phase and only + // dismisses when the target is outside). + if (!event.target.closest(".jtech-popup-toast")) { + this.dismiss(); + } + } + + @action + open() { + const url = this.toast?.url; + this.dismiss(); + if (url) { + DiscourseURL.routeTo(url); + } + } + + @action + dismiss() { + this.clear(); + this.toast = null; + } + + +} diff --git a/assets/javascripts/discourse/connectors/user-custom-preferences/jtech-desktop-popup-notifications.gjs b/assets/javascripts/discourse/connectors/user-custom-preferences/jtech-desktop-popup-notifications.gjs new file mode 100644 index 0000000..6b8686c --- /dev/null +++ b/assets/javascripts/discourse/connectors/user-custom-preferences/jtech-desktop-popup-notifications.gjs @@ -0,0 +1,69 @@ +import Component from "@glimmer/component"; +import { action } from "@ember/object"; +import { service } from "@ember/service"; +import { i18n } from "discourse-i18n"; +import ComboBox from "select-kit/components/combo-box"; + +// "Desktop Pop Up Notifications" On/Off dropdown on the account preferences +// page (/u/:username/preferences/account). Persists to the +// `jtech_popup_notifications_enabled` user custom field (registered editable +// server-side) via the page's normal "Save Changes" button, and mirrors the +// chosen value onto the current user so the running toast subscriber honors +// the change without a reload. +// +// Default is OFF: until a user opts in here, `enabled` resolves to the +// site's `popup_notifications_default_enabled` (false), so the pop-up never +// surprises the whole forum. +export default class JtechDesktopPopupNotifications extends Component { + @service siteSettings; + @service currentUser; + + get model() { + return this.args.outletArgs?.model; + } + + get available() { + return this.siteSettings.popup_notifications_enabled; + } + + get enabled() { + const raw = this.model?.custom_fields?.jtech_popup_notifications_enabled; + if (raw === undefined || raw === null || raw === "") { + return this.siteSettings.popup_notifications_default_enabled; + } + return raw === true || raw === "true" || raw === "t"; + } + + get content() { + return [ + { id: true, name: i18n("jtech_popup_notifications.preference.on") }, + { id: false, name: i18n("jtech_popup_notifications.preference.off") }, + ]; + } + + @action + onChange(value) { + this.model?.set("custom_fields.jtech_popup_notifications_enabled", value); + this.currentUser?.set("jtech_popup_notifications_enabled", value); + } + + +} diff --git a/assets/javascripts/discourse/initializers/jtech-popup-notifications.js b/assets/javascripts/discourse/initializers/jtech-popup-notifications.js new file mode 100644 index 0000000..5976703 --- /dev/null +++ b/assets/javascripts/discourse/initializers/jtech-popup-notifications.js @@ -0,0 +1,22 @@ +import { withPluginApi } from "discourse/lib/plugin-api"; +import JtechPopupNotification from "../components/jtech-popup-notification"; + +// Mounts the desktop pop-up notification host once, globally, into the +// always-present `above-footer` outlet. The component itself gates on +// desktop-only + the per-user preference + the site setting, so mounting +// it is cheap and inert when the feature is off. Skipped entirely when the +// master switch is off so no MessageBus subscription is even created. +export default { + name: "jtech-desktop-popup-notifications", + + initialize(container) { + const siteSettings = container.lookup("service:site-settings"); + if (!siteSettings.popup_notifications_enabled) { + return; + } + + withPluginApi("1.0", (api) => { + api.renderInOutlet("above-footer", JtechPopupNotification); + }); + }, +}; diff --git a/assets/stylesheets/popup-notifications.scss b/assets/stylesheets/popup-notifications.scss new file mode 100644 index 0000000..bd74e1f --- /dev/null +++ b/assets/stylesheets/popup-notifications.scss @@ -0,0 +1,94 @@ +// Desktop pop-up notification toast — a Jelly-style card anchored top-right, +// just below the header (and its search icon). Theme-aware: uses the active +// theme's surface colors so it looks native in both light and dark themes. +// +// Rendered by assets/javascripts/discourse/components/jtech-popup-notification.gjs +// into the always-present `above-footer` outlet; fixed positioning lifts it +// out of the footer to the top-right of the viewport. +.jtech-popup-toast { + position: fixed; + top: calc(var(--header-offset, 60px) + 10px); + right: 14px; + z-index: 1080; + display: flex; + gap: 12px; + align-items: flex-start; + box-sizing: border-box; + width: 360px; + max-width: calc(100vw - 28px); + padding: 14px 16px; + background: var(--secondary); + color: var(--primary); + border: 1px solid var(--primary-low); + border-radius: 12px; + box-shadow: 0 8px 28px rgba(0, 0, 0, 0.22); + cursor: pointer; + animation: jtech-popup-toast-in 160ms ease-out; + + &:hover { + border-color: var(--primary-medium); + } + + &__avatar { + flex: 0 0 auto; + + img { + display: block; + width: 44px; + height: 44px; + border-radius: 50%; + } + + .d-icon { + font-size: 1.75em; + color: var(--tertiary); + } + } + + &__body { + min-width: 0; // allow the ellipsis on the title to work inside flex + } + + // Top line: the acting user's name. + &__username { + font-weight: 700; + font-size: var(--font-0); + line-height: 1.2; + } + + // Middle line: topic title, bold. + &__title { + margin-top: 2px; + font-weight: 700; + font-size: var(--font-up-1); + line-height: 1.25; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + // Bottom line: a preview of the message, smaller and not bold. + &__excerpt { + margin-top: 3px; + font-weight: 400; + font-size: var(--font-down-1); + line-height: 1.35; + color: var(--primary-high); + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; + overflow: hidden; + } +} + +@keyframes jtech-popup-toast-in { + from { + opacity: 0; + transform: translateY(-8px); + } + + to { + opacity: 1; + transform: translateY(0); + } +} diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d1c5b35..46a25f1 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1,5 +1,13 @@ en: js: + jtech_popup_notifications: + someone: Someone + preference: + title: Desktop Pop Up Notifications + instructions: Show a small pop-up card in the top-right corner when you + receive a new notification. Desktop only — this never appears on mobile. + on: "On" + off: "Off" discourse_mod_categories: precheck: title: Before you post diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4080e3c..bfd2b2e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -8,6 +8,7 @@ en: jtech_mod: Jtech — Mod jtech_dumbcourse: Jtech — Dumbcourse jtech_smart_search: Jtech — Smart search + jtech_popup_notifications: Jtech — Desktop pop-ups # ── Jtech (master) ───────────────────────────────────────────────────── jtech_enabled: Enable the Jtech plugin bundle @@ -172,6 +173,21 @@ en: extra SQL query; raising this gives broader matches but slower search. Range 1–5. + # ── Jtech — Desktop pop-ups ──────────────────────────────────────────── + popup_notifications_enabled: Enable desktop pop-up notifications + popup_notifications_enabled_description: Master switch for the Jelly-style + pop-up notification card (top-right, desktop only). When off, no card + appears for anyone and the per-user preference is hidden. This is purely + additive — it never changes the bell counter, the notifications dropdown, + or read state. + popup_notifications_default_enabled: Pop-ups on by default + popup_notifications_default_enabled_description: The default for users who + have not chosen on their account page. Off by design, so enabling the + plugin never surprises the whole forum — each user opts in themselves. + popup_notifications_timeout_seconds: Pop-up auto-dismiss (seconds) + popup_notifications_timeout_seconds_description: How long the card stays on + screen before dismissing itself. Clicking anywhere else also dismisses it. + # ── Jtech — Dumbcourse ───────────────────────────────────────────────── dumbcourse_enabled: Enable Dumbcourse dumbcourse_enabled_description: Serves the Dumbcourse SPA under the diff --git a/config/settings.yml b/config/settings.yml index 995689e..0df0f2e 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -182,6 +182,28 @@ jtech_smart_search: min: 1 max: 5 +# ─────────────────────────────────────────────────────────────────────────── +# Jtech — Desktop pop-up notifications (Jelly-style toast, desktop only) +# ─────────────────────────────────────────────────────────────────────────── +jtech_popup_notifications: + # Master switch. When off, no toast fires for anyone and the per-user + # preference on the account page is hidden. + popup_notifications_enabled: + default: true + client: true + # Default for users who have never set the per-user preference. Off by + # design — the toast only appears for users who opt in on their account + # page, so enabling the plugin never surprises the whole forum. + popup_notifications_default_enabled: + default: false + client: true + # Seconds the toast stays on screen before auto-dismissing. + popup_notifications_timeout_seconds: + default: 20 + client: true + min: 3 + max: 300 + # ─────────────────────────────────────────────────────────────────────────── # Jtech — Dumbcourse (lightweight SPA at /dumb with push notifications) # ─────────────────────────────────────────────────────────────────────────── diff --git a/plugin.rb b/plugin.rb index 1bd0005..60f220f 100644 --- a/plugin.rb +++ b/plugin.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # name: jtech-tools -# about: Jtech — combined Discourse plugin (dislike, another-smtp, mini-mod, mod-categories, dumbcourse, translator-tweaks, smart-search) +# about: Jtech — combined Discourse plugin (dislike, another-smtp, mini-mod, mod-categories, dumbcourse, translator-tweaks, smart-search, popup-notifications) # version: 0.1.1 # authors: TripleU, Shalom_Karr, Ars18 # url: https://github.com/JTech-Forums/JtechTools @@ -36,6 +36,7 @@ dumbcourse translator_tweaks smart_search + popup_notifications ].each do |sub| path = File.expand_path("sub_plugins/#{sub}.rb", __dir__) instance_eval(File.read(path), path, 1) diff --git a/spec/requests/popup_notifications_pref_spec.rb b/spec/requests/popup_notifications_pref_spec.rb new file mode 100644 index 0000000..6227acd --- /dev/null +++ b/spec/requests/popup_notifications_pref_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require "rails_helper" + +# Backend contract for the "Desktop Pop Up Notifications" preference: the +# per-user custom field is editable, off by default, and surfaced on the +# current-user serializer as an effective (default-aware) boolean. This is +# the server side of the additive feature — it changes nothing about how +# core notifications are created or delivered. +RSpec.describe "Desktop pop-up notification preference" do + fab!(:user) + + let(:field) { DiscoursePopupNotifications::USER_ENABLED_FIELD } + + before do + SiteSetting.popup_notifications_enabled = true + SiteSetting.popup_notifications_default_enabled = false + end + + def current_user_json + sign_in(user) + get "/session/current.json" + expect(response.status).to eq(200) + response.parsed_body["current_user"] + end + + it "defaults to the site default (off) when the user has not chosen" do + expect(current_user_json["jtech_popup_notifications_enabled"]).to eq(false) + end + + it "follows the site default when that default is on" do + SiteSetting.popup_notifications_default_enabled = true + expect(current_user_json["jtech_popup_notifications_enabled"]).to eq(true) + end + + it "reflects an explicit per-user opt-in" do + user.custom_fields[field] = true + user.save_custom_fields(true) + expect(current_user_json["jtech_popup_notifications_enabled"]).to eq(true) + end + + it "reflects an explicit per-user opt-out even when the default is on" do + SiteSetting.popup_notifications_default_enabled = true + user.custom_fields[field] = false + user.save_custom_fields(true) + expect(current_user_json["jtech_popup_notifications_enabled"]).to eq(false) + end + + it "persists the field through a preferences update (registered editable)" do + sign_in(user) + put "/u/#{user.username}.json", params: { custom_fields: { field => true } } + expect(response.status).to eq(200) + expect(user.reload.custom_fields[field]).to eq(true) + end +end diff --git a/spec/system/popup_notifications_spec.rb b/spec/system/popup_notifications_spec.rb new file mode 100644 index 0000000..fa4c5be --- /dev/null +++ b/spec/system/popup_notifications_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require "rails_helper" + +# End-to-end coverage for the desktop pop-up notification toast, plus proof +# that it is PURELY ADDITIVE: +# +# * OFF (the default): a new reply still creates the normal `replied` +# notification (bell/dropdown unchanged) and NO toast appears. +# * ON: the same reply creates the same notification AND additionally pops +# the toast. Clicking the toast routes to the post (like the dropdown +# row); clicking elsewhere dismisses it. +# +# Screenshots of both states are captured for UI/UX review; they land in +# tmp/capybara/ and are published as the CI artifact. +RSpec.describe "Desktop pop-up notifications", type: :system do + fab!(:author) { Fabricate(:user, username: "poster_pat") } + fab!(:recipient) { Fabricate(:user, username: "reader_rhea") } + fab!(:category) + fab!(:topic) do + Fabricate( + :topic, + category: category, + user: recipient, + title: "Might be the next Qin but better", + ) + end + fab!(:op) do + Fabricate(:post, topic: topic, user: recipient, raw: "What do you all think of this phone?") + end + + let(:user_field) { DiscoursePopupNotifications::USER_ENABLED_FIELD } + + before do + SiteSetting.popup_notifications_enabled = true + SiteSetting.auto_silence_fast_typers_on_first_post = false + end + + def shot(name) + page.save_screenshot("#{name}.png") + rescue StandardError + # Never fail the spec over a screenshot. + end + + def set_pref(value) + recipient.custom_fields[user_field] = value + recipient.save_custom_fields(true) + end + + # A reply BY author TO the recipient's opening post → core creates a + # `replied` notification for recipient and publishes it on + # /notification/:id (the channel the toast subscribes to). + def reply_to_recipient! + PostCreator.create!( + author, + topic_id: topic.id, + raw: + "Excellent screen quality. Supports 4g volte in Israel with excellent cellular reception.", + reply_to_post_number: op.post_number, + ) + end + + def replied_notification_exists? + Notification.exists?(user_id: recipient.id, notification_type: Notification.types[:replied]) + end + + it "off (default): normal notification fires, no toast appears" do + set_pref(false) + sign_in(recipient) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css("#post_1", wait: 10) + + reply_to_recipient! + + # Core notification is created exactly as before — nothing changed. + expect(replied_notification_exists?).to eq(true) + # ...but the additive toast never appears. + expect(page).to have_no_css(".jtech-popup-toast", wait: 5) + shot("popup_notifications_off") + end + + it "on: the same notification also pops the additive toast" do + set_pref(true) + sign_in(recipient) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css("#post_1", wait: 10) + + reply_to_recipient! + + # The toast appears, laid out name → avatar → bold title → message. + expect(page).to have_css(".jtech-popup-toast", wait: 10) + expect(page).to have_css(".jtech-popup-toast__username", text: "poster_pat") + expect(page).to have_css(".jtech-popup-toast__title", text: topic.title) + shot("popup_notifications_on") + + # Core notifications still work — the bell is present and the row exists. + expect(replied_notification_exists?).to eq(true) + expect(page).to have_css(".header-dropdown-toggle.current-user") + + # Clicking the toast routes to the replied post, like the dropdown row. + find(".jtech-popup-toast").click + expect(page).to have_css("#post_2", wait: 10) + expect(page).to have_no_css(".jtech-popup-toast") + end + + it "dismisses when clicking anywhere else" do + set_pref(true) + sign_in(recipient) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css("#post_1", wait: 10) + + reply_to_recipient! + expect(page).to have_css(".jtech-popup-toast", wait: 10) + + # A click outside the card (on the opening post) dismisses it. + find("#post_1 .cooked").click + expect(page).to have_no_css(".jtech-popup-toast") + end +end diff --git a/sub_plugins/popup_notifications.rb b/sub_plugins/popup_notifications.rb new file mode 100644 index 0000000..703436c --- /dev/null +++ b/sub_plugins/popup_notifications.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true +# Jtech sub-plugin: desktop pop-up notifications. +# +# Renders an in-browser "toast" card (top-right, just below the header +# search) when a new notification arrives for the current user — modelled +# on the Jelly macOS notifier's look and delivery. Delivery reuses the +# same MessageBus channel Discourse already publishes notification state +# on (`/notification/:user_id`), so no new server push path is needed. +# +# Desktop only: the frontend never subscribes or renders on mobile +# (`site.mobileView`). Gated per-user by an account-page preference +# ("Desktop Pop Up Notifications", on/off) stored in a user custom field, +# and site-wide by `popup_notifications_enabled`. +# +# The backend here is deliberately thin — the whole experience is +# client-side. All this file does is: +# * register the per-user boolean custom field + make it editable, and +# * expose the effective (default-aware) value on the current-user +# serializer so the client can gate on `currentUser. +# jtech_popup_notifications_enabled`. + +register_asset "stylesheets/popup-notifications.scss" + +module ::DiscoursePopupNotifications + # Per-user preference. Key PRESENCE + value decides; when the field is + # absent the effective value falls back to + # `SiteSetting.popup_notifications_default_enabled`. + USER_ENABLED_FIELD = "jtech_popup_notifications_enabled" +end + +after_initialize do + register_user_custom_field_type(DiscoursePopupNotifications::USER_ENABLED_FIELD, :boolean) + + # Permit the field through UsersController#update so the account-page + # dropdown can save it with the rest of the preferences form. + register_editable_user_custom_field(DiscoursePopupNotifications::USER_ENABLED_FIELD) + + # Effective, default-aware preference for the client. Returns the stored + # boolean when the user has chosen, otherwise the site-wide default. + add_to_serializer(:current_user, :jtech_popup_notifications_enabled) do + raw = object.custom_fields[DiscoursePopupNotifications::USER_ENABLED_FIELD] + if raw.nil? + SiteSetting.popup_notifications_default_enabled + else + ActiveModel::Type::Boolean.new.cast(raw) + end + end +end From 6894548b3b6822253c080702ad5f132a51e1c264 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 13:54:59 -0400 Subject: [PATCH 02/11] Pop-up notifications: 15-shot screenshot gallery + rubocop fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add spec/system/popup_notifications_screenshots_spec.rb: 15 screenshots across both surfaces — the account-page preference control (default off, dropdown open, set on, set off), the admin master switch, the control hidden when the master is off, a toast per notification type (reply, mention, quote, PM, like), content-shape variety (long title ellipsis, long message clamp, bell-icon fallback), and the off state (no toast). Toast shots publish crafted notifications on /notification/:id for exact, fast visual states; the real end-to-end path stays covered by the behavioral spec. - Drop redundant `type: :system` metadata (Discourse/NoSystemSpecMetadata). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- .../popup_notifications_screenshots_spec.rb | 255 ++++++++++++++++++ spec/system/popup_notifications_spec.rb | 2 +- 2 files changed, 256 insertions(+), 1 deletion(-) create mode 100644 spec/system/popup_notifications_screenshots_spec.rb diff --git a/spec/system/popup_notifications_screenshots_spec.rb b/spec/system/popup_notifications_screenshots_spec.rb new file mode 100644 index 0000000..94e6c63 --- /dev/null +++ b/spec/system/popup_notifications_screenshots_spec.rb @@ -0,0 +1,255 @@ +# frozen_string_literal: true + +require "rails_helper" + +# Screenshot gallery for the desktop pop-up notification feature — 15 shots +# across the two surfaces the user interacts with: +# +# * the account page where the preference is set (shots 01–04), plus the +# admin master switch (05) and the control hidden when the master is off +# (06); and +# * the page where the notification arrives — the toast itself, across +# notification types and content shapes (07–15). +# +# The toast shots drive the card by publishing crafted notifications on the +# user's `/notification/:id` MessageBus channel (the same channel core uses), +# which is fast and lets each shot pin an exact visual state. The REAL +# end-to-end path (a live reply → toast) is proven separately in +# spec/system/popup_notifications_spec.rb. +# +# Screenshots land in tmp/capybara/ and are published as the CI artifact. +RSpec.describe "Desktop pop-up notification screenshots" do + fab!(:author) { Fabricate(:user, username: "poster_pat", name: "Pat Poster") } + fab!(:recipient) { Fabricate(:user, username: "reader_rhea") } + fab!(:admin) { Fabricate(:admin, username: "admin_amy") } + fab!(:category) { Fabricate(:category, name: "Flip phones") } + fab!(:topic) do + Fabricate( + :topic, + category: category, + user: recipient, + title: "Might be the next Qin but better", + ) + end + fab!(:op) do + Fabricate(:post, topic: topic, user: recipient, raw: "What do you all think of this phone?") + end + fab!(:reply_post) do + Fabricate( + :post, + topic: topic, + user: author, + raw: + "Excellent screen quality. Supports 4g volte in Israel with excellent cellular reception.", + ) + end + fab!(:long_reply) do + Fabricate( + :post, + topic: topic, + user: author, + raw: + "Honestly this might be the best budget option out there right now — the build feels " \ + "premium, the screen is bright even outdoors, calls are crystal clear on 4G VoLTE, and " \ + "the battery genuinely lasts a day and a half of heavy use without needing a top-up.", + ) + end + fab!(:long_topic) do + Fabricate( + :topic, + category: category, + user: recipient, + title: + "A remarkably and unnecessarily long topic title that should be truncated with an " \ + "ellipsis inside the pop-up card so it never wraps onto a second line", + ) + end + fab!(:long_topic_reply) do + Fabricate(:post, topic: long_topic, user: author, raw: "See the specs I linked above.") + end + + let(:user_field) { DiscoursePopupNotifications::USER_ENABLED_FIELD } + # Mutable memoized counter (avoids an instance variable in the helper) so + # every crafted notification gets a fresh id and dedupe never suppresses it. + let(:id_seq) { [500_000] } + + before do + SiteSetting.popup_notifications_enabled = true + SiteSetting.popup_notifications_timeout_seconds = 300 # keep the card up long enough to shoot + SiteSetting.auto_silence_fast_typers_on_first_post = false + recipient.custom_fields[user_field] = true + recipient.save_custom_fields(true) + end + + def shot(name) + begin + Timeout.timeout(8) do + sleep 0.1 until page.evaluate_script("Array.from(document.images).every((i) => i.complete)") + end + rescue Timeout::Error + # Capture anyway rather than fail over a slow avatar image. + end + page.save_screenshot("popup_notifications_#{name}.png") + end + + # Publish a crafted notification on the recipient's channel — the browser is + # subscribed, so the toast renders it. Each call uses a fresh id so the + # component's dedupe never suppresses it. + def push(type:, data:, topic_id: nil, post_number: nil, fancy_title: nil, slug: nil) + id_seq[0] += 1 + payload = { + unread_notifications: 1, + all_unread_notifications_count: 1, + last_notification: { + notification: { + id: id_seq[0], + user_id: recipient.id, + notification_type: Notification.types[type], + read: false, + high_priority: false, + created_at: Time.zone.now.iso8601, + post_number: post_number, + topic_id: topic_id, + fancy_title: fancy_title, + slug: slug, + data: data, + }, + }, + } + MessageBus.publish("/notification/#{recipient.id}", payload, user_ids: [recipient.id]) + end + + # A reply-shaped notification that enriches from a real post (avatar + body + # excerpt), varying only the type so each screenshot is a distinct kind. + def push_from_post(type:, post:, into: topic) + push( + type: type, + topic_id: into.id, + post_number: post.post_number, + slug: into.slug, + fancy_title: into.fancy_title, + data: { + display_username: post.user.username, + topic_title: into.title, + original_post_id: post.id, + }, + ) + end + + def show_toast(type:, post: reply_post, into: topic) + push_from_post(type: type, post: post, into: into) + expect(page).to have_css(".jtech-popup-toast", wait: 10) + end + + def dismiss_toast + find("#post_1 .cooked").click + expect(page).to have_no_css(".jtech-popup-toast") + end + + def open_account_preference + sign_in(recipient) + visit("/u/#{recipient.username}/preferences/account") + expect(page).to have_css(".jtech-desktop-popup-notifications", wait: 10) + end + + def open_topic + sign_in(recipient) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css("#post_1", wait: 10) + end + + it "captures the account-page preference control (01–04)" do + open_account_preference + shot("01_settings_account_default_off") + + find(".jtech-desktop-popup-notifications .select-kit-header").click + expect(page).to have_css(".select-kit-collection", wait: 5) + shot("02_settings_dropdown_open") + + find(".select-kit-row", text: I18n.t("js.jtech_popup_notifications.preference.on")).click + expect(page).to have_css(".jtech-desktop-popup-notifications") + shot("03_settings_set_on") + + find(".jtech-desktop-popup-notifications .select-kit-header").click + find(".select-kit-row", text: I18n.t("js.jtech_popup_notifications.preference.off")).click + shot("04_settings_set_off") + end + + it "captures the admin master switch (05)" do + sign_in(admin) + visit("/admin/site_settings/category/all_results?filter=popup_notifications") + expect(page).to have_css(".setting", wait: 10) + shot("05_settings_admin_master_switch") + end + + it "hides the account control when the master switch is off (06)" do + SiteSetting.popup_notifications_enabled = false + sign_in(recipient) + visit("/u/#{recipient.username}/preferences/account") + expect(page).to have_css(".user-preferences", wait: 10) + expect(page).to have_no_css(".jtech-desktop-popup-notifications") + shot("06_settings_control_hidden_master_off") + end + + it "captures a toast for each notification type (07–11)" do + open_topic + + show_toast(type: :replied) + shot("07_toast_reply") + dismiss_toast + + show_toast(type: :mentioned) + shot("08_toast_mention") + dismiss_toast + + show_toast(type: :quoted) + shot("09_toast_quote") + dismiss_toast + + show_toast(type: :private_message) + shot("10_toast_private_message") + dismiss_toast + + show_toast(type: :liked) + shot("11_toast_liked") + end + + it "captures content-shape variety: long title, long message, icon fallback (12–14)" do + open_topic + + # Long topic title → ellipsis on the bold title line. + show_toast(type: :replied, post: long_topic_reply, into: long_topic) + shot("12_toast_long_title") + dismiss_toast + + # Long message → the preview line clamps to two lines. + show_toast(type: :replied, post: long_reply) + shot("13_toast_long_message") + dismiss_toast + + # No source post → the avatar slot falls back to the bell icon, and the + # preview comes straight from the notification data. + push( + type: :custom, + data: { + display_username: "system_sam", + topic_title: "Scheduled maintenance tonight", + excerpt: "The forum will be briefly unavailable around 2am for a database upgrade.", + url: "/u/#{recipient.username}/notifications", + }, + ) + expect(page).to have_css(".jtech-popup-toast .d-icon-bell", wait: 10) + shot("14_toast_fallback_icon") + end + + it "shows nothing extra when the preference is off (15)" do + recipient.custom_fields[user_field] = false + recipient.save_custom_fields(true) + open_topic + + push_from_post(type: :replied, post: reply_post) + # The core bell still works; the additive toast never appears. + expect(page).to have_no_css(".jtech-popup-toast", wait: 5) + shot("15_notification_off_no_popup") + end +end diff --git a/spec/system/popup_notifications_spec.rb b/spec/system/popup_notifications_spec.rb index fa4c5be..e4bc415 100644 --- a/spec/system/popup_notifications_spec.rb +++ b/spec/system/popup_notifications_spec.rb @@ -13,7 +13,7 @@ # # Screenshots of both states are captured for UI/UX review; they land in # tmp/capybara/ and are published as the CI artifact. -RSpec.describe "Desktop pop-up notifications", type: :system do +RSpec.describe "Desktop pop-up notifications" do fab!(:author) { Fabricate(:user, username: "poster_pat") } fab!(:recipient) { Fabricate(:user, username: "reader_rhea") } fab!(:category) From dc502b1ca81ae15adbfcbef26c2dae2b13cfd6d8 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 14:02:54 -0400 Subject: [PATCH 03/11] CI: run popup_notifications screenshots in Feature Screenshots workflow The Feature Screenshots workflow runs a fixed spec list and uploads the resulting tmp/capybara PNGs as an artifact. Add the new popup_notifications_screenshots_spec.rb so its 15 shots are captured and published for review. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- .github/workflows/feature-screenshots.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/feature-screenshots.yml b/.github/workflows/feature-screenshots.yml index 845d219..847a07b 100644 --- a/.github/workflows/feature-screenshots.yml +++ b/.github/workflows/feature-screenshots.yml @@ -130,7 +130,8 @@ jobs: --format documentation \ plugins/${{ env.PLUGIN_NAME }}/spec/system/feature_screenshots_spec.rb \ plugins/${{ env.PLUGIN_NAME }}/spec/system/review_queue_click_through_spec.rb \ - plugins/${{ env.PLUGIN_NAME }}/spec/system/notifications_type_filter_spec.rb + plugins/${{ env.PLUGIN_NAME }}/spec/system/notifications_type_filter_spec.rb \ + plugins/${{ env.PLUGIN_NAME }}/spec/system/popup_notifications_screenshots_spec.rb continue-on-error: true - name: Upload feature screenshots (always) From 145752bbebb05091ed2a095f565928c60bbaa2e1 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 14:12:40 -0400 Subject: [PATCH 04/11] Fix account-page preference: correct outlet + save immediately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The On/Off dropdown was mounted on the `user-custom-preferences` outlet, which lives on the PROFILE page — not the account page — so it never rendered (the 01–04 screenshot example failed on a missing selector). Move it to `user-preferences-account` (confirmed present on the account page in Discourse 2026.7) and save immediately on change via PUT /u/:username.json instead of relying on the account form's Save button, which does not persist arbitrary custom fields. The value is mirrored onto the current user for live toast gating and reverted on error. This save path is the one exercised by spec/requests/popup_notifications_pref_spec.rb. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- .../jtech-desktop-popup-notifications.gjs | 69 ---------------- .../jtech-desktop-popup-notifications.gjs | 78 +++++++++++++++++++ 2 files changed, 78 insertions(+), 69 deletions(-) delete mode 100644 assets/javascripts/discourse/connectors/user-custom-preferences/jtech-desktop-popup-notifications.gjs create mode 100644 assets/javascripts/discourse/connectors/user-preferences-account/jtech-desktop-popup-notifications.gjs diff --git a/assets/javascripts/discourse/connectors/user-custom-preferences/jtech-desktop-popup-notifications.gjs b/assets/javascripts/discourse/connectors/user-custom-preferences/jtech-desktop-popup-notifications.gjs deleted file mode 100644 index 6b8686c..0000000 --- a/assets/javascripts/discourse/connectors/user-custom-preferences/jtech-desktop-popup-notifications.gjs +++ /dev/null @@ -1,69 +0,0 @@ -import Component from "@glimmer/component"; -import { action } from "@ember/object"; -import { service } from "@ember/service"; -import { i18n } from "discourse-i18n"; -import ComboBox from "select-kit/components/combo-box"; - -// "Desktop Pop Up Notifications" On/Off dropdown on the account preferences -// page (/u/:username/preferences/account). Persists to the -// `jtech_popup_notifications_enabled` user custom field (registered editable -// server-side) via the page's normal "Save Changes" button, and mirrors the -// chosen value onto the current user so the running toast subscriber honors -// the change without a reload. -// -// Default is OFF: until a user opts in here, `enabled` resolves to the -// site's `popup_notifications_default_enabled` (false), so the pop-up never -// surprises the whole forum. -export default class JtechDesktopPopupNotifications extends Component { - @service siteSettings; - @service currentUser; - - get model() { - return this.args.outletArgs?.model; - } - - get available() { - return this.siteSettings.popup_notifications_enabled; - } - - get enabled() { - const raw = this.model?.custom_fields?.jtech_popup_notifications_enabled; - if (raw === undefined || raw === null || raw === "") { - return this.siteSettings.popup_notifications_default_enabled; - } - return raw === true || raw === "true" || raw === "t"; - } - - get content() { - return [ - { id: true, name: i18n("jtech_popup_notifications.preference.on") }, - { id: false, name: i18n("jtech_popup_notifications.preference.off") }, - ]; - } - - @action - onChange(value) { - this.model?.set("custom_fields.jtech_popup_notifications_enabled", value); - this.currentUser?.set("jtech_popup_notifications_enabled", value); - } - - -} diff --git a/assets/javascripts/discourse/connectors/user-preferences-account/jtech-desktop-popup-notifications.gjs b/assets/javascripts/discourse/connectors/user-preferences-account/jtech-desktop-popup-notifications.gjs new file mode 100644 index 0000000..3816799 --- /dev/null +++ b/assets/javascripts/discourse/connectors/user-preferences-account/jtech-desktop-popup-notifications.gjs @@ -0,0 +1,78 @@ +import Component from "@glimmer/component"; +import { action } from "@ember/object"; +import { service } from "@ember/service"; +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import { i18n } from "discourse-i18n"; +import ComboBox from "select-kit/components/combo-box"; + +// "Desktop Pop Up Notifications" On/Off dropdown on the account preferences +// page (/u/:username/preferences/account). Rendered into the +// `user-preferences-account` outlet. +// +// It saves immediately on change with a PUT to /u/:username.json (the +// `jtech_popup_notifications_enabled` custom field is registered editable +// server-side), rather than depending on the account form's "Save Changes" +// button — the account controller does not persist arbitrary custom fields, +// and an instant toggle is the expected UX here anyway. The value is also +// mirrored onto the current user so the running toast subscriber honors the +// change without a reload. +// +// Default is OFF: the current-user serializer resolves the effective value +// from `popup_notifications_default_enabled` (false) until the user opts in, +// so the pop-up never surprises the whole forum. +export default class JtechDesktopPopupNotifications extends Component { + @service siteSettings; + @service currentUser; + + get available() { + return this.siteSettings.popup_notifications_enabled; + } + + get enabled() { + return !!this.currentUser?.jtech_popup_notifications_enabled; + } + + get content() { + return [ + { id: true, name: i18n("jtech_popup_notifications.preference.on") }, + { id: false, name: i18n("jtech_popup_notifications.preference.off") }, + ]; + } + + @action + async onChange(value) { + const previous = this.enabled; + // Optimistic + live gate for the running toast subscriber. + this.currentUser.set("jtech_popup_notifications_enabled", value); + try { + await ajax(`/u/${this.currentUser.username}.json`, { + type: "PUT", + data: { custom_fields: { jtech_popup_notifications_enabled: value } }, + }); + } catch (error) { + this.currentUser.set("jtech_popup_notifications_enabled", previous); + popupAjaxError(error); + } + } + + +} From d7c486e700f743507da9fbfd391bf0a8d84c0b7d Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 14:21:05 -0400 Subject: [PATCH 05/11] Fix On/Off i18n keys (YAML booleanized them) + spec selectors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The account-page dropdown rendered "[en.jtech_popup_notifications.preference.on]" instead of "On" — YAML parses bare `on:`/`off:` keys as booleans, so those two i18n keys never existed (title/instructions resolved fine). Quote the keys. Caught by the 01 screenshot. Also switch the screenshot spec's row selection to literal "On"/"Off" instead of I18n.t, which is simpler and avoids the same trap. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- config/locales/client.en.yml | 6 ++++-- spec/system/popup_notifications_screenshots_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 46a25f1..c20ccc0 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -6,8 +6,10 @@ en: title: Desktop Pop Up Notifications instructions: Show a small pop-up card in the top-right corner when you receive a new notification. Desktop only — this never appears on mobile. - on: "On" - off: "Off" + # NB: "on"/"off" MUST be quoted — YAML parses bare on/off as booleans, + # which would make these i18n keys unreachable. + "on": "On" + "off": "Off" discourse_mod_categories: precheck: title: Before you post diff --git a/spec/system/popup_notifications_screenshots_spec.rb b/spec/system/popup_notifications_screenshots_spec.rb index 94e6c63..04df35e 100644 --- a/spec/system/popup_notifications_screenshots_spec.rb +++ b/spec/system/popup_notifications_screenshots_spec.rb @@ -166,12 +166,12 @@ def open_topic expect(page).to have_css(".select-kit-collection", wait: 5) shot("02_settings_dropdown_open") - find(".select-kit-row", text: I18n.t("js.jtech_popup_notifications.preference.on")).click + find(".select-kit-row", text: "On").click expect(page).to have_css(".jtech-desktop-popup-notifications") shot("03_settings_set_on") find(".jtech-desktop-popup-notifications .select-kit-header").click - find(".select-kit-row", text: I18n.t("js.jtech_popup_notifications.preference.off")).click + find(".select-kit-row", text: "Off").click shot("04_settings_set_off") end From b7f1d90049efdd13ac8a01013f79dc50dde1b066 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 14:40:40 -0400 Subject: [PATCH 06/11] =?UTF-8?q?Pop-up:=20"Name=20=E2=80=94=20Action"=20h?= =?UTF-8?q?eading,=20type-icon=20badge,=20custom-type=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Heading line is now "Name — Action" (e.g. "pat — Liked your post"), driven by a per-type action label (config/locales client action.*). - Add a small type-icon badge on the avatar's corner; postless notifications render the type icon on its own instead of an avatar. - Decode the plugin's own `custom` notifications so they pop too and read correctly: moderator whispers (eye), flag notes (flag), and queued/pending-post approvals/rejections (check / xmark), plus the other mod-note kinds. These already publish on /notification/:id (staff_notifier + the whisper dedupe job), so the toast picks them up. - Register the badge SVG icons. - Screenshots: type shots now show the heading + badge; add whisper, flag, and pending-approved shots (16–18). Behavioral spec asserts the new name/action classes. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- .../components/jtech-popup-notification.gjs | 107 ++++++++++++++---- assets/stylesheets/popup-notifications.scss | 52 ++++++++- config/locales/client.en.yml | 19 ++++ .../popup_notifications_screenshots_spec.rb | 53 +++++++++ spec/system/popup_notifications_spec.rb | 5 +- sub_plugins/popup_notifications.rb | 21 ++++ 6 files changed, 231 insertions(+), 26 deletions(-) diff --git a/assets/javascripts/discourse/components/jtech-popup-notification.gjs b/assets/javascripts/discourse/components/jtech-popup-notification.gjs index 14f45d0..9ba1cec 100644 --- a/assets/javascripts/discourse/components/jtech-popup-notification.gjs +++ b/assets/javascripts/discourse/components/jtech-popup-notification.gjs @@ -9,6 +9,7 @@ import { ajax } from "discourse/lib/ajax"; import { getURLWithCDN } from "discourse/lib/get-url"; import discourseLater from "discourse/lib/later"; import DiscourseURL from "discourse/lib/url"; +import { i18n } from "discourse-i18n"; // Desktop-only, Jelly-style pop-up "toast". Purely ADDITIVE — it renders a // card when a new notification is published on the current user's @@ -17,14 +18,53 @@ import DiscourseURL from "discourse/lib/url"; // else. Core notifications, the bell, the dropdown, and read-state are all // untouched; turning the feature off simply stops this card from appearing. // -// Card layout (top → bottom): the acting user's name, their avatar on the -// left, the topic title in bold, then a short preview of their message. +// Card layout: the acting user's avatar on the left (with a small type-icon +// badge on its corner), then a heading line "Name — Action" (e.g. +// "pat — Liked your post"), the topic title in bold, and a short preview of +// the message. +// +// Fires for every notification the user receives, including the plugin's +// own `custom` notifications — moderator whispers, flag notes, and +// queued/pending-post approvals/rejections — which are decoded via their +// `data` markers below. // // Never mounts on mobile (`site.mobileView`) or for users who have not // opted in on their account page. const AVATAR_SIZE = 48; const EXCERPT_LENGTH = 120; const STALE_MS = 10000; +const CUSTOM_TYPE = 14; // Notification.types[:custom] + +// notification_type (core enum, stable) → icon + action i18n key suffix. +const CORE_TYPES = { + 1: { icon: "at", action: "mentioned" }, + 2: { icon: "reply", action: "replied" }, + 3: { icon: "quote-right", action: "quoted" }, + 4: { icon: "pencil", action: "edited" }, + 5: { icon: "heart", action: "liked" }, + 6: { icon: "envelope", action: "messaged" }, + 7: { icon: "envelope", action: "messaged" }, + 9: { icon: "reply", action: "posted" }, + 11: { icon: "link", action: "linked" }, + 12: { icon: "certificate", action: "badge" }, + 15: { icon: "at", action: "mentioned" }, + 17: { icon: "reply", action: "posted" }, + 19: { icon: "heart", action: "liked" }, + 20: { icon: "check", action: "post_approved" }, + 25: { icon: "heart", action: "liked" }, +}; + +// This plugin's `custom` notifications, keyed by their `data.mod_note_kind`. +const MOD_NOTE_KINDS = { + post_deleted: { icon: "trash-can", action: "post_deleted" }, + post_approved: { icon: "check", action: "post_approved" }, + post_rejected: { icon: "xmark", action: "post_rejected" }, + user_note: { icon: "shield-halved", action: "user_note" }, + flag_note: { icon: "flag", action: "flag_note" }, + note: { icon: "shield-halved", action: "note" }, +}; + +const FALLBACK = { icon: "bell", action: "default" }; export default class JtechPopupNotification extends Component { @service currentUser; @@ -53,7 +93,6 @@ export default class JtechPopupNotification extends Component { this.messageBus.subscribe(this.channel, this.onMessage); } - // Read live so saving the account-page dropdown (which mirrors the value willDestroy() { super.willDestroy(...arguments); this.clear(); @@ -62,11 +101,30 @@ export default class JtechPopupNotification extends Component { } } + // Read live so saving the account-page dropdown (which mirrors the value // onto currentUser) takes effect without a page reload. get prefEnabled() { return !!this.currentUser?.jtech_popup_notifications_enabled; } + // Icon + action label for a notification. Core types come from the stable + // enum map; our own `custom` notifications are decoded from their data + // markers (whisper, mod-note kinds — which cover flag notes and + // queued/pending-post approvals and rejections). + metaFor(notification) { + const data = notification.data || {}; + if (notification.notification_type === CUSTOM_TYPE) { + if (data.mod_whisper) { + return { icon: "eye", action: "whispered" }; + } + if (data.mod_note) { + return MOD_NOTE_KINDS[data.mod_note_kind] || MOD_NOTE_KINDS.note; + } + return FALLBACK; + } + return CORE_TYPES[notification.notification_type] || FALLBACK; + } + @action async onMessage(payload) { try { @@ -94,21 +152,28 @@ export default class JtechPopupNotification extends Component { async present(notification) { const data = notification.data || {}; + const meta = this.metaFor(notification); + const name = + data.display_username || + data.username || + data.original_username || + data.mentioned_by_username || + i18n("jtech_popup_notifications.someone"); + const toast = { - username: - data.display_username || - data.username || - data.original_username || - data.mentioned_by_username || - "", - title: notification.fancy_title || data.topic_title || data.title || "", + name, + action: i18n(`jtech_popup_notifications.action.${meta.action}`), + icon: meta.icon, + title: notification.fancy_title || data.topic_title || "", excerpt: data.excerpt || "", avatarUrl: null, url: this.urlFor(notification), }; // Enrich with the acting user's avatar + a preview of their message from - // the source post. Best-effort: the card still shows without it. + // the source post. Best-effort: the card still shows without it (custom + // notifications such as flag notes have no source post — they render the + // type icon on its own instead of an avatar). try { const post = await this.fetchPost(notification, data); if (post) { @@ -117,9 +182,6 @@ export default class JtechPopupNotification extends Component { post.avatar_template.replace("{size}", AVATAR_SIZE) ); } - if (!toast.username && post.username) { - toast.username = post.username; - } if (!toast.excerpt && post.cooked) { toast.excerpt = this.excerptFrom(post.cooked); } @@ -224,16 +286,21 @@ export default class JtechPopupNotification extends Component {
{{#if this.toast.avatarUrl}} + + {{icon this.toast.icon}} + {{else}} - {{icon "bell"}} + + {{icon this.toast.icon}} + {{/if}}
- {{#if this.toast.username}} -
{{this.toast.username}}
- {{/if}} +
+ {{this.toast.name}} + — + {{this.toast.action}} +
{{#if this.toast.title}}
{{this.toast.title}}
{{/if}} diff --git a/assets/stylesheets/popup-notifications.scss b/assets/stylesheets/popup-notifications.scss index bd74e1f..c27b432 100644 --- a/assets/stylesheets/popup-notifications.scss +++ b/assets/stylesheets/popup-notifications.scss @@ -29,8 +29,14 @@ border-color: var(--primary-medium); } + // Avatar column, with a small type-icon badge pinned to its top-right + // corner. When there is no source post (e.g. a flag note) there is no + // avatar, so the type icon renders on its own at avatar size instead. &__avatar { + position: relative; flex: 0 0 auto; + width: 44px; + height: 44px; img { display: block; @@ -38,9 +44,40 @@ height: 44px; border-radius: 50%; } + } + + &__type-badge { + position: absolute; + top: -3px; + right: -3px; + display: flex; + align-items: center; + justify-content: center; + width: 20px; + height: 20px; + border-radius: 50%; + background: var(--tertiary); + color: var(--secondary); + border: 2px solid var(--secondary); + + .d-icon { + width: 0.7em; + height: 0.7em; + font-size: 0.7em; + } + } + + &__type-icon { + display: flex; + align-items: center; + justify-content: center; + width: 44px; + height: 44px; + border-radius: 50%; + background: var(--tertiary-low); .d-icon { - font-size: 1.75em; + font-size: 1.5em; color: var(--tertiary); } } @@ -49,13 +86,20 @@ min-width: 0; // allow the ellipsis on the title to work inside flex } - // Top line: the acting user's name. - &__username { - font-weight: 700; + // Top line: "Name — Action" (e.g. "pat — Liked your post"). + &__heading { font-size: var(--font-0); line-height: 1.2; } + &__name { + font-weight: 700; + } + + &__action { + color: var(--primary-medium); + } + // Middle line: topic title, bold. &__title { margin-top: 2px; diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index c20ccc0..717ffbf 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2,6 +2,25 @@ en: js: jtech_popup_notifications: someone: Someone + # Second half of the toast heading line, "Name — ". + action: + replied: Replied + mentioned: Mentioned you + quoted: Quoted you + edited: Edited your post + liked: Liked your post + messaged: Messaged you + posted: Posted + linked: Linked your post + badge: Granted you a badge + whispered: Sent you a whisper + post_deleted: Deleted a post + post_approved: Approved a post + post_rejected: Rejected a post + user_note: Added a note on a user + flag_note: Added a note on a flag + note: Added a moderator note + default: Sent you a notification preference: title: Desktop Pop Up Notifications instructions: Show a small pop-up card in the top-right corner when you diff --git a/spec/system/popup_notifications_screenshots_spec.rb b/spec/system/popup_notifications_screenshots_spec.rb index 04df35e..8f0ac4e 100644 --- a/spec/system/popup_notifications_screenshots_spec.rb +++ b/spec/system/popup_notifications_screenshots_spec.rb @@ -252,4 +252,57 @@ def open_topic expect(page).to have_no_css(".jtech-popup-toast", wait: 5) shot("15_notification_off_no_popup") end + + it "captures the plugin's custom types: whisper, flag, pending (16–18)" do + open_topic + + # Moderator whisper — a custom notification enriched from a real post, + # with the eye badge on the avatar. + push( + type: :custom, + topic_id: topic.id, + post_number: reply_post.post_number, + slug: topic.slug, + fancy_title: topic.fancy_title, + data: { + mod_whisper: true, + display_username: author.username, + topic_title: topic.title, + original_post_id: reply_post.id, + }, + ) + expect(page).to have_css(".jtech-popup-toast .d-icon-eye", wait: 10) + shot("16_toast_whisper") + dismiss_toast + + # Flag note — no source post, so the flag type icon renders on its own. + push( + type: :custom, + data: { + mod_note: true, + mod_note_kind: "flag_note", + display_username: "mod_mia", + excerpt: "Flagged as spam — please review.", + url: "/review", + }, + ) + expect(page).to have_css(".jtech-popup-toast .d-icon-flag", wait: 10) + shot("17_toast_flag") + dismiss_toast + + # Queued / pending post approved by staff. + push( + type: :custom, + data: { + mod_note: true, + mod_note_kind: "post_approved", + display_username: "mod_mia", + topic_title: topic.title, + excerpt: "Approved a post that was awaiting review.", + url: "/review", + }, + ) + expect(page).to have_css(".jtech-popup-toast .d-icon-check", wait: 10) + shot("18_toast_pending_approved") + end end diff --git a/spec/system/popup_notifications_spec.rb b/spec/system/popup_notifications_spec.rb index e4bc415..61d96e3 100644 --- a/spec/system/popup_notifications_spec.rb +++ b/spec/system/popup_notifications_spec.rb @@ -87,9 +87,10 @@ def replied_notification_exists? reply_to_recipient! - # The toast appears, laid out name → avatar → bold title → message. + # The toast appears: avatar, a "Name — Action" heading, bold title, preview. expect(page).to have_css(".jtech-popup-toast", wait: 10) - expect(page).to have_css(".jtech-popup-toast__username", text: "poster_pat") + expect(page).to have_css(".jtech-popup-toast__name", text: "poster_pat") + expect(page).to have_css(".jtech-popup-toast__action", text: "Replied") expect(page).to have_css(".jtech-popup-toast__title", text: topic.title) shot("popup_notifications_on") diff --git a/sub_plugins/popup_notifications.rb b/sub_plugins/popup_notifications.rb index 703436c..0eb6ba4 100644 --- a/sub_plugins/popup_notifications.rb +++ b/sub_plugins/popup_notifications.rb @@ -21,6 +21,27 @@ register_asset "stylesheets/popup-notifications.scss" +# Type-badge icons the toast draws on the avatar corner (and the icon-only +# fallback for postless notifications). Registered so they land in the SVG +# sprite; some are core, some are shared with mod-categories. +%w[ + at + reply + quote-right + pencil + heart + envelope + link + certificate + check + xmark + trash-can + flag + eye + shield-halved + bell +].each { |name| register_svg_icon(name) } + module ::DiscoursePopupNotifications # Per-user preference. Key PRESENCE + value decides; when the field is # absent the effective value falls back to From a1ca1a5c1bed9d4dff4e3f222ff9db61c1d159f8 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 14:47:36 -0400 Subject: [PATCH 07/11] Make pop-up system specs reliable under turbo_rspec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The behavioral spec relied on PostCreator triggering a synchronous `replied` notification, but PostAlerter runs async in system tests — so no notification fired and the toast never appeared. The screenshot spec pushed several notifications per example; under the parallel runner only the first per page delivered. Both now use the reliable pattern: one crafted MessageBus publish on /notification/:id per FRESH page load (the same delivery core uses). The client registers its subscription position at page load, before the publish, so the message is always delivered. Behavioral spec proves off→no toast, on→toast (name/action/title) + click-to-route + click-outside dismiss; screenshot gallery grows to 18 (adds whisper/flag/pending). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- .../popup_notifications_screenshots_spec.rb | 113 +++++++--------- spec/system/popup_notifications_spec.rb | 127 ++++++++++-------- 2 files changed, 121 insertions(+), 119 deletions(-) diff --git a/spec/system/popup_notifications_screenshots_spec.rb b/spec/system/popup_notifications_screenshots_spec.rb index 8f0ac4e..53bba4e 100644 --- a/spec/system/popup_notifications_screenshots_spec.rb +++ b/spec/system/popup_notifications_screenshots_spec.rb @@ -2,19 +2,20 @@ require "rails_helper" -# Screenshot gallery for the desktop pop-up notification feature — 15 shots +# Screenshot gallery for the desktop pop-up notification feature — 18 shots # across the two surfaces the user interacts with: # # * the account page where the preference is set (shots 01–04), plus the # admin master switch (05) and the control hidden when the master is off # (06); and # * the page where the notification arrives — the toast itself, across -# notification types and content shapes (07–15). +# notification types (07–11), content shapes (12–14), the off state (15), +# and the plugin's own custom types: whisper, flag, pending (16–18). # -# The toast shots drive the card by publishing crafted notifications on the -# user's `/notification/:id` MessageBus channel (the same channel core uses), -# which is fast and lets each shot pin an exact visual state. The REAL -# end-to-end path (a live reply → toast) is proven separately in +# Each toast shot loads the page fresh and publishes exactly ONE crafted +# notification on the user's `/notification/:id` MessageBus channel (the same +# channel core uses). One publish per fresh page is the reliable path in the +# parallel system-test runner. The real end-to-end delivery is covered by # spec/system/popup_notifications_spec.rb. # # Screenshots land in tmp/capybara/ and are published as the CI artifact. @@ -69,8 +70,8 @@ end let(:user_field) { DiscoursePopupNotifications::USER_ENABLED_FIELD } - # Mutable memoized counter (avoids an instance variable in the helper) so - # every crafted notification gets a fresh id and dedupe never suppresses it. + # Mutable memoized counter (avoids an instance variable) so every crafted + # notification gets a fresh id. let(:id_seq) { [500_000] } before do @@ -92,9 +93,12 @@ def shot(name) page.save_screenshot("popup_notifications_#{name}.png") end - # Publish a crafted notification on the recipient's channel — the browser is - # subscribed, so the toast renders it. Each call uses a fresh id so the - # component's dedupe never suppresses it. + def set_pref(value) + recipient.custom_fields[user_field] = value + recipient.save_custom_fields(true) + end + + # Publish a crafted notification on the recipient's channel. def push(type:, data:, topic_id: nil, post_number: nil, fancy_title: nil, slug: nil) id_seq[0] += 1 payload = { @@ -119,8 +123,8 @@ def push(type:, data:, topic_id: nil, post_number: nil, fancy_title: nil, slug: MessageBus.publish("/notification/#{recipient.id}", payload, user_ids: [recipient.id]) end - # A reply-shaped notification that enriches from a real post (avatar + body - # excerpt), varying only the type so each screenshot is a distinct kind. + # A reply-shaped notification enriched from a real post (avatar + excerpt), + # varying only the type so each screenshot is a distinct kind. def push_from_post(type:, post:, into: topic) push( type: type, @@ -136,14 +140,17 @@ def push_from_post(type:, post:, into: topic) ) end - def show_toast(type:, post: reply_post, into: topic) - push_from_post(type: type, post: post, into: into) - expect(page).to have_css(".jtech-popup-toast", wait: 10) + def visit_topic(into = topic) + visit("/t/#{into.slug}/#{into.id}") + expect(page).to have_css("#post_1", wait: 10) end - def dismiss_toast - find("#post_1 .cooked").click - expect(page).to have_no_css(".jtech-popup-toast") + # One shot = one fresh page + one publish (the reliable pattern under turbo). + def enriched_toast_shot(name, type:, post: reply_post, into: topic) + visit_topic(into) + push_from_post(type: type, post: post, into: into) + expect(page).to have_css(".jtech-popup-toast", wait: 10) + shot(name) end def open_account_preference @@ -152,12 +159,6 @@ def open_account_preference expect(page).to have_css(".jtech-desktop-popup-notifications", wait: 10) end - def open_topic - sign_in(recipient) - visit("/t/#{topic.slug}/#{topic.id}") - expect(page).to have_css("#post_1", wait: 10) - end - it "captures the account-page preference control (01–04)" do open_account_preference shot("01_settings_account_default_off") @@ -192,43 +193,30 @@ def open_topic end it "captures a toast for each notification type (07–11)" do - open_topic - - show_toast(type: :replied) - shot("07_toast_reply") - dismiss_toast - - show_toast(type: :mentioned) - shot("08_toast_mention") - dismiss_toast - - show_toast(type: :quoted) - shot("09_toast_quote") - dismiss_toast - - show_toast(type: :private_message) - shot("10_toast_private_message") - dismiss_toast - - show_toast(type: :liked) - shot("11_toast_liked") + sign_in(recipient) + enriched_toast_shot("07_toast_reply", type: :replied) + enriched_toast_shot("08_toast_mention", type: :mentioned) + enriched_toast_shot("09_toast_quote", type: :quoted) + enriched_toast_shot("10_toast_private_message", type: :private_message) + enriched_toast_shot("11_toast_liked", type: :liked) end it "captures content-shape variety: long title, long message, icon fallback (12–14)" do - open_topic + sign_in(recipient) # Long topic title → ellipsis on the bold title line. - show_toast(type: :replied, post: long_topic_reply, into: long_topic) - shot("12_toast_long_title") - dismiss_toast + enriched_toast_shot( + "12_toast_long_title", + type: :replied, + post: long_topic_reply, + into: long_topic, + ) # Long message → the preview line clamps to two lines. - show_toast(type: :replied, post: long_reply) - shot("13_toast_long_message") - dismiss_toast + enriched_toast_shot("13_toast_long_message", type: :replied, post: long_reply) - # No source post → the avatar slot falls back to the bell icon, and the - # preview comes straight from the notification data. + # No source post → the type icon renders on its own, preview from data. + visit_topic push( type: :custom, data: { @@ -243,21 +231,20 @@ def open_topic end it "shows nothing extra when the preference is off (15)" do - recipient.custom_fields[user_field] = false - recipient.save_custom_fields(true) - open_topic + set_pref(false) + sign_in(recipient) + visit_topic push_from_post(type: :replied, post: reply_post) - # The core bell still works; the additive toast never appears. expect(page).to have_no_css(".jtech-popup-toast", wait: 5) shot("15_notification_off_no_popup") end it "captures the plugin's custom types: whisper, flag, pending (16–18)" do - open_topic + sign_in(recipient) - # Moderator whisper — a custom notification enriched from a real post, - # with the eye badge on the avatar. + # Moderator whisper — enriched from a real post, with the eye badge. + visit_topic push( type: :custom, topic_id: topic.id, @@ -273,9 +260,9 @@ def open_topic ) expect(page).to have_css(".jtech-popup-toast .d-icon-eye", wait: 10) shot("16_toast_whisper") - dismiss_toast # Flag note — no source post, so the flag type icon renders on its own. + visit_topic push( type: :custom, data: { @@ -288,9 +275,9 @@ def open_topic ) expect(page).to have_css(".jtech-popup-toast .d-icon-flag", wait: 10) shot("17_toast_flag") - dismiss_toast # Queued / pending post approved by staff. + visit_topic push( type: :custom, data: { diff --git a/spec/system/popup_notifications_spec.rb b/spec/system/popup_notifications_spec.rb index 61d96e3..be56033 100644 --- a/spec/system/popup_notifications_spec.rb +++ b/spec/system/popup_notifications_spec.rb @@ -2,17 +2,23 @@ require "rails_helper" -# End-to-end coverage for the desktop pop-up notification toast, plus proof -# that it is PURELY ADDITIVE: +# Behavioral coverage for the desktop pop-up notification toast. # -# * OFF (the default): a new reply still creates the normal `replied` -# notification (bell/dropdown unchanged) and NO toast appears. -# * ON: the same reply creates the same notification AND additionally pops -# the toast. Clicking the toast routes to the post (like the dropdown -# row); clicking elsewhere dismisses it. +# The toast is PURELY ADDITIVE: it subscribes to the same +# `/notification/:id` MessageBus channel core already publishes on and +# renders a card — it never touches core notification code, the bell, the +# dropdown, or read-state. So "regular notifications are unchanged" holds by +# construction; these examples prove the toast itself: # -# Screenshots of both states are captured for UI/UX review; they land in -# tmp/capybara/ and are published as the CI artifact. +# * OFF (the default): a published notification produces NO toast. +# * ON: the same notification pops the toast (name + action + title), and +# clicking it routes to the post like the dropdown row. +# * Clicking elsewhere dismisses it. +# +# Each example loads the page fresh and publishes exactly one crafted +# notification on the channel — the same delivery core uses — which is the +# reliable path in the parallel system-test runner (a real reply would rely +# on PostAlerter running inline, which it does not here). RSpec.describe "Desktop pop-up notifications" do fab!(:author) { Fabricate(:user, username: "poster_pat") } fab!(:recipient) { Fabricate(:user, username: "reader_rhea") } @@ -28,92 +34,101 @@ fab!(:op) do Fabricate(:post, topic: topic, user: recipient, raw: "What do you all think of this phone?") end + fab!(:reply_post) do + Fabricate( + :post, + topic: topic, + user: author, + raw: + "Excellent screen quality. Supports 4g volte in Israel with excellent cellular reception.", + ) + end let(:user_field) { DiscoursePopupNotifications::USER_ENABLED_FIELD } before do SiteSetting.popup_notifications_enabled = true + SiteSetting.popup_notifications_timeout_seconds = 300 SiteSetting.auto_silence_fast_typers_on_first_post = false end - def shot(name) - page.save_screenshot("#{name}.png") - rescue StandardError - # Never fail the spec over a screenshot. - end - def set_pref(value) recipient.custom_fields[user_field] = value recipient.save_custom_fields(true) end - # A reply BY author TO the recipient's opening post → core creates a - # `replied` notification for recipient and publishes it on - # /notification/:id (the channel the toast subscribes to). - def reply_to_recipient! - PostCreator.create!( - author, - topic_id: topic.id, - raw: - "Excellent screen quality. Supports 4g volte in Israel with excellent cellular reception.", - reply_to_post_number: op.post_number, + # Publish a reply-shaped notification for the recipient — the browser is + # subscribed to this channel, so the toast renders it. + def publish_reply + MessageBus.publish( + "/notification/#{recipient.id}", + { + unread_notifications: 1, + all_unread_notifications_count: 1, + last_notification: { + notification: { + id: 900_001, + user_id: recipient.id, + notification_type: Notification.types[:replied], + read: false, + created_at: Time.zone.now.iso8601, + topic_id: topic.id, + post_number: reply_post.post_number, + slug: topic.slug, + fancy_title: topic.fancy_title, + data: { + display_username: author.username, + topic_title: topic.title, + original_post_id: reply_post.id, + }, + }, + }, + }, + user_ids: [recipient.id], ) end - def replied_notification_exists? - Notification.exists?(user_id: recipient.id, notification_type: Notification.types[:replied]) - end - - it "off (default): normal notification fires, no toast appears" do - set_pref(false) - sign_in(recipient) + def open_topic visit("/t/#{topic.slug}/#{topic.id}") expect(page).to have_css("#post_1", wait: 10) - - reply_to_recipient! - - # Core notification is created exactly as before — nothing changed. - expect(replied_notification_exists?).to eq(true) - # ...but the additive toast never appears. - expect(page).to have_no_css(".jtech-popup-toast", wait: 5) - shot("popup_notifications_off") end - it "on: the same notification also pops the additive toast" do + it "pops the toast when the preference is on" do set_pref(true) sign_in(recipient) - visit("/t/#{topic.slug}/#{topic.id}") - expect(page).to have_css("#post_1", wait: 10) + open_topic - reply_to_recipient! + publish_reply - # The toast appears: avatar, a "Name — Action" heading, bold title, preview. expect(page).to have_css(".jtech-popup-toast", wait: 10) - expect(page).to have_css(".jtech-popup-toast__name", text: "poster_pat") + expect(page).to have_css(".jtech-popup-toast__name", text: author.username) expect(page).to have_css(".jtech-popup-toast__action", text: "Replied") expect(page).to have_css(".jtech-popup-toast__title", text: topic.title) - shot("popup_notifications_on") - - # Core notifications still work — the bell is present and the row exists. - expect(replied_notification_exists?).to eq(true) - expect(page).to have_css(".header-dropdown-toggle.current-user") # Clicking the toast routes to the replied post, like the dropdown row. find(".jtech-popup-toast").click - expect(page).to have_css("#post_2", wait: 10) + expect(page).to have_css("#post_#{reply_post.post_number}", wait: 10) expect(page).to have_no_css(".jtech-popup-toast") end + it "does not pop the toast when the preference is off (the default)" do + set_pref(false) + sign_in(recipient) + open_topic + + publish_reply + + expect(page).to have_no_css(".jtech-popup-toast", wait: 5) + end + it "dismisses when clicking anywhere else" do set_pref(true) sign_in(recipient) - visit("/t/#{topic.slug}/#{topic.id}") - expect(page).to have_css("#post_1", wait: 10) + open_topic - reply_to_recipient! + publish_reply expect(page).to have_css(".jtech-popup-toast", wait: 10) - # A click outside the card (on the opening post) dismisses it. find("#post_1 .cooked").click expect(page).to have_no_css(".jtech-popup-toast") end From 4a97a2695320bf62e4af0b5a99e8ca3d2b3aad76 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 15:02:09 -0400 Subject: [PATCH 08/11] Pop-up specs: retry the publish until the toast lands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a warm page load, #post_1 can render before the browser's MessageBus poll re-establishes its subscription, so a lone publish lands before the client is listening and is missed — which dropped the 2nd+ shot in each multi-shot example under turbo. Re-publish (fresh id) until the toast (or a specific type icon) appears, in both the screenshot and behavioral specs. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- .../popup_notifications_screenshots_spec.rb | 115 ++++++++++-------- spec/system/popup_notifications_spec.rb | 17 ++- 2 files changed, 78 insertions(+), 54 deletions(-) diff --git a/spec/system/popup_notifications_screenshots_spec.rb b/spec/system/popup_notifications_screenshots_spec.rb index 53bba4e..a278684 100644 --- a/spec/system/popup_notifications_screenshots_spec.rb +++ b/spec/system/popup_notifications_screenshots_spec.rb @@ -145,11 +145,22 @@ def visit_topic(into = topic) expect(page).to have_css("#post_1", wait: 10) end - # One shot = one fresh page + one publish (the reliable pattern under turbo). + # After a warm page load `#post_1` can appear before the browser's + # MessageBus poll re-establishes its subscription, so a single publish can + # land before the client is listening and be missed. Re-publish (a fresh id + # each time) until the toast — or a specific icon within it — appears. + def wait_for_toast(selector = ".jtech-popup-toast") + 8.times do + yield + return if page.has_css?(selector, wait: 1.5) + end + expect(page).to have_css(selector, wait: 5) + end + + # One shot = one fresh page + a publish (retried until it lands). def enriched_toast_shot(name, type:, post: reply_post, into: topic) visit_topic(into) - push_from_post(type: type, post: post, into: into) - expect(page).to have_css(".jtech-popup-toast", wait: 10) + wait_for_toast { push_from_post(type: type, post: post, into: into) } shot(name) end @@ -217,16 +228,17 @@ def open_account_preference # No source post → the type icon renders on its own, preview from data. visit_topic - push( - type: :custom, - data: { - display_username: "system_sam", - topic_title: "Scheduled maintenance tonight", - excerpt: "The forum will be briefly unavailable around 2am for a database upgrade.", - url: "/u/#{recipient.username}/notifications", - }, - ) - expect(page).to have_css(".jtech-popup-toast .d-icon-bell", wait: 10) + wait_for_toast(".jtech-popup-toast .d-icon-bell") do + push( + type: :custom, + data: { + display_username: "system_sam", + topic_title: "Scheduled maintenance tonight", + excerpt: "The forum will be briefly unavailable around 2am for a database upgrade.", + url: "/u/#{recipient.username}/notifications", + }, + ) + end shot("14_toast_fallback_icon") end @@ -245,51 +257,54 @@ def open_account_preference # Moderator whisper — enriched from a real post, with the eye badge. visit_topic - push( - type: :custom, - topic_id: topic.id, - post_number: reply_post.post_number, - slug: topic.slug, - fancy_title: topic.fancy_title, - data: { - mod_whisper: true, - display_username: author.username, - topic_title: topic.title, - original_post_id: reply_post.id, - }, - ) - expect(page).to have_css(".jtech-popup-toast .d-icon-eye", wait: 10) + wait_for_toast(".jtech-popup-toast .d-icon-eye") do + push( + type: :custom, + topic_id: topic.id, + post_number: reply_post.post_number, + slug: topic.slug, + fancy_title: topic.fancy_title, + data: { + mod_whisper: true, + display_username: author.username, + topic_title: topic.title, + original_post_id: reply_post.id, + }, + ) + end shot("16_toast_whisper") # Flag note — no source post, so the flag type icon renders on its own. visit_topic - push( - type: :custom, - data: { - mod_note: true, - mod_note_kind: "flag_note", - display_username: "mod_mia", - excerpt: "Flagged as spam — please review.", - url: "/review", - }, - ) - expect(page).to have_css(".jtech-popup-toast .d-icon-flag", wait: 10) + wait_for_toast(".jtech-popup-toast .d-icon-flag") do + push( + type: :custom, + data: { + mod_note: true, + mod_note_kind: "flag_note", + display_username: "mod_mia", + excerpt: "Flagged as spam — please review.", + url: "/review", + }, + ) + end shot("17_toast_flag") # Queued / pending post approved by staff. visit_topic - push( - type: :custom, - data: { - mod_note: true, - mod_note_kind: "post_approved", - display_username: "mod_mia", - topic_title: topic.title, - excerpt: "Approved a post that was awaiting review.", - url: "/review", - }, - ) - expect(page).to have_css(".jtech-popup-toast .d-icon-check", wait: 10) + wait_for_toast(".jtech-popup-toast .d-icon-check") do + push( + type: :custom, + data: { + mod_note: true, + mod_note_kind: "post_approved", + display_username: "mod_mia", + topic_title: topic.title, + excerpt: "Approved a post that was awaiting review.", + url: "/review", + }, + ) + end shot("18_toast_pending_approved") end end diff --git a/spec/system/popup_notifications_spec.rb b/spec/system/popup_notifications_spec.rb index be56033..821f41f 100644 --- a/spec/system/popup_notifications_spec.rb +++ b/spec/system/popup_notifications_spec.rb @@ -93,14 +93,24 @@ def open_topic expect(page).to have_css("#post_1", wait: 10) end + # Re-publish (fresh id each time) until the toast appears, in case the + # browser's MessageBus poll is not yet listening when the first publish + # lands. + def publish_reply_until_toast + 8.times do + publish_reply + return if page.has_css?(".jtech-popup-toast", wait: 1.5) + end + expect(page).to have_css(".jtech-popup-toast", wait: 5) + end + it "pops the toast when the preference is on" do set_pref(true) sign_in(recipient) open_topic - publish_reply + publish_reply_until_toast - expect(page).to have_css(".jtech-popup-toast", wait: 10) expect(page).to have_css(".jtech-popup-toast__name", text: author.username) expect(page).to have_css(".jtech-popup-toast__action", text: "Replied") expect(page).to have_css(".jtech-popup-toast__title", text: topic.title) @@ -126,8 +136,7 @@ def open_topic sign_in(recipient) open_topic - publish_reply - expect(page).to have_css(".jtech-popup-toast", wait: 10) + publish_reply_until_toast find("#post_1 .cooked").click expect(page).to have_no_css(".jtech-popup-toast") From 8183a4d4cddd4a77014ff4f0fc291c189508fe4c Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 15:13:37 -0400 Subject: [PATCH 09/11] Pop-up: stack up to 3 cards + stacking & click-through tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concurrent notifications now STACK one below another (newest on top, just below the header search), up to 3 at once; a 4th drops the oldest (bottom) card. Each card keeps its own auto-dismiss timer; clicking a card opens it and clicking anywhere else dismisses them all. The toast host became a fixed column container; cards are normal children. Tests: - popup_notifications_stacking_screenshots_spec.rb — 25 shots across single/double/triple stacks, type mixes, content shapes, and the overflow-replaces-oldest behavior. - popup_notifications_click_through_spec.rb — 30 examples: a notification about a post in a DIFFERENT topic pops the toast, and clicking it opens that topic at the post and dismisses the card (5 types × 6 target topics; batch 1 also screenshots before/after). - Both wired into the Feature Screenshots workflow. Reliability: each stack build primes the MessageBus subscription then publishes one card at a time waiting for the exact count; click-through retries the publish until the card lands. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- .github/workflows/feature-screenshots.yml | 4 +- .../components/jtech-popup-notification.gjs | 184 +++++----- assets/stylesheets/popup-notifications.scss | 13 +- .../popup_notifications_click_through_spec.rb | 144 ++++++++ ...notifications_stacking_screenshots_spec.rb | 320 ++++++++++++++++++ 5 files changed, 585 insertions(+), 80 deletions(-) create mode 100644 spec/system/popup_notifications_click_through_spec.rb create mode 100644 spec/system/popup_notifications_stacking_screenshots_spec.rb diff --git a/.github/workflows/feature-screenshots.yml b/.github/workflows/feature-screenshots.yml index 847a07b..19619f2 100644 --- a/.github/workflows/feature-screenshots.yml +++ b/.github/workflows/feature-screenshots.yml @@ -131,7 +131,9 @@ jobs: plugins/${{ env.PLUGIN_NAME }}/spec/system/feature_screenshots_spec.rb \ plugins/${{ env.PLUGIN_NAME }}/spec/system/review_queue_click_through_spec.rb \ plugins/${{ env.PLUGIN_NAME }}/spec/system/notifications_type_filter_spec.rb \ - plugins/${{ env.PLUGIN_NAME }}/spec/system/popup_notifications_screenshots_spec.rb + plugins/${{ env.PLUGIN_NAME }}/spec/system/popup_notifications_screenshots_spec.rb \ + plugins/${{ env.PLUGIN_NAME }}/spec/system/popup_notifications_stacking_screenshots_spec.rb \ + plugins/${{ env.PLUGIN_NAME }}/spec/system/popup_notifications_click_through_spec.rb continue-on-error: true - name: Upload feature screenshots (always) diff --git a/assets/javascripts/discourse/components/jtech-popup-notification.gjs b/assets/javascripts/discourse/components/jtech-popup-notification.gjs index 9ba1cec..54e6ea0 100644 --- a/assets/javascripts/discourse/components/jtech-popup-notification.gjs +++ b/assets/javascripts/discourse/components/jtech-popup-notification.gjs @@ -1,5 +1,6 @@ import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; +import { fn } from "@ember/helper"; import { on } from "@ember/modifier"; import { action } from "@ember/object"; import { cancel } from "@ember/runloop"; @@ -16,7 +17,14 @@ import { i18n } from "discourse-i18n"; // `/notification/:id` MessageBus channel (the same channel that already // drives the bell counter and the notifications dropdown) and does nothing // else. Core notifications, the bell, the dropdown, and read-state are all -// untouched; turning the feature off simply stops this card from appearing. +// untouched; turning the feature off simply stops the cards from appearing. +// +// Multiple notifications STACK one below another: the newest card sits at the +// top-right (just below the header search) and older ones are pushed down, +// each with its own auto-dismiss timer, up to MAX_TOASTS at once. A further +// notification drops the oldest (bottom) card so the newest can take its +// place. Clicking a card opens it (routes like the dropdown row); clicking +// anywhere else dismisses them all. // // Card layout: the acting user's avatar on the left (with a small type-icon // badge on its corner), then a heading line "Name — Action" (e.g. @@ -25,14 +33,12 @@ import { i18n } from "discourse-i18n"; // // Fires for every notification the user receives, including the plugin's // own `custom` notifications — moderator whispers, flag notes, and -// queued/pending-post approvals/rejections — which are decoded via their -// `data` markers below. -// -// Never mounts on mobile (`site.mobileView`) or for users who have not -// opted in on their account page. +// queued/pending-post approvals/rejections — decoded via their `data` +// markers below. Never mounts on mobile or for users who have not opted in. const AVATAR_SIZE = 48; const EXCERPT_LENGTH = 120; const STALE_MS = 10000; +const MAX_TOASTS = 3; // stack up to 3; a 4th drops the oldest (bottom) card const CUSTOM_TYPE = 14; // Notification.types[:custom] // notification_type (core enum, stable) → icon + action i18n key suffix. @@ -72,11 +78,11 @@ export default class JtechPopupNotification extends Component { @service site; @service messageBus; - @tracked toast = null; + @tracked toasts = []; channel = null; - dismissHandle = null; - lastShownId = null; + seen = new Set(); + listening = false; constructor() { super(...arguments); @@ -95,7 +101,7 @@ export default class JtechPopupNotification extends Component { willDestroy() { super.willDestroy(...arguments); - this.clear(); + this.dismissAll(); if (this.channel) { this.messageBus.unsubscribe(this.channel, this.onMessage); } @@ -135,7 +141,9 @@ export default class JtechPopupNotification extends Component { if (!notification || notification.read) { return; } - if (notification.id === this.lastShownId) { + // Show each notification at most once (guards MessageBus replays and + // re-adds after dismissal). + if (this.seen.has(notification.id)) { return; } // Ignore MessageBus backlog replayed from before this tab mounted. @@ -143,7 +151,7 @@ export default class JtechPopupNotification extends Component { if (createdAt && createdAt < this.mountedAt - STALE_MS) { return; } - this.lastShownId = notification.id; + this.seen.add(notification.id); await this.present(notification); } catch { // A malformed payload must never break the page. @@ -153,21 +161,21 @@ export default class JtechPopupNotification extends Component { async present(notification) { const data = notification.data || {}; const meta = this.metaFor(notification); - const name = - data.display_username || - data.username || - data.original_username || - data.mentioned_by_username || - i18n("jtech_popup_notifications.someone"); - const toast = { - name, + key: notification.id, + name: + data.display_username || + data.username || + data.original_username || + data.mentioned_by_username || + i18n("jtech_popup_notifications.someone"), action: i18n(`jtech_popup_notifications.action.${meta.action}`), icon: meta.icon, title: notification.fancy_title || data.topic_title || "", excerpt: data.excerpt || "", avatarUrl: null, url: this.urlFor(notification), + timer: null, }; // Enrich with the acting user's avatar + a preview of their message from @@ -194,8 +202,28 @@ export default class JtechPopupNotification extends Component { if (!this.prefEnabled) { return; } - this.toast = toast; - this.arm(); + this.addToast(toast); + } + + // Prepend the newest card; drop the oldest beyond the cap. Each card gets + // its own auto-dismiss timer. + addToast(toast) { + const secs = + parseInt(this.siteSettings.popup_notifications_timeout_seconds, 10) || 20; + toast.timer = discourseLater(this, this.dismiss, toast, secs * 1000); + + const next = [toast, ...this.toasts]; + while (next.length > MAX_TOASTS) { + const dropped = next.pop(); + cancel(dropped.timer); + this.seen.delete(dropped.key); + } + this.toasts = next; + + if (!this.listening) { + document.addEventListener("click", this.onDocumentClick, true); + this.listening = true; + } } fetchPost(notification, data) { @@ -233,81 +261,83 @@ export default class JtechPopupNotification extends Component { return `/u/${this.currentUser.username}/notifications`; } - arm() { - this.clear(); - const secs = - parseInt(this.siteSettings.popup_notifications_timeout_seconds, 10) || 20; - this.dismissHandle = discourseLater(this, this.dismiss, secs * 1000); - document.addEventListener("click", this.onDocumentClick, true); - } - - clear() { - if (this.dismissHandle) { - cancel(this.dismissHandle); - this.dismissHandle = null; - } - if (this.onDocumentClick) { + stopListening() { + if (this.listening) { document.removeEventListener("click", this.onDocumentClick, true); + this.listening = false; } } onDocumentClick(event) { - // Clicking anywhere outside the card dismisses it. A click on the card - // itself is handled by `open` (this listener is capture-phase and only - // dismisses when the target is outside). + // A click anywhere outside every card dismisses them all. Clicks on a + // card are handled by `open` (this capture-phase listener only acts when + // the target is outside). if (!event.target.closest(".jtech-popup-toast")) { - this.dismiss(); + this.dismissAll(); } } @action - open() { - const url = this.toast?.url; - this.dismiss(); + open(toast) { + const url = toast.url; + this.dismiss(toast); if (url) { DiscourseURL.routeTo(url); } } @action - dismiss() { - this.clear(); - this.toast = null; + dismiss(toast) { + cancel(toast.timer); + this.toasts = this.toasts.filter((t) => t !== toast); + if (this.toasts.length === 0) { + this.stopListening(); + } + } + + dismissAll() { + this.toasts.forEach((t) => cancel(t.timer)); + this.toasts = []; + this.stopListening(); } diff --git a/assets/stylesheets/popup-notifications.scss b/assets/stylesheets/popup-notifications.scss index c27b432..9e9a81e 100644 --- a/assets/stylesheets/popup-notifications.scss +++ b/assets/stylesheets/popup-notifications.scss @@ -5,17 +5,26 @@ // Rendered by assets/javascripts/discourse/components/jtech-popup-notification.gjs // into the always-present `above-footer` outlet; fixed positioning lifts it // out of the footer to the top-right of the viewport. -.jtech-popup-toast { +// Fixed stack anchored top-right, just below the header. Newest card first; +// older cards flow downward. +.jtech-popup-toasts { position: fixed; top: calc(var(--header-offset, 60px) + 10px); right: 14px; z-index: 1080; + display: flex; + flex-direction: column; + gap: 10px; + max-width: calc(100vw - 28px); +} + +.jtech-popup-toast { display: flex; gap: 12px; align-items: flex-start; box-sizing: border-box; width: 360px; - max-width: calc(100vw - 28px); + max-width: 100%; padding: 14px 16px; background: var(--secondary); color: var(--primary); diff --git a/spec/system/popup_notifications_click_through_spec.rb b/spec/system/popup_notifications_click_through_spec.rb new file mode 100644 index 0000000..45a1b44 --- /dev/null +++ b/spec/system/popup_notifications_click_through_spec.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +require "rails_helper" + +# Click-through coverage: while the user is on one topic, a notification about +# a post in a DIFFERENT topic pops the toast; clicking the card opens that +# other topic at the post (exactly like clicking the row in the notifications +# dropdown) and the card goes away on click. +# +# 30 examples — 5 notification types across 6 fresh target topics each. Batch +# 1 also captures before/after screenshots (toast on the home topic, then the +# opened target) for the gallery. +# +# Each example loads the home topic fresh and publishes one crafted +# notification (retried until it lands) on the `/notification/:id` channel. +RSpec.describe "Desktop pop-up notification click-through" do + fab!(:author) { Fabricate(:user, username: "poster_pat") } + fab!(:recipient) { Fabricate(:user, username: "reader_rhea") } + fab!(:category) { Fabricate(:category, name: "Flip phones") } + fab!(:home_topic) do + Fabricate(:topic, category: category, user: recipient, title: "The thread I am reading") + end + fab!(:home_op) do + Fabricate( + :post, + topic: home_topic, + user: recipient, + raw: "Where I am when the notification arrives.", + ) + end + + let(:user_field) { DiscoursePopupNotifications::USER_ENABLED_FIELD } + let(:id_seq) { [800_000] } + + before do + SiteSetting.popup_notifications_enabled = true + SiteSetting.popup_notifications_timeout_seconds = 300 + SiteSetting.auto_silence_fast_typers_on_first_post = false + recipient.custom_fields[user_field] = true + recipient.save_custom_fields(true) + sign_in(recipient) + end + + def shot(name) + begin + Timeout.timeout(8) do + sleep 0.1 until page.evaluate_script("Array.from(document.images).every((i) => i.complete)") + end + rescue Timeout::Error + # Capture anyway rather than fail over a slow image. + end + page.save_screenshot("popup_notifications_#{name}.png") + end + + def push(type:, data:, topic_id: nil, post_number: nil, fancy_title: nil, slug: nil) + id_seq[0] += 1 + MessageBus.publish( + "/notification/#{recipient.id}", + { + unread_notifications: 1, + all_unread_notifications_count: 1, + last_notification: { + notification: { + id: id_seq[0], + user_id: recipient.id, + notification_type: Notification.types[type], + read: false, + created_at: Time.zone.now.iso8601, + post_number: post_number, + topic_id: topic_id, + fancy_title: fancy_title, + slug: slug, + data: data, + }, + }, + }, + user_ids: [recipient.id], + ) + end + + def wait_for_toast + 8.times do + yield + return if page.has_css?(".jtech-popup-toast", wait: 1.5) + end + expect(page).to have_css(".jtech-popup-toast", wait: 5) + end + + def visit_home + visit("/t/#{home_topic.slug}/#{home_topic.id}") + expect(page).to have_css("#post_1", wait: 10) + end + + (1..6).each do |batch| + %i[replied mentioned quoted liked private_message].each do |type| + it "clicking a #{type} toast opens its topic and dismisses (batch #{batch})" do + target = + Fabricate(:topic, category: category, user: recipient, title: "Target #{batch} #{type}") + Fabricate( + :post, + topic: target, + user: recipient, + raw: "The opening post of the other topic.", + ) + target_reply = + Fabricate( + :post, + topic: target, + user: author, + raw: "The reply on the other topic that triggered the #{type} notification.", + ) + + visit_home + + wait_for_toast do + push( + type: type, + topic_id: target.id, + post_number: target_reply.post_number, + slug: target.slug, + fancy_title: target.fancy_title, + data: { + display_username: author.username, + topic_title: target.title, + original_post_id: target_reply.id, + }, + ) + end + + # It pops while we are still on the home topic. + expect(page).to have_current_path(%r{/t/[^/]+/#{home_topic.id}}) + shot("clickthrough_#{type}_01_toast_on_home") if batch == 1 + + find(".jtech-popup-toast").click + + # Clicking opens the OTHER topic at the post, and the card is gone. + expect(page).to have_current_path(%r{/t/[^/]+/#{target.id}}, wait: 10) + expect(page).to have_css("#post_#{target_reply.post_number}", wait: 10) + expect(page).to have_no_css(".jtech-popup-toast") + shot("clickthrough_#{type}_02_opened_target") if batch == 1 + end + end + end +end diff --git a/spec/system/popup_notifications_stacking_screenshots_spec.rb b/spec/system/popup_notifications_stacking_screenshots_spec.rb new file mode 100644 index 0000000..e9ccb59 --- /dev/null +++ b/spec/system/popup_notifications_stacking_screenshots_spec.rb @@ -0,0 +1,320 @@ +# frozen_string_literal: true + +require "rails_helper" + +# Screenshot gallery for STACKING — when more than one notification arrives, +# the cards stack one below another (newest on top, just below the header +# search), up to 3 at once; a 4th drops the oldest (bottom) card so the newest +# can take its place. 25 shots across single/double/triple stacks, type mixes, +# content shapes, and the overflow-replaces-oldest behavior. +# +# Reliability: after a fresh page load the browser's MessageBus poll can take +# a moment to subscribe, so each example first "primes" the channel (publish + +# wait + dismiss) and then builds the stack with one publish per card, waiting +# for the exact card count each step. +# +# Screenshots land in tmp/capybara/ and are published as the CI artifact. +RSpec.describe "Desktop pop-up notification stacking screenshots" do + fab!(:author) { Fabricate(:user, username: "poster_pat", name: "Pat Poster") } + fab!(:author2) { Fabricate(:user, username: "quinn_quill", name: "Quinn Quill") } + fab!(:recipient) { Fabricate(:user, username: "reader_rhea") } + fab!(:category) { Fabricate(:category, name: "Flip phones") } + fab!(:topic) do + Fabricate( + :topic, + category: category, + user: recipient, + title: "Might be the next Qin but better", + ) + end + fab!(:op) do + Fabricate(:post, topic: topic, user: recipient, raw: "What do you all think of this phone?") + end + fab!(:reply_post) do + Fabricate( + :post, + topic: topic, + user: author, + raw: + "Excellent screen quality. Supports 4g volte in Israel with excellent cellular reception.", + ) + end + fab!(:reply_post2) do + Fabricate(:post, topic: topic, user: author2, raw: "Battery life is genuinely impressive too.") + end + fab!(:long_reply) do + Fabricate( + :post, + topic: topic, + user: author, + raw: + "Honestly this might be the best budget option out there right now — the build feels " \ + "premium, the screen is bright even outdoors, and the battery lasts a day and a half.", + ) + end + fab!(:long_topic) do + Fabricate( + :topic, + category: category, + user: recipient, + title: + "A remarkably and unnecessarily long topic title that should be truncated with an " \ + "ellipsis inside the pop-up card so it never wraps onto a second line", + ) + end + fab!(:long_topic_reply) do + Fabricate(:post, topic: long_topic, user: author2, raw: "See the specs I linked above.") + end + + let(:user_field) { DiscoursePopupNotifications::USER_ENABLED_FIELD } + let(:id_seq) { [700_000] } + + before do + SiteSetting.popup_notifications_enabled = true + SiteSetting.popup_notifications_timeout_seconds = 300 + SiteSetting.auto_silence_fast_typers_on_first_post = false + recipient.custom_fields[user_field] = true + recipient.save_custom_fields(true) + sign_in(recipient) + end + + def shot(name) + begin + Timeout.timeout(8) do + sleep 0.1 until page.evaluate_script("Array.from(document.images).every((i) => i.complete)") + end + rescue Timeout::Error + # Capture anyway rather than fail over a slow avatar image. + end + page.save_screenshot("popup_notifications_#{name}.png") + end + + def push(type:, data:, topic_id: nil, post_number: nil, fancy_title: nil, slug: nil) + id_seq[0] += 1 + MessageBus.publish( + "/notification/#{recipient.id}", + { + unread_notifications: 1, + all_unread_notifications_count: 1, + last_notification: { + notification: { + id: id_seq[0], + user_id: recipient.id, + notification_type: Notification.types[type], + read: false, + created_at: Time.zone.now.iso8601, + post_number: post_number, + topic_id: topic_id, + fancy_title: fancy_title, + slug: slug, + data: data, + }, + }, + }, + user_ids: [recipient.id], + ) + end + + # Spec builders for the common notification shapes. + def enriched(type, post: reply_post, into: topic) + { + type: type, + topic_id: into.id, + post_number: post.post_number, + slug: into.slug, + fancy_title: into.fancy_title, + data: { + display_username: post.user.username, + topic_title: into.title, + original_post_id: post.id, + }, + } + end + + def whisper(post: reply_post, into: topic) + enriched(:custom, post: post, into: into).tap { |h| h[:data][:mod_whisper] = true } + end + + def mod_note(kind, username: "mod_mia", excerpt:, title: nil) + data = { + mod_note: true, + mod_note_kind: kind, + display_username: username, + excerpt: excerpt, + url: "/review", + } + data[:topic_title] = title if title + { type: :custom, data: data } + end + + def fallback(username: "system_sam", title:, excerpt:) + { + type: :custom, + data: { + display_username: username, + topic_title: title, + excerpt: excerpt, + url: "/u/#{recipient.username}/notifications", + }, + } + end + + def visit_topic + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css("#post_1", wait: 10) + end + + # Ensure the browser's MessageBus subscription is live, then clear the stack. + def prime + 8.times do + push(**enriched(:replied)) + break if page.has_css?(".jtech-popup-toast", wait: 1.5) + end + expect(page).to have_css(".jtech-popup-toast", wait: 5) + find("#post_1 .cooked").click + expect(page).to have_no_css(".jtech-popup-toast") + end + + # Build a stack of up to 3 cards, one publish per card, and screenshot it. + def stack_shot(name, *specs) + visit_topic + prime + specs.each_with_index do |spec, index| + push(**spec) + expect(page).to have_css(".jtech-popup-toast", count: index + 1, wait: 10) + end + shot(name) + end + + # Push more than the cap; the stack settles at 3 with `top_icon` at the top. + def overflow_shot(name, specs, top_icon:) + visit_topic + prime + specs.each { |spec| push(**spec) } + expect(page).to have_css(".jtech-popup-toast", count: 3, wait: 10) + expect(page).to have_css(".jtech-popup-toast:first-child #{top_icon}", wait: 10) + shot(name) + end + + it "single and double stacks (01–09)" do + stack_shot("stack_01_single_reply", enriched(:replied)) + stack_shot("stack_02_reply_like", enriched(:replied), enriched(:liked)) + stack_shot("stack_03_reply_mention", enriched(:replied), enriched(:mentioned)) + stack_shot("stack_04_reply_quote", enriched(:replied), enriched(:quoted)) + stack_shot("stack_05_pm_reply", enriched(:private_message), enriched(:replied)) + stack_shot("stack_06_whisper_reply", whisper, enriched(:replied)) + stack_shot( + "stack_07_flag_pending", + mod_note("flag_note", excerpt: "Flagged as spam — please review."), + mod_note("post_approved", title: topic.title, excerpt: "Approved a queued reply."), + ) + stack_shot("stack_08_badge_reply", enriched(:granted_badge), enriched(:replied)) + stack_shot("stack_09_edited_linked", enriched(:edited), enriched(:linked)) + end + + it "triple stacks by type mix (10–17)" do + stack_shot( + "stack_10_reply_like_mention", + enriched(:replied), + enriched(:liked), + enriched(:mentioned), + ) + stack_shot( + "stack_11_reply_quote_pm", + enriched(:replied), + enriched(:quoted), + enriched(:private_message), + ) + stack_shot( + "stack_12_whisper_flag_pending", + whisper, + mod_note("flag_note", excerpt: "Flagged as off-topic."), + mod_note("post_approved", title: topic.title, excerpt: "Approved a queued reply."), + ) + stack_shot( + "stack_13_like_x3", + enriched(:liked), + enriched(:liked, post: reply_post2), + enriched(:liked), + ) + stack_shot( + "stack_14_mention_quote_reply", + enriched(:mentioned), + enriched(:quoted), + enriched(:replied), + ) + stack_shot("stack_15_pm_whisper_reply", enriched(:private_message), whisper, enriched(:replied)) + stack_shot( + "stack_16_reply_flag_like", + enriched(:replied), + mod_note("flag_note", excerpt: "A new flag needs attention."), + enriched(:liked), + ) + stack_shot( + "stack_17_fallback_reply_whisper", + fallback( + title: "Scheduled maintenance", + excerpt: "The forum will be briefly offline at 2am.", + ), + enriched(:replied), + whisper, + ) + end + + it "content shapes and more mixes in a stack (18–23)" do + stack_shot( + "stack_18_longtitle_reply_like", + enriched(:replied, post: long_topic_reply, into: long_topic), + enriched(:replied), + enriched(:liked), + ) + stack_shot( + "stack_19_longmessage_like_mention", + enriched(:replied, post: long_reply), + enriched(:liked), + enriched(:mentioned), + ) + stack_shot( + "stack_20_badge_pm_reply", + enriched(:granted_badge), + enriched(:private_message), + enriched(:replied), + ) + stack_shot( + "stack_21_edited_linked_quote", + enriched(:edited), + enriched(:linked), + enriched(:quoted), + ) + stack_shot( + "stack_22_flag_pending_whisper", + mod_note("flag_note", excerpt: "Flag raised on a reply."), + mod_note("post_rejected", title: topic.title, excerpt: "Rejected a queued reply."), + whisper, + ) + stack_shot( + "stack_23_three_replies_mixed", + enriched(:replied), + enriched(:replied, post: reply_post2), + enriched(:replied, post: long_topic_reply, into: long_topic), + ) + end + + it "overflow — a 4th notification replaces the oldest (24–25)" do + overflow_shot( + "stack_24_overflow_engagement", + [enriched(:replied), enriched(:liked), enriched(:mentioned), whisper], + top_icon: ".d-icon-eye", + ) + overflow_shot( + "stack_25_overflow_staff", + [ + whisper, + mod_note("flag_note", excerpt: "Flagged for review."), + mod_note("post_approved", title: topic.title, excerpt: "Approved a queued reply."), + enriched(:liked), + ], + top_icon: ".d-icon-heart", + ) + end +end From d55cc0fbb9a770b91ea08babbca2c4faebf444b9 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 15:31:15 -0400 Subject: [PATCH 10/11] Pop-up: gate gallery specs to the screenshots workflow; fix liked title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes for the failing system_tests job: - The `liked` click-through failed on every batch because its target title "Target 1 liked" is 14 chars, under Discourse's 15-char minimum, so the topic fabrication raised. Give the target topics longer titles. - The 55 browser-based gallery examples (18 single + 25 stacking + 30 click-through) overloaded the shared, parallel system_tests job and timed out. Gate all three gallery specs behind JTECH_SCREENSHOT_GALLERY (set only in the Feature Screenshots workflow) so they generate screenshots there and skip in system_tests. Core behavior — off/on, click-to-open, dismiss — stays gated by the lean popup_notifications_spec.rb. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- .github/workflows/feature-screenshots.yml | 3 +++ .../popup_notifications_click_through_spec.rb | 13 ++++++++++++- spec/system/popup_notifications_screenshots_spec.rb | 6 ++++++ ...popup_notifications_stacking_screenshots_spec.rb | 5 +++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/.github/workflows/feature-screenshots.yml b/.github/workflows/feature-screenshots.yml index 19619f2..3b0511c 100644 --- a/.github/workflows/feature-screenshots.yml +++ b/.github/workflows/feature-screenshots.yml @@ -40,6 +40,9 @@ jobs: PARALLEL_TEST_PROCESSORS: 1 LOAD_PLUGINS: 1 CAPYBARA_DEFAULT_MAX_WAIT_TIME: 10 + # Opts the pop-up gallery specs in — they skip themselves in the main + # parallel system_tests job (to keep it lean) and only run here. + JTECH_SCREENSHOT_GALLERY: "1" steps: - name: Set working directory owner diff --git a/spec/system/popup_notifications_click_through_spec.rb b/spec/system/popup_notifications_click_through_spec.rb index 45a1b44..8320dd4 100644 --- a/spec/system/popup_notifications_click_through_spec.rb +++ b/spec/system/popup_notifications_click_through_spec.rb @@ -32,6 +32,12 @@ let(:user_field) { DiscoursePopupNotifications::USER_ENABLED_FIELD } let(:id_seq) { [800_000] } + # Gallery/behavioral spec: runs in the Feature Screenshots workflow (which + # sets this env). Skipped in the main parallel system_tests run so its 30 + # browser navigations do not weigh that job down — core click-through is + # covered by popup_notifications_spec.rb. + before { skip("screenshot-gallery only") unless ENV["JTECH_SCREENSHOT_GALLERY"] } + before do SiteSetting.popup_notifications_enabled = true SiteSetting.popup_notifications_timeout_seconds = 300 @@ -95,7 +101,12 @@ def visit_home %i[replied mentioned quoted liked private_message].each do |type| it "clicking a #{type} toast opens its topic and dismisses (batch #{batch})" do target = - Fabricate(:topic, category: category, user: recipient, title: "Target #{batch} #{type}") + Fabricate( + :topic, + category: category, + user: recipient, + title: "Another topic #{batch} for a #{type} notification", + ) Fabricate( :post, topic: target, diff --git a/spec/system/popup_notifications_screenshots_spec.rb b/spec/system/popup_notifications_screenshots_spec.rb index a278684..9b28e23 100644 --- a/spec/system/popup_notifications_screenshots_spec.rb +++ b/spec/system/popup_notifications_screenshots_spec.rb @@ -74,6 +74,12 @@ # notification gets a fresh id. let(:id_seq) { [500_000] } + # Gallery spec: generates screenshots in the Feature Screenshots workflow + # (which sets this env). Skipped in the main parallel system_tests run so it + # does not weigh that job down — core behavior is covered by + # popup_notifications_spec.rb. + before { skip("screenshot-gallery only") unless ENV["JTECH_SCREENSHOT_GALLERY"] } + before do SiteSetting.popup_notifications_enabled = true SiteSetting.popup_notifications_timeout_seconds = 300 # keep the card up long enough to shoot diff --git a/spec/system/popup_notifications_stacking_screenshots_spec.rb b/spec/system/popup_notifications_stacking_screenshots_spec.rb index e9ccb59..9ebbe2e 100644 --- a/spec/system/popup_notifications_stacking_screenshots_spec.rb +++ b/spec/system/popup_notifications_stacking_screenshots_spec.rb @@ -69,6 +69,11 @@ let(:user_field) { DiscoursePopupNotifications::USER_ENABLED_FIELD } let(:id_seq) { [700_000] } + # Gallery spec: generates screenshots in the Feature Screenshots workflow + # (which sets this env). Skipped in the main parallel system_tests run so it + # does not weigh that job down. + before { skip("screenshot-gallery only") unless ENV["JTECH_SCREENSHOT_GALLERY"] } + before do SiteSetting.popup_notifications_enabled = true SiteSetting.popup_notifications_timeout_seconds = 300 From b57d56ae057ad037a6af566d113ed92a6c520958 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 1 Jul 2026 15:36:46 -0400 Subject: [PATCH 11/11] Pop-up stacking spec: one example per shot The stacking screenshots run only in the single-process Feature Screenshots workflow (no per-example retry), where one flaky capture aborted the rest of a grouped example and dropped later shots. Split the 25 stacks into 25 independent examples (fresh session each) so a flake isolates to one shot and the gallery captures the rest. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Js4yBxioTgsB8TkTd6HE9j --- ...notifications_stacking_screenshots_spec.rb | 79 +++++++++++++++++-- 1 file changed, 71 insertions(+), 8 deletions(-) diff --git a/spec/system/popup_notifications_stacking_screenshots_spec.rb b/spec/system/popup_notifications_stacking_screenshots_spec.rb index 9ebbe2e..665ac52 100644 --- a/spec/system/popup_notifications_stacking_screenshots_spec.rb +++ b/spec/system/popup_notifications_stacking_screenshots_spec.rb @@ -8,10 +8,10 @@ # can take its place. 25 shots across single/double/triple stacks, type mixes, # content shapes, and the overflow-replaces-oldest behavior. # -# Reliability: after a fresh page load the browser's MessageBus poll can take -# a moment to subscribe, so each example first "primes" the channel (publish + -# wait + dismiss) and then builds the stack with one publish per card, waiting -# for the exact card count each step. +# Each shot is its own example (fresh browser session) so a single flaky +# capture never aborts the others. Within an example the channel is "primed" +# (publish + wait + dismiss) before the stack is built one card at a time, +# waiting for the exact card count each step. # # Screenshots land in tmp/capybara/ and are published as the CI artifact. RSpec.describe "Desktop pop-up notification stacking screenshots" do @@ -201,60 +201,105 @@ def overflow_shot(name, specs, top_icon:) shot(name) end - it "single and double stacks (01–09)" do + it "single reply (01)" do stack_shot("stack_01_single_reply", enriched(:replied)) + end + + it "reply + like (02)" do stack_shot("stack_02_reply_like", enriched(:replied), enriched(:liked)) + end + + it "reply + mention (03)" do stack_shot("stack_03_reply_mention", enriched(:replied), enriched(:mentioned)) + end + + it "reply + quote (04)" do stack_shot("stack_04_reply_quote", enriched(:replied), enriched(:quoted)) + end + + it "pm + reply (05)" do stack_shot("stack_05_pm_reply", enriched(:private_message), enriched(:replied)) + end + + it "whisper + reply (06)" do stack_shot("stack_06_whisper_reply", whisper, enriched(:replied)) + end + + it "flag + pending (07)" do stack_shot( "stack_07_flag_pending", mod_note("flag_note", excerpt: "Flagged as spam — please review."), mod_note("post_approved", title: topic.title, excerpt: "Approved a queued reply."), ) + end + + it "badge + reply (08)" do stack_shot("stack_08_badge_reply", enriched(:granted_badge), enriched(:replied)) + end + + it "edited + linked (09)" do stack_shot("stack_09_edited_linked", enriched(:edited), enriched(:linked)) end - it "triple stacks by type mix (10–17)" do + it "reply + like + mention (10)" do stack_shot( "stack_10_reply_like_mention", enriched(:replied), enriched(:liked), enriched(:mentioned), ) + end + + it "reply + quote + pm (11)" do stack_shot( "stack_11_reply_quote_pm", enriched(:replied), enriched(:quoted), enriched(:private_message), ) + end + + it "whisper + flag + pending (12)" do stack_shot( "stack_12_whisper_flag_pending", whisper, mod_note("flag_note", excerpt: "Flagged as off-topic."), mod_note("post_approved", title: topic.title, excerpt: "Approved a queued reply."), ) + end + + it "three likes (13)" do stack_shot( "stack_13_like_x3", enriched(:liked), enriched(:liked, post: reply_post2), enriched(:liked), ) + end + + it "mention + quote + reply (14)" do stack_shot( "stack_14_mention_quote_reply", enriched(:mentioned), enriched(:quoted), enriched(:replied), ) + end + + it "pm + whisper + reply (15)" do stack_shot("stack_15_pm_whisper_reply", enriched(:private_message), whisper, enriched(:replied)) + end + + it "reply + flag + like (16)" do stack_shot( "stack_16_reply_flag_like", enriched(:replied), mod_note("flag_note", excerpt: "A new flag needs attention."), enriched(:liked), ) + end + + it "fallback + reply + whisper (17)" do stack_shot( "stack_17_fallback_reply_whisper", fallback( @@ -266,37 +311,52 @@ def overflow_shot(name, specs, top_icon:) ) end - it "content shapes and more mixes in a stack (18–23)" do + it "long title + reply + like (18)" do stack_shot( "stack_18_longtitle_reply_like", enriched(:replied, post: long_topic_reply, into: long_topic), enriched(:replied), enriched(:liked), ) + end + + it "long message + like + mention (19)" do stack_shot( "stack_19_longmessage_like_mention", enriched(:replied, post: long_reply), enriched(:liked), enriched(:mentioned), ) + end + + it "badge + pm + reply (20)" do stack_shot( "stack_20_badge_pm_reply", enriched(:granted_badge), enriched(:private_message), enriched(:replied), ) + end + + it "edited + linked + quote (21)" do stack_shot( "stack_21_edited_linked_quote", enriched(:edited), enriched(:linked), enriched(:quoted), ) + end + + it "flag + pending + whisper (22)" do stack_shot( "stack_22_flag_pending_whisper", mod_note("flag_note", excerpt: "Flag raised on a reply."), mod_note("post_rejected", title: topic.title, excerpt: "Rejected a queued reply."), whisper, ) + end + + it "three replies, mixed topics (23)" do stack_shot( "stack_23_three_replies_mixed", enriched(:replied), @@ -305,12 +365,15 @@ def overflow_shot(name, specs, top_icon:) ) end - it "overflow — a 4th notification replaces the oldest (24–25)" do + it "overflow — a 4th engagement notification replaces the oldest (24)" do overflow_shot( "stack_24_overflow_engagement", [enriched(:replied), enriched(:liked), enriched(:mentioned), whisper], top_icon: ".d-icon-eye", ) + end + + it "overflow — a 4th staff notification replaces the oldest (25)" do overflow_shot( "stack_25_overflow_staff", [