mirror of
https://github.com/we-promise/sure.git
synced 2026-09-05 06:41:08 +00:00
fix(insights): drop the dismiss toast and fix both empty states (#2965)
* 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.
This commit is contained in:
Guillem Arias Fauste
@@ -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|
|
||||
|
||||
@@ -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). %>
|
||||
<div id="<%= dom_id(insight, :undo) %>"
|
||||
role="status"
|
||||
aria-live="polite"
|
||||
class="relative flex items-center gap-3 rounded-lg bg-container p-4 group w-full md:max-w-80 shadow-border-xs"
|
||||
data-controller="element-removal">
|
||||
<p class="text-primary text-sm font-medium grow"><%= t("insights.card.acknowledged") %></p>
|
||||
|
||||
<%# autofocus so Undo is one keystroke away: the acknowledged card is gone from
|
||||
the DOM, which drops focus to <body>, 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" }
|
||||
) %>
|
||||
</div>
|
||||
@@ -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 %>
|
||||
|
||||
@@ -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 %>
|
||||
|
||||
@@ -6,4 +6,12 @@
|
||||
|
||||
<div id="sync-toast"></div>
|
||||
<div id="cta"></div>
|
||||
|
||||
<%# 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. %>
|
||||
<div id="aria-announcer" class="sr-only" role="status" aria-live="polite" aria-atomic="true"></div>
|
||||
</div>
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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!
|
||||
|
||||
|
||||
Reference in New Issue
Block a user