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!