From 213b940a1ba484ecd36d1719ae4ef8cdc32deee6 Mon Sep 17 00:00:00 2001 From: Roland Date: Sat, 1 Aug 2026 14:50:53 -0600 Subject: [PATCH] ui: toasts move to the top right, get readable titles, and get tests The title was unreadable in dark, night and amber. Mantine paints a dark-scheme notification title with `--mantine-color-white`, and the bridge aliases that to `--btn-text` -- deliberately, because it is the text drawn on a filled button, near-black in those three themes. So the title came out rgb(28, 31, 33) on a rgb(38, 41, 43) surface: about 1.1:1, present and invisible. Fixing the alias is not an option, it is correct for the buttons it exists for, so notifications now take their surface, border and text from our tokens instead. Measured after: 14.9, 11.9, 5.0 and 8.6 to one. The description follows the same route, and night and amber drop its muting. `--muted` on a panel is 2.0:1 in night and 3.1:1 in amber -- the description is the message itself, so those two let the title's weight carry the hierarchy instead. (`--muted` being that dim in night is not only a toast problem; statistic labels and inactive tabs use it too. Left alone here.) Position moves from bottom right to top right. Bottom right put toasts over the file browser's sticky footer and a video's player controls, which are the two places a background task is most likely to report while the user is doing something else. Gallery gets a Toasts section: one button per type, one that stacks a numbered toast per press, one that fills the container to its limit of five, one that never expires, one with no title, one with text long enough to wrap, and clear-all. A toast is the one component that cannot be seen by loading a page, so it needs buttons rather than a sample. Tests ----- Thirteen, where there were none for thirteen call sites -- and `toast` has already failed once app-wide, when App.js dropped Semantic's container while seven pages still imported the old helper and every notification silently went nowhere. Six were verified red against planted defects (a shared `id`, which makes Mantine replace instead of stack; one colour for every type; `autoClose` taking the 0 literally). The duration tests use real timers with short durations, which matters. With fake timers the element is never removed from the DOM -- hiding a notification runs an exit transition, and advancing the clock, even awaited inside `act`, does not complete it. Both the assertion and its control then read the same whatever the code does. The test for `time: 0` passed against a planted `autoClose: 0` twice before this was rewritten: once asserting synchronously, once with a control that could not fail either. It now waits for a normally-expiring toast to disappear first, so dismissal is demonstrably working before the survivor is asserted. --- app/src/components/Theme.tsx | 7 +- app/src/components/ThemeSamplePage.js | 76 ++++++++++ app/src/components/ui/toast.test.js | 204 ++++++++++++++++++++++++++ app/src/components/ui/ui.css | 48 ++++++ 4 files changed, 334 insertions(+), 1 deletion(-) create mode 100644 app/src/components/ui/toast.test.js diff --git a/app/src/components/Theme.tsx b/app/src/components/Theme.tsx index 3c5d6625..7e9b0aa8 100644 --- a/app/src/components/Theme.tsx +++ b/app/src/components/Theme.tsx @@ -215,7 +215,12 @@ export function ThemeProvider({children, ...props}: ThemeProviderProps) { cssVariablesResolver={cssVariablesResolver} forceColorScheme={dark ? 'dark' : 'light'} > - + {/* + * Top right. Bottom right put toasts over the sticky footer on the file browser + * and over the player controls on a video, which are the two places a background + * task is most likely to report while the user is doing something else. + */} + {children} diff --git a/app/src/components/ThemeSamplePage.js b/app/src/components/ThemeSamplePage.js index 730186e4..7d2dabc6 100644 --- a/app/src/components/ThemeSamplePage.js +++ b/app/src/components/ThemeSamplePage.js @@ -33,6 +33,7 @@ import { ThemePicker, Toggle, Tooltip, + clearToasts, toast, } from './ui'; import {semanticColorNames} from '../themes/mantine'; @@ -61,6 +62,10 @@ const Row = ({children}) =>
; +// Numbers the stacked toasts so it is obvious which press produced which, and that a second +// press adds a toast rather than replacing the first. Module scope, so it survives re-renders. +let toastCounter = 0; + export function ThemeSamplePage() { const {theme} = useContext(ThemeContext); const [confirmOpen, setConfirmOpen] = useState(false); @@ -212,6 +217,77 @@ export function ThemeSamplePage() { +
+ + {/* + * `toast` has thirteen call sites and no test, and it has already failed once + * app-wide: App.js dropped Semantic's toast container while seven pages were + * still importing the old helper, and every notification silently vanished. + * A toast is also the one component you cannot see by loading a page -- it has + * to be provoked -- which is why it needs buttons here rather than a sample. + */} + + + + + + + + + {/* Click this repeatedly: each press is a separate toast, and the + container holds five at once. */} + + + + + + + + +

+ Bottom right, five at a time, five seconds each unless the caller says + otherwise. Check each type in all four themes: night has no second hue to + spend, so info, success, warning and error cannot be told apart by colour + there. +

+ +
+
diff --git a/app/src/components/ui/toast.test.js b/app/src/components/ui/toast.test.js new file mode 100644 index 00000000..6e94768a --- /dev/null +++ b/app/src/components/ui/toast.test.js @@ -0,0 +1,204 @@ +import React from 'react'; +import {act, render, screen, waitFor} from '@testing-library/react'; +import {MantineProvider} from '@mantine/core'; +import {Notifications} from '@mantine/notifications'; +import {cssVariablesResolver, mantineTheme} from '../../themes/mantine'; +import {clearToasts, toast} from './toast'; + +/* + * Tests for toasts. + * + * `toast` has thirteen call sites and had no test at all, having already failed once in the + * way that is hardest to notice: App.js dropped Semantic's toast container while seven pages + * were still importing the old helper, so every notification in the app silently went + * nowhere. Nothing threw, and nothing looked wrong until somebody expected a message. + * + * The container is mounted here the way ThemeProvider mounts it, because a toast with no + * container is exactly the failure worth covering. + */ + +const withToasts = () => render( + + + +); + +const showToast = (options) => act(() => { + toast(options); +}); + +const visibleToasts = () => + document.querySelectorAll('[class*="mantine-Notification-root"]'); + +afterEach(() => { + act(() => clearToasts()); +}); + +describe('toast', () => { + it('shows a title and its description', async () => { + withToasts(); + + showToast({type: 'success', title: 'Saved', description: 'Channel settings written.'}); + + expect(await screen.findByText('Saved')).toBeInTheDocument(); + expect(screen.getByText('Channel settings written.')).toBeInTheDocument(); + }); + + it('shows a description on its own', async () => { + // Several call sites send no title. + withToasts(); + + showToast({description: 'Nothing to download.'}); + + expect(await screen.findByText('Nothing to download.')).toBeInTheDocument(); + }); + + it('stacks, rather than replacing what is already up', async () => { + /* + * The behaviour that matters when a background task reports repeatedly -- three + * downloads finishing should be three messages, not one that keeps being overwritten. + * Mantine replaces a notification when it is given an `id` it has already seen, so + * this would break the moment somebody added one to dedupe. + */ + withToasts(); + + showToast({title: 'First'}); + showToast({title: 'Second'}); + showToast({title: 'Third'}); + + await waitFor(() => expect(visibleToasts()).toHaveLength(3)); + expect(screen.getByText('First')).toBeInTheDocument(); + expect(screen.getByText('Third')).toBeInTheDocument(); + }); + + it('stacks two identical toasts as two', async () => { + // Same title and description twice over: still two messages. + withToasts(); + + showToast({title: 'Download failed', description: 'HTTP 403'}); + showToast({title: 'Download failed', description: 'HTTP 403'}); + + await waitFor(() => expect(visibleToasts()).toHaveLength(2)); + }); + + it('holds no more than the container allows', async () => { + // Five at once, so the sixth waits rather than the stack growing without limit. + withToasts(); + + for (let i = 1; i <= 7; i += 1) { + showToast({title: `Toast ${i}`}); + } + + await waitFor(() => expect(visibleToasts()).toHaveLength(5)); + }); + + it('clears everything on request', async () => { + // Used when navigating away from a failed action, so its errors do not follow you. + withToasts(); + showToast({title: 'One'}); + showToast({title: 'Two'}); + await waitFor(() => expect(visibleToasts()).toHaveLength(2)); + + act(() => clearToasts()); + + await waitFor(() => expect(visibleToasts()).toHaveLength(0)); + }); + + it('carries the colour its type maps to', async () => { + withToasts(); + + showToast({type: 'error', title: 'Download failed'}); + + await waitFor(() => expect(visibleToasts()).toHaveLength(1)); + // Mantine writes the resolved colour onto the element as a custom property. + const notification = visibleToasts()[0]; + expect(notification.getAttribute('style')).toContain('red'); + }); + + it('gives each type a colour of its own', async () => { + /* + * Four types that must not collapse into one in light and dark. Night and amber are + * the deliberate exception -- they have no second hue to spend -- but that is decided + * by the token tables, not here, and here they have to differ. + */ + withToasts(); + + const colours = new Set(); + for (const type of ['info', 'success', 'warning', 'error']) { + showToast({type, title: `A ${type}`}); + } + await waitFor(() => expect(visibleToasts()).toHaveLength(4)); + for (const notification of visibleToasts()) { + colours.add((notification.getAttribute('style') || '').match(/--notification-color:[^;]*/)?.[0]); + } + + expect(colours.size).toBe(4); + }); + + it('defaults to info when given no type', async () => { + withToasts(); + + showToast({title: 'Refresh started'}); + + await waitFor(() => expect(visibleToasts()).toHaveLength(1)); + expect(visibleToasts()[0].getAttribute('style')).toContain('blue'); + }); + + it('survives being called with nothing at all', () => { + // Defensive: a call site building options from an API response can end up with none. + withToasts(); + + expect(() => showToast()).not.toThrow(); + }); +}); + +describe('how long a toast stays up', () => { + /* + * Real timers, with short durations chosen by the caller. + * + * Fake timers do not work here. Hiding a notification runs an exit transition -- a timer + * plus a state update plus a transition end -- and advancing the clock, even awaited inside + * `act`, never gets the element removed from the DOM. Both the assertion and its control + * then read the same whatever the code does, which is a test that cannot fail. It read + * green against a deliberately planted `autoClose: 0` before this was rewritten. + */ + + it('goes away on its own', async () => { + withToasts(); + showToast({title: 'Saved', time: 60}); + + await waitFor(() => expect(screen.queryByText('Saved')).not.toBeInTheDocument()); + }); + + it('is still up a moment after being shown, when no time is given', async () => { + // The default is five seconds, which is not worth waiting for; what is worth asserting + // is that a caller who names no time does not get a toast that vanishes instantly. + withToasts(); + showToast({title: 'Refresh started'}); + + await new Promise(resolve => setTimeout(resolve, 150)); + + expect(screen.getByText('Refresh started')).toBeInTheDocument(); + }); + + it('stays until dismissed when the caller asks for time 0', async () => { + /* + * Semantic treated `time: 0` as "stay up". Mantine wants `autoClose: false`: its + * `getAutoClose` returns any number as given, and the container then calls + * `setTimeout(hide, 0)` -- so passing the 0 straight through dismisses the toast on the + * next tick. Call sites use 0 for errors the user must actually see, so getting this + * backwards would throw away precisely the messages that matter most. + * + * A toast that expires normally goes up alongside it as a control. Waiting for that one + * to disappear proves dismissal is working, which is what makes the survivor a fact + * about `time: 0` rather than about how long the test happened to wait. + */ + withToasts(); + showToast({title: 'Expires normally', time: 60}); + showToast({title: 'Download failed', time: 0}); + + await waitFor(() => expect(screen.queryByText('Expires normally')).not.toBeInTheDocument()); + + expect(screen.getByText('Download failed')).toBeInTheDocument(); + }); +}); diff --git a/app/src/components/ui/ui.css b/app/src/components/ui/ui.css index 16406f9b..6e1a6f8b 100644 --- a/app/src/components/ui/ui.css +++ b/app/src/components/ui/ui.css @@ -779,6 +779,54 @@ html[data-theme="amber"] .wrolpi-label { --label-text: var(--btn-text) !important; } +/* ----------------------------------------------------------------- Toasts */ +/* + * Notifications take their colours from our tokens rather than from Mantine's colour scheme. + * + * Mantine paints a dark-scheme notification title with `--mantine-color-white`, and the + * bridge aliases that to `--btn-text` -- deliberately, because it is the text drawn on a + * filled button, which is near-black in dark, night and amber. So the title came out at + * rgb(28, 31, 33) on a rgb(38, 41, 43) surface: about 1.1:1, present and invisible. Fixing + * the alias is not an option; it is correct for the buttons it exists for. + * + * The `html` prefix is only there to outrank Mantine's single-class rules. One set of values + * covers every theme, because the tokens are what differ. + */ + +html .mantine-Notification-root { + background-color: var(--panel); + border-color: var(--border); +} + +html .mantine-Notification-title { + color: var(--text); +} + +html .mantine-Notification-description { + color: var(--muted); +} + +/* + * The description is the message itself, so it has to be read, and `--muted` is too dim to + * read on a panel in the two monochrome themes -- 2.0:1 in night, 3.1:1 in amber, against + * 5.1:1 and 5.7:1 in dark and light. Those two drop the muting and let the title's weight + * carry the hierarchy instead, which is the same brightness-over-hue trade the rest of night + * makes. + */ +html[data-theme="night"] .mantine-Notification-description, +html[data-theme="amber"] .mantine-Notification-description { + color: var(--text); +} + +html .mantine-Notification-closeButton { + color: var(--muted); +} + +html .mantine-Notification-closeButton:hover { + background-color: var(--head); + color: var(--text); +} + /* --------------------------------------------------------------- Messages */ .wrolpi-message {