From 2bcef99441f92d4f4a27918d8db7aa35cab2f5ee Mon Sep 17 00:00:00 2001 From: Guillem Arias Fauste Date: Tue, 11 Aug 2026 06:15:02 +0200 Subject: [PATCH] fix(insights): drop the dismiss toast and fix both empty states (#2965) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(insights): drop the dismiss toast and fix both empty states Three problems with acknowledging an insight, all in the turbo-stream path. Every dismissal appended an undo toast to the notification tray. Acknowledging already means "hidden until these numbers change" — GenerateInsightsJob resurfaces a row whose metadata moves materially, and 6 of 8 generators scope `dedup_key` to a month — so a toast interrupting the flow bought little. Removed, along with the now-orphaned `_undo_toast` partial and its two locale keys. Dismissing the last insight left the dashboard widget on screen: the stream re-rendered the well unconditionally, so the section shell stayed with its header above an empty box until a reload. A full render already drops it (PagesController#insights_feed_section sets `visible: @feed_insights.any?`); the stream now removes the whole section to match, targeted by `[data-section-key='insights_feed']` because the shared dashboard loop emits no id on the section element. Dismissing the last insight on /insights left a blank page. The card left via `turbo_stream.remove`, which emptied #insights-list without re-rendering the partial that owns the empty state, so "No insights yet" only appeared after a reload. The list is now replaced rather than the card removed — the same thing unacknowledge already did. InsightsController#unacknowledge, its route and Insight#unacknowledge! are kept and still work; only the toast that reached them is gone, so undo can be re-wired to a different surface without resurrecting them. * fix(insights): announce the dismissal now the toast is gone Removing the undo toast took the only `role=status` element with it, and the stream also replaces the list containing the "Got it" control the user just activated — so a screen-reader or keyboard user was left with no confirmation that anything happened. Add a shared, visually hidden live region to the notification tray and update it from the acknowledge stream. It sits outside every stream target and is rendered with the page, which matters: a live region that arrives together with its own content is not announced. Updated rather than appended, so messages replace instead of piling up, and it stays empty (and free) until something uses it. This is a general primitive, not an insights one — the tray already holds `#sync-toast` and `#cta` as stable stream targets, and any flow that changes the page without leaving something on screen to read can use it. Verified in a browser: after dismissing, the region reads "Insight dismissed" at 1x1px with `clip: rect(0,0,0,0)` — announced, invisible. --- app/controllers/insights_controller.rb | 5 ++ app/views/insights/_undo_toast.html.erb | 35 ------------ .../insights/acknowledge.turbo_stream.erb | 40 ++++++++++---- .../insights/unacknowledge.turbo_stream.erb | 9 ++-- .../shared/_notification_tray.html.erb | 8 +++ config/locales/views/insights/en.yml | 3 +- config/locales/views/insights/fr.yml | 2 +- config/locales/views/insights/pl.yml | 2 +- config/locales/views/insights/tr.yml | 2 +- test/controllers/insights_controller_test.rb | 54 ++++++++++++++++++- 10 files changed, 103 insertions(+), 57 deletions(-) delete mode 100644 app/views/insights/_undo_toast.html.erb diff --git a/app/controllers/insights_controller.rb b/app/controllers/insights_controller.rb index 6b07137bd..0fec7a722 100644 --- a/app/controllers/insights_controller.rb +++ b/app/controllers/insights_controller.rb @@ -17,6 +17,11 @@ class InsightsController < ApplicationController def acknowledge @insight.acknowledge! + # Both surfaces: the response carries streams for the /insights list and the + # dashboard widget, and each page applies only the ones whose targets it has. + # The list (not just the acknowledged card) is reloaded so its empty state + # can take over when the last insight goes. + load_feed load_widget_feed respond_to do |format| diff --git a/app/views/insights/_undo_toast.html.erb b/app/views/insights/_undo_toast.html.erb deleted file mode 100644 index 273e54fbd..000000000 --- a/app/views/insights/_undo_toast.html.erb +++ /dev/null @@ -1,35 +0,0 @@ -<%# locals: (insight:) %> - -<%# The card leaves the page via a Turbo `remove`, which is silent to a screen - reader — this toast is the only announcement of what happened, so it carries - the live region (same contract as shared/notifications/_sync_toast). %> -
-

<%= t("insights.card.acknowledged") %>

- - <%# autofocus so Undo is one keystroke away: the acknowledged card is gone from - the DOM, which drops focus to , and Turbo focuses the first - [autofocus] element in stream-rendered content. %> - <%= render DS::Button.new( - text: t("insights.card.undo"), - variant: "ghost", - size: "sm", - href: unacknowledge_insight_path(insight), - method: :patch, - autofocus: true - ) %> - - <%# A real button, not a bare icon with a click action: the glyph alone is - neither focusable nor named, so the toast could only be closed by mouse. %> - <%= render DS::Button.new( - variant: "icon", - size: "sm", - icon: "x", - type: "button", - "aria-label": t("defaults.common.close"), - data: { action: "click->element-removal#remove" } - ) %> -
diff --git a/app/views/insights/acknowledge.turbo_stream.erb b/app/views/insights/acknowledge.turbo_stream.erb index f0a9cfa84..2de2142f6 100644 --- a/app/views/insights/acknowledge.turbo_stream.erb +++ b/app/views/insights/acknowledge.turbo_stream.erb @@ -1,16 +1,34 @@ -<%# Acknowledging is reachable from two surfaces now, so this response carries - the update for both and each applies only the streams whose targets it has — +<%# Acknowledging is reachable from two surfaces, so this response carries the + update for both and each applies only the streams whose targets it has — Turbo silently ignores a stream whose target is absent. %> -<%# /insights: drop the card. %> -<%= turbo_stream.remove dom_id(@insight) %> - -<%# Dashboard: re-render the well rather than removing a row, so the next - insight is promoted into the freed slot instead of leaving a gap. %> -<%= turbo_stream.replace "insights-feed" do %> - <%= render "pages/dashboard/insights_feed", insights: @feed_insights %> +<%# The card leaves via a stream, which is silent to a screen reader, and the + "Got it" control the user just activated goes with it. Dropping the undo + toast took away the only announcement that anything happened, so say it in + the shared live region instead — same confirmation, no toast. %> +<%= turbo_stream.update "aria-announcer" do %> + <%= t("insights.card.acknowledged") %> <% end %> -<%= turbo_stream.append "notification-tray" do %> - <%= render "insights/undo_toast", insight: @insight %> +<%# /insights: re-render the whole list rather than removing the one card. + Removing left an empty #insights-list behind, so dismissing the last insight + showed a blank page until reload — the empty state lives in this partial and + only renders when the partial does. %> +<%= turbo_stream.replace "insights-list" do %> + <%= render "insights/list", insights: @insights, unread_ids: @unread_ids %> +<% end %> + +<% if @feed_insights.any? %> + <%# Dashboard: re-render the well rather than removing a row, so the next + insight is promoted into the freed slot instead of leaving a gap. %> + <%= turbo_stream.replace "insights-feed" do %> + <%= render "pages/dashboard/insights_feed", insights: @feed_insights %> + <% end %> +<% else %> + <%# Nothing left to show: drop the whole dashboard section, not just the well + inside it, or the section shell lingers with its header and an empty box. + Matches what a reload does — PagesController#insights_feed_section sets + `visible: @feed_insights.any?`. Targeted by attribute because the section + element is emitted by the shared dashboard loop and carries no id. %> + <%= turbo_stream.remove_all "[data-section-key='insights_feed']" %> <% end %> diff --git a/app/views/insights/unacknowledge.turbo_stream.erb b/app/views/insights/unacknowledge.turbo_stream.erb index 0428fc2e2..d3d4914e9 100644 --- a/app/views/insights/unacknowledge.turbo_stream.erb +++ b/app/views/insights/unacknowledge.turbo_stream.erb @@ -1,11 +1,12 @@ -<%= turbo_stream.remove dom_id(@insight, :undo) %> - <%= turbo_stream.replace "insights-list" do %> <%= render "insights/list", insights: @insights, unread_ids: @unread_ids %> <% end %> -<%# Undo can be pressed from the dashboard, where the toast is the only part of - this flow on screen — so the well needs restoring there too. %> +<%# Dashboard well, when it's still on screen. If acknowledging the last insight + removed the whole section, there is no target left to replace and the widget + comes back on the next dashboard render — this action has no UI entry point + since the undo toast was dropped, so it isn't worth rebuilding the section + shell from here. %> <%= turbo_stream.replace "insights-feed" do %> <%= render "pages/dashboard/insights_feed", insights: @feed_insights %> <% end %> diff --git a/app/views/layouts/shared/_notification_tray.html.erb b/app/views/layouts/shared/_notification_tray.html.erb index 7f999bf72..6ea5c3cc8 100644 --- a/app/views/layouts/shared/_notification_tray.html.erb +++ b/app/views/layouts/shared/_notification_tray.html.erb @@ -6,4 +6,12 @@
+ + <%# Announces an action that changed the page without leaving anything on + screen to read — a turbo_stream can update this instead of rendering a + toast. It has to live here, outside every stream target, and be present + before the change: a live region that arrives with its own content is not + announced. Update it (don't append) so messages replace rather than pile + up. Empty and visually hidden, so it costs nothing when unused. %> +
diff --git a/config/locales/views/insights/en.yml b/config/locales/views/insights/en.yml index 2ed0fb64f..cc557e3c6 100644 --- a/config/locales/views/insights/en.yml +++ b/config/locales/views/insights/en.yml @@ -12,11 +12,10 @@ en: queued: We're generating fresh insights. Check back in a minute. checking: Checking… card: + acknowledged: "Insight dismissed" acknowledge: Got it acknowledge_title: Hide this until the numbers change new: New - acknowledged: Got it — hidden until this changes - undo: Undo feed: view_all: View all insights header: Latest diff --git a/config/locales/views/insights/fr.yml b/config/locales/views/insights/fr.yml index 1c72b712c..c0c8be64d 100644 --- a/config/locales/views/insights/fr.yml +++ b/config/locales/views/insights/fr.yml @@ -10,10 +10,10 @@ fr: spending_anomaly: Voir les transactions de %{category} subscription_audit: Vérifier les transactions récurrentes card: + acknowledged: "Analyse ignorée" dismiss: Ignorer dismissed: Analyse ignorée new: Nouveau - undo: Annuler feed: header: Dernières header_new: Nouveau diff --git a/config/locales/views/insights/pl.yml b/config/locales/views/insights/pl.yml index 264a18928..98f407c5d 100644 --- a/config/locales/views/insights/pl.yml +++ b/config/locales/views/insights/pl.yml @@ -12,10 +12,10 @@ pl: queued: Generujemy świeże analizy. Sprawdź za minutę. checking: Sprawdzanie... card: + acknowledged: "Analiza odrzucona" dismiss: Odrzuć new: Nowa dismissed: Analiza odrzucona - undo: Cofnij feed: view_all: Zobacz wszystkie analizy header: Najnowsze diff --git a/config/locales/views/insights/tr.yml b/config/locales/views/insights/tr.yml index 4992dad6b..eba60b8c7 100644 --- a/config/locales/views/insights/tr.yml +++ b/config/locales/views/insights/tr.yml @@ -10,10 +10,10 @@ tr: spending_anomaly: "%{category} işlemlerini görüntüle" subscription_audit: Yinelenen işlemleri incele card: + acknowledged: "İçgörü yoksayıldı" dismiss: Yoksay dismissed: İçgörü yoksayıldı new: Yeni - undo: Geri al feed: header: Son header_new: Yeni diff --git a/test/controllers/insights_controller_test.rb b/test/controllers/insights_controller_test.rb index 01b252c1f..40bc6c82b 100644 --- a/test/controllers/insights_controller_test.rb +++ b/test/controllers/insights_controller_test.rb @@ -49,15 +49,65 @@ class InsightsControllerTest < ActionDispatch::IntegrationTest "insights_feed should be prepended, not appended, for saved orders that predate it" end - test "acknowledge removes the insight from the feed and offers undo via turbo stream" do + # Acknowledging is a quiet action — no undo toast. Acknowledgement only covers + # the numbers the user saw (see Insight's class comment), so a dismissed + # insight resurfaces on its own when those numbers move; a toast interrupting + # every dismissal bought little. + test "acknowledge clears the insight without an undo toast" do patch acknowledge_insight_url(@insight), as: :turbo_stream assert_response :success assert_match "turbo-stream", response.body - assert_match unacknowledge_insight_path(@insight), response.body + assert_no_match(/#{Regexp.escape(unacknowledge_insight_path(@insight))}/, response.body) assert @insight.reload.acknowledged? end + # The card leaves via a stream, which is silent to a screen reader, and takes + # the control the user just activated with it. With the toast gone, the shared + # live region carries that confirmation instead. + test "acknowledge announces the dismissal in the shared live region" do + patch acknowledge_insight_url(@insight), as: :turbo_stream + + assert_response :success + assert_select "turbo-stream[action=update][target=?]", "aria-announcer" do + assert_match CGI.escapeHTML(I18n.t("insights.card.acknowledged")), response.body + end + end + + test "the live region is present before any stream updates it" do + get insights_url + + assert_response :success + assert_select "#aria-announcer[role=status][aria-live=polite]", 1, + "a live region that arrives with its content is not announced" + end + + # The card used to leave via `turbo_stream.remove`, which emptied + # #insights-list without re-rendering the partial that owns the empty state — + # so dismissing the last insight showed a blank page until reload. + test "acknowledging the last insight renders the empty state" do + @user.family.insights.where.not(id: @insight.id).destroy_all + + patch acknowledge_insight_url(@insight), as: :turbo_stream + + assert_response :success + assert_match "insights-list", response.body + assert_match CGI.escapeHTML(I18n.t("insights.index.empty.title")), response.body + end + + # A full dashboard render already drops the section (insights_feed_section sets + # `visible: @feed_insights.any?`), so the turbo path has to as well — otherwise + # the section shell lingered with its header above an empty well. + test "acknowledging the last insight removes the dashboard section" do + @user.family.insights.where.not(id: @insight.id).destroy_all + + patch acknowledge_insight_url(@insight), as: :turbo_stream + + assert_response :success + assert_select "turbo-stream[action=remove][targets=?]", "[data-section-key='insights_feed']" + assert_select "turbo-stream[action=replace][target=?]", "insights-feed", count: 0 + end + test "unacknowledge restores the insight as read and re-renders the list" do @insight.acknowledge!