mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
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.
This commit is contained in:
Roland
parent
db3889589f
commit
213b940a1b
4 changed files with 334 additions and 1 deletions
|
|
@ -215,7 +215,12 @@ export function ThemeProvider({children, ...props}: ThemeProviderProps) {
|
|||
cssVariablesResolver={cssVariablesResolver}
|
||||
forceColorScheme={dark ? 'dark' : 'light'}
|
||||
>
|
||||
<Notifications position='bottom-right' limit={5}/>
|
||||
{/*
|
||||
* 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.
|
||||
*/}
|
||||
<Notifications position='top-right' limit={5}/>
|
||||
{children}
|
||||
</MantineProvider>
|
||||
</ThemeContext.Provider>
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ import {
|
|||
ThemePicker,
|
||||
Toggle,
|
||||
Tooltip,
|
||||
clearToasts,
|
||||
toast,
|
||||
} from './ui';
|
||||
import {semanticColorNames} from '../themes/mantine';
|
||||
|
|
@ -61,6 +62,10 @@ const Row = ({children}) => <div style={{display: 'flex', gap: 10, flexWrap: 'wr
|
|||
{children}
|
||||
</div>;
|
||||
|
||||
// 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() {
|
|||
</Panel>
|
||||
</Section>
|
||||
|
||||
<Section label='Toasts'>
|
||||
<Panel>
|
||||
{/*
|
||||
* `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.
|
||||
*/}
|
||||
<Row>
|
||||
<Button role='primary' onClick={() => toast({
|
||||
type: 'info', title: 'Refresh started',
|
||||
description: 'Indexing the media directory.',
|
||||
})}>Info</Button>
|
||||
<Button role='save' onClick={() => toast({
|
||||
type: 'success', title: 'Saved',
|
||||
description: 'Channel settings written to config/channels.yaml.',
|
||||
})}>Success</Button>
|
||||
<Button role='cancel' onClick={() => toast({
|
||||
type: 'warning', title: 'Daily download limit reached',
|
||||
description: 'Downloads will resume tomorrow at 00:00.',
|
||||
})}>Warning</Button>
|
||||
<Button role='danger' onClick={() => toast({
|
||||
type: 'error', title: 'Download failed',
|
||||
description: 'HTTP 403 from the remote server after 3 attempts.',
|
||||
})}>Error</Button>
|
||||
</Row>
|
||||
|
||||
<Row>
|
||||
{/* Click this repeatedly: each press is a separate toast, and the
|
||||
container holds five at once. */}
|
||||
<Button role='cancel' onClick={() => toast({
|
||||
type: 'info', title: `Stacked toast ${++toastCounter}`,
|
||||
description: 'Press again before this one expires.',
|
||||
})}>Stack one more</Button>
|
||||
<Button role='cancel' onClick={() => {
|
||||
// All five at once, to see the container at its limit.
|
||||
for (let i = 1; i <= 5; i += 1) {
|
||||
toast({
|
||||
type: 'info', title: `Toast ${i} of 5`,
|
||||
description: 'Filling the container to its limit.',
|
||||
});
|
||||
}
|
||||
}}>Fill the stack</Button>
|
||||
<Button role='cancel' onClick={() => toast({
|
||||
type: 'error', title: 'This one stays',
|
||||
description: 'time: 0 means it waits to be dismissed. Close it yourself.',
|
||||
time: 0,
|
||||
})}>Never expires</Button>
|
||||
<Button role='cancel' onClick={() => toast({
|
||||
type: 'warning',
|
||||
description: 'No title, only a description — some call sites send just this.',
|
||||
})}>No title</Button>
|
||||
<Button role='cancel' onClick={() => toast({
|
||||
type: 'error', title: 'A long one, to see how it wraps',
|
||||
description: 'Failed to fetch https://example.com/a/very/long/path/that/'
|
||||
+ 'keeps/going/for/a/while.html: the remote server closed the '
|
||||
+ 'connection after sending part of the response body.',
|
||||
})}>Long text</Button>
|
||||
<Button role='danger' onClick={() => clearToasts()}>Clear all</Button>
|
||||
</Row>
|
||||
|
||||
<p style={{fontSize: 12, color: 'var(--muted)', marginBottom: 0}}>
|
||||
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.
|
||||
</p>
|
||||
</Panel>
|
||||
</Section>
|
||||
|
||||
<Section label='Messages'>
|
||||
<div style={{display: 'flex', flexDirection: 'column', gap: 10}}>
|
||||
<Message kind='info' title='Refresh started'>
|
||||
|
|
|
|||
204
app/src/components/ui/toast.test.js
Normal file
204
app/src/components/ui/toast.test.js
Normal file
|
|
@ -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(
|
||||
<MantineProvider theme={mantineTheme} cssVariablesResolver={cssVariablesResolver}>
|
||||
<Notifications position='top-right' limit={5}/>
|
||||
</MantineProvider>
|
||||
);
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue