From a461ff97bbcd5cb72358a6a58b21604efbd63998 Mon Sep 17 00:00:00 2001 From: Guillem Arias Fauste Date: Sat, 6 Jun 2026 17:02:26 +0200 Subject: [PATCH] fix(sync-toast): morph refresh, defer behind modals, DS conformance (#2105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(sync-toast): morph refresh, defer behind modals, DS conformance Follow-up to #1964 (addresses #2071). - Refresh via Turbo morph visit instead of window.location.reload, so scroll position and data-turbo-permanent elements (the AI chat panel) survive and there is no white flash. - Defer the toast while a is open and reveal it on close. A refresh mid-modal closes the dialog and discards its in-progress input, which is the exact data loss this toast exists to prevent. Handles stacked modals. - Refresh CTA and close button now use DS::Button (secondary / icon). The close is always visible, inside the card, focusable, and has an aria-label; the old hover-only corner chip was unreachable on touch and not keyboard-focusable. - Add role="status" / aria-live="polite" to the toast. - Fix icon color: "inverse" is not a key in the icon helper color map, so it silently rendered no color class (dark icon on bg-info). Use "white", which maps to the functional text-inverse token. - Tighten copy: "New data available" / "Refresh". - Sync the broadcast comment with the actual replace/morph behavior. * fix(sync-toast): detach deferred dialog listener on disconnect A toast replaced by a newer broadcast_replace_to while a was open kept its 'close' listener attached, so the detached controller fired #reveal()/#arm() when the dialog closed — a spurious auto-refresh from a stale toast (and repeated syncs could queue several). Store the dialog + handler refs and remove the listener in disconnect(). Flagged by codex + coderabbit on #2105. * fix(sync-toast): re-check interaction and dialogs at refresh-fire time The interaction check ran once at arm time but the refresh fired two seconds later. The post-dialog reveal made that window matter: the user closes a dialog sitting on a form, resumes typing, and the timer morphs the page — wiping non-turbo-permanent input, the exact data-loss class this toast exists to prevent. A dialog opened during the window had the mirror problem (the refresh would close it). Bail inside the callback instead, leaving the toast visible for a manual refresh, matching the mid-form behavior. Also documents the dialog-removed-without-close edge on the deferred listener. --- .../controllers/sync_toast_controller.js | 98 +++++++++++++++++-- app/models/family/sync_complete_event.rb | 9 +- .../shared/notifications/_sync_toast.html.erb | 33 ++++--- config/locales/views/shared/en.yml | 4 +- 4 files changed, 119 insertions(+), 25 deletions(-) diff --git a/app/javascript/controllers/sync_toast_controller.js b/app/javascript/controllers/sync_toast_controller.js index 567f54635..20cfc4c9e 100644 --- a/app/javascript/controllers/sync_toast_controller.js +++ b/app/javascript/controllers/sync_toast_controller.js @@ -2,32 +2,114 @@ import { Controller } from "@hotwired/stimulus"; // Connects to data-controller="sync-toast" // -// Shown when a background sync completes and the family's data changes. -// - If the user is not interacting with a form, auto-reloads after a short delay. -// - If the user is mid-form, the toast stays visible so they can choose when to refresh. +// Shown when a background sync completes and the family's data has changed. +// - Idle → morph-refreshes the page after a short delay. +// - Mid-form → stays put; the user refreshes when ready. +// - A modal is open → the toast is deferred (it would otherwise sit +// dimmed-but-clickable behind the dialog's top-layer backdrop, and a refresh +// would close the dialog and discard its in-progress input). It is revealed +// once the dialog closes — the first moment a refresh is actually safe. export default class extends Controller { static values = { autoRefreshDelay: { type: Number, default: 2000 }, }; connect() { - if (!this.#userIsInteracting()) { - this._timer = setTimeout(() => this.refresh(), this.autoRefreshDelayValue); + if (this.#dialogOpen()) { + this.#deferUntilDialogCloses(); + return; } + this.#arm(); } disconnect() { clearTimeout(this._timer); + this.#removeDeferredDialogListener(); } + // Turbo 8 morph refresh (the app sets `turbo_refreshes_with method: :morph, + // scroll: :preserve`) instead of window.location.reload(): no white flash, + // scroll position and `data-turbo-permanent` elements (the AI chat panel) + // are preserved. refresh() { clearTimeout(this._timer); - window.location.reload(); + Turbo.visit(window.location.href, { action: "replace" }); + } + + #arm() { + if (this.#userIsInteracting()) return; // mid-form: wait for a manual refresh + // Re-check at fire time, not just arm time: the post-dialog reveal often + // lands on a form the dialog was sitting on, and the user resumes typing + // inside this window (a morph would wipe their non-turbo-permanent + // input). A dialog opened during the window is the same hazard — the + // refresh would close it. Either way, bail and leave the toast visible + // for a manual refresh, matching the mid-form behavior. + this._timer = setTimeout(() => { + if (this.#userIsInteracting() || this.#dialogOpen()) return; + this.refresh(); + }, this.autoRefreshDelayValue); + } + + #deferUntilDialogCloses() { + this.element.style.display = "none"; + const dialog = document.querySelector("dialog[open]"); + if (!dialog) { + this.#reveal(); + return; + } + // Keep refs so disconnect() can detach this listener. Otherwise a toast + // replaced by a newer broadcast while the dialog is still open stays + // subscribed, and its now-detached controller fires #reveal()/#arm() on + // close — a spurious auto-refresh from a stale toast. + // + // Known edge: if this dialog leaves the DOM without firing `close` (e.g. + // a morph removes it), the toast stays hidden until the next broadcast + // replaces it. Acceptable: the next sync re-delivers the toast. + this._deferredDialog = dialog; + this._dialogCloseHandler = () => this.#onDialogClose(); + dialog.addEventListener("close", this._dialogCloseHandler, { once: true }); + } + + #onDialogClose() { + // The `once` listener has already fired and detached itself. + this._deferredDialog = null; + this._dialogCloseHandler = null; + // Another dialog may still be open (stacked modals) — keep deferring until + // every dialog has closed. + if (this.#dialogOpen()) { + this.#deferUntilDialogCloses(); + return; + } + this.#reveal(); + } + + #removeDeferredDialogListener() { + if (this._deferredDialog && this._dialogCloseHandler) { + this._deferredDialog.removeEventListener( + "close", + this._dialogCloseHandler, + ); + } + this._deferredDialog = null; + this._dialogCloseHandler = null; + } + + #reveal() { + this.element.style.display = ""; + this.#arm(); + } + + #dialogOpen() { + return !!document.querySelector("dialog[open]"); } #userIsInteracting() { const el = document.activeElement; - if (!el || el === document.body || el === document.documentElement) return false; - return el.isContentEditable || el.closest("form, dialog, [role='dialog']") !== null; + if (!el || el === document.body || el === document.documentElement) + return false; + return ( + el.isContentEditable || + el.closest("form, dialog, [role='dialog']") !== null + ); } } diff --git a/app/models/family/sync_complete_event.rb b/app/models/family/sync_complete_event.rb index 859e380fd..3949990d3 100644 --- a/app/models/family/sync_complete_event.rb +++ b/app/models/family/sync_complete_event.rb @@ -6,10 +6,11 @@ class Family::SyncCompleteEvent end def broadcast - # Append a lightweight toast to the notification tray instead of a full - # page refresh. The sync-toast Stimulus controller handles two cases: - # - User is idle → auto-reloads after a short delay - # - User is mid-form → toast stays visible; user clicks "Refresh now" + # Replace the #sync-toast slot with a lightweight toast instead of a full + # page refresh. The sync-toast Stimulus controller handles three cases: + # - User is idle → morph-refreshes after a short delay + # - User is mid-form → toast stays visible; user clicks "Refresh" + # - A modal is open → toast defers until the dialog closes # # This avoids wiping in-progress form state when a background sync fires. # The partial contains no user-scoped data (Current.user is nil here), so diff --git a/app/views/shared/notifications/_sync_toast.html.erb b/app/views/shared/notifications/_sync_toast.html.erb index 763a6557f..4bd4efbe7 100644 --- a/app/views/shared/notifications/_sync_toast.html.erb +++ b/app/views/shared/notifications/_sync_toast.html.erb @@ -1,23 +1,34 @@
+ class="relative flex gap-3 rounded-lg bg-container p-4 w-full md:max-w-80 shadow-border-xs">
- <%= icon "refresh-cw", size: "xs", color: "inverse" %> + <%= icon "refresh-cw", size: "xs", color: "white" %>
-
+

<%= t("shared.sync_toast.message") %>

- + <%= render DS::Button.new( + text: t("shared.sync_toast.refresh"), + variant: "secondary", + size: "sm", + icon: "refresh-cw", + type: "button", + data: { action: "sync-toast#refresh" } + ) %>
-
- +
+ <%= 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/config/locales/views/shared/en.yml b/config/locales/views/shared/en.yml index 50599089c..3ec0b37ea 100644 --- a/config/locales/views/shared/en.yml +++ b/config/locales/views/shared/en.yml @@ -6,8 +6,8 @@ en: shared: preview: Preview sync_toast: - message: "Your data has been updated" - refresh: "Refresh now" + message: "New data available" + refresh: "Refresh" confirm_modal: accept: Confirm body_html: "

You will not be able to undo this decision

"