Skip to content

Commit 5afbb0d

Browse files
os-zhuangclaude
andcommitted
fix(notifications): the config, position and action variant are read instead of forked or ignored (#3014 follow-up)
The last of the notification contract. After displayType (#3071) and icon (#3076), four gaps of the same family were left: - the config was 3/4 inert: only `defaultDuration` was ever read, while `maxVisible` / `stacking` were carried and ignored and NotificationBanners capped at a hard-coded 3 of its own; - its field names forked from `NotificationConfigSchema` (`position` vs `defaultPosition`, a renderer-local `stacking` boolean, no `pauseOnHover`); - a notification could not declare a `position` at all — the #3008 parity guard asserted the position VOCABULARY while nothing positioned anything by it; - `NotificationActionButton.variant` was the shadcn Button vocabulary (`default | destructive | outline`) under a spec-shaped name, forking `NotificationActionSchema.variant` (`primary | secondary | link`). Positioning resolves as `notification.position ?? config.defaultPosition ?? nothing`, and "nothing" is a real answer: declared → the surface pins itself there and `presentNotificationToast` passes it per-toast so the contract beats the container; undeclared → the surface keeps its own anchor, or defers to the host's toast chrome. That asymmetry is the decision — the sonner container also serves toasts that are NOT spec notifications (the action runtime's own `toast.*` calls), so it stays the fallback authority for placement, never a competing one. Hence `defaultPosition` has no fabricated default: "the host didn't say" has to be representable. `maxVisible` / `stackDirection` now drive every stacking surface through one shared `visibleNotificationStack`; `pauseOnHover` holds a transient timer and resumes it with the time it had left, which needed the provider to track live timers instead of fire-and-forget setTimeouts. Legacy spellings still resolve: `position` folds into `defaultPosition`, `stacking: false` reads as `maxVisible: 1`. `onToast` gains the resolved config as a second argument (one-arg handlers are unaffected), and the spec-parity guard gained the action-variant vocabulary — the one notification enum it did not cover. Verified in the running console, in one frame: an undeclared toast stays where the sonner container puts it (bottom-right), a toast declaring `top_left` moves there, and a snackbar declaring `top_right` leaves its bottom anchor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c4d7b20 commit 5afbb0d

17 files changed

Lines changed: 752 additions & 63 deletions
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
"@object-ui/react": minor
3+
"@object-ui/components": minor
4+
"@object-ui/app-shell": minor
5+
---
6+
7+
fix(notifications): the config, `position` and action `variant` are read instead of forked or ignored (#3014 follow-up)
8+
9+
The last of the notification contract. After `displayType` (#3071) and `icon`
10+
(#3076), four gaps of the same family were left:
11+
12+
- **the config was 3/4 inert** — only `defaultDuration` was ever read.
13+
`maxVisible` and `stacking` were carried and ignored, while
14+
`NotificationBanners` capped at a hard-coded `3` of its own;
15+
- **its field names forked from `NotificationConfigSchema`**`position` vs
16+
`defaultPosition`, a renderer-local `stacking` boolean with no spec
17+
counterpart, and no `pauseOnHover` at all;
18+
- **a notification could not declare a `position`.** The #3008 parity guard
19+
asserted the position *vocabulary* matched the spec while nothing positioned
20+
anything by it — a guard passing over an unused value;
21+
- **`NotificationActionButton.variant` was the shadcn Button vocabulary**
22+
(`default | destructive | outline`) under a spec-shaped name, forking
23+
`NotificationActionSchema.variant` (`primary | secondary | link`).
24+
25+
**How positioning resolves now** — `notification.position ?? config.defaultPosition
26+
?? nothing`, and "nothing" is a real answer:
27+
28+
- **declared** → the surface pins itself there, always. `presentNotificationToast`
29+
passes it per-toast so the contract wins over the container;
30+
- **undeclared** → the surface keeps its own anchor (a snackbar's bottom edge) or
31+
defers to the host's toast chrome.
32+
33+
That asymmetry is the design decision. The host's sonner container also serves
34+
toasts that are *not* spec notifications (the console action runtime's own
35+
`toast.*` calls), so it stays the fallback authority for placement — never a
36+
competing one. A declared position a component prop could silently override
37+
would be the same "validates, then does nothing" shape this whole area is about.
38+
Hence `defaultPosition` has no fabricated default: "the host didn't say" has to
39+
be representable.
40+
41+
Also: `maxVisible` / `stackDirection` now drive every stacking surface through
42+
one shared `visibleNotificationStack` (cap keeps the NEWEST, stack grows in the
43+
declared direction); `pauseOnHover` holds a transient notification's timer and
44+
resumes it with the time it had left, which needed the provider to track live
45+
timers rather than fire-and-forget `setTimeout`s. Legacy spellings still resolve:
46+
`position` folds into `defaultPosition`, and `stacking: false` reads as
47+
`maxVisible: 1` rather than being ignored.
48+
49+
`onToast` now receives the resolved config as a second argument, so the delegate
50+
can apply the parts of the contract only it can. Existing one-argument handlers
51+
are unaffected. The spec-parity guard gained the action-variant vocabulary, the
52+
one notification enum it did not cover.

content/docs/guide/notifications.md

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,26 @@ notify({ title: 'Viewing a draft', severity: 'warning', displayType: 'banner' })
9696
notify({ title: 'Session expired', severity: 'error', displayType: 'alert' });
9797
```
9898

99+
### `actions`
100+
101+
An action's `variant` is the spec vocabulary — `primary` (default) / `secondary`
102+
/ `link` — describing the action's **role**. Each surface maps it onto its own
103+
button styling; it is not the shadcn Button vocabulary, which is a look.
104+
105+
```tsx
106+
notify({
107+
title: 'Storage almost full', severity: 'warning', displayType: 'banner',
108+
actions: [
109+
{ label: 'Upgrade', onClick: upgrade }, // primary by default
110+
{ label: 'Learn more', onClick: openDocs, variant: 'link' },
111+
],
112+
});
113+
```
114+
115+
A `snackbar` and a `toast` render only the **first** action — both have one
116+
action slot by nature. A `banner` and an `inline` render all of them; an `alert`
117+
renders them beside its acknowledge button.
118+
99119
### `inline` and `scope`
100120

101121
An inline notification is rendered by the surface that raised it. `scope` is the
@@ -130,6 +150,41 @@ deliberately *not* the generic `Database` glyph `getLazyIcon` returns for
130150
data-shaped schema slots, which on an error notification would replace a
131151
meaningful icon with a meaningless one.
132152

153+
### `position`
154+
155+
Honored by the **floating** presentations — `toast` and `snackbar`. `banner`,
156+
`inline` and `alert` are anchored by what they are (content top / in place /
157+
centred modal) and ignore it.
158+
159+
Resolution is `notification.position ?? config.defaultPosition ?? nothing`, and
160+
"nothing" is a real answer rather than a missing one:
161+
162+
- **declared** → the surface pins itself there, always;
163+
- **undeclared** → the surface keeps its own anchor (a snackbar's bottom edge)
164+
or defers to the host's toast chrome.
165+
166+
That asymmetry is deliberate. The host's toast container is shared with toasts
167+
that are *not* spec notifications (in the console, the action runtime's own
168+
`toast.*` calls), so it stays the fallback authority for placement — but never a
169+
competing one. A declared position that a component prop could silently override
170+
would be the same "validates, then does nothing" shape this whole area is about.
171+
172+
## Configuring the system
173+
174+
`NotificationProvider`'s `config` is the spec `NotificationConfigSchema`:
175+
176+
| Key | Default | Effect |
177+
|---|---|---|
178+
| `defaultPosition` || Fallback position for the floating presentations. Deliberately unset: see above. |
179+
| `defaultDuration` | `5000` | Auto-dismiss for the transient presentations. |
180+
| `maxVisible` | `5` | Cap on a stacking surface (banner, inline); the newest survive. |
181+
| `stackDirection` | `down` | Which way a stack grows — `down` puts the newest below. |
182+
| `pauseOnHover` | `true` | Hold a transient notification's timer while it is hovered. |
183+
184+
The legacy spellings `position` and `stacking` are still accepted:
185+
`defaultPosition` wins over `position`, and `stacking: false` reads as
186+
`maxVisible: 1` ("show only the newest") rather than being ignored.
187+
133188
### `dismissible`
134189

135190
Defaults to `true`. On the persistent presentations, `dismissible: false` removes

packages/app-shell/src/chrome/notificationToast.test.tsx

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import { describe, it, expect, vi, beforeEach } from 'vitest';
99
import { isValidElement } from 'react';
10-
import type { NotificationItem } from '@object-ui/react';
10+
import { resolveNotificationConfig, type NotificationItem } from '@object-ui/react';
1111

1212
vi.mock('sonner', () => ({
1313
toast: Object.assign(vi.fn(), {
@@ -133,4 +133,39 @@ describe('presentNotificationToast', () => {
133133
const options = vi.mocked(toast.success).mock.calls[0][1] as Record<string, unknown>;
134134
expect(options).not.toHaveProperty('icon');
135135
});
136+
137+
// Position is the half of the contract that had to be settled: declared wins,
138+
// undeclared defers to the sonner container (host chrome, which also places
139+
// the console's many direct `toast.*` calls).
140+
describe('position', () => {
141+
it('omits the key when neither the notification nor the config declares one', () => {
142+
presentNotificationToast(item(), resolveNotificationConfig());
143+
const options = vi.mocked(toast.success).mock.calls[0][1] as Record<string, unknown>;
144+
expect(options).not.toHaveProperty('position');
145+
});
146+
147+
it('passes the configured default, translated to sonner ids', () => {
148+
presentNotificationToast(item(), resolveNotificationConfig({ defaultPosition: 'top_right' }));
149+
expect(toast.success).toHaveBeenCalledWith('Saved', expect.objectContaining({
150+
position: 'top-right',
151+
}));
152+
});
153+
154+
it("lets the notification's own position win", () => {
155+
presentNotificationToast(
156+
item({ position: 'bottom_left' }),
157+
resolveNotificationConfig({ defaultPosition: 'top_right' }),
158+
);
159+
expect(toast.success).toHaveBeenCalledWith('Saved', expect.objectContaining({
160+
position: 'bottom-left',
161+
}));
162+
});
163+
164+
it('works with no config at all', () => {
165+
presentNotificationToast(item({ position: 'top_center' }));
166+
expect(toast.success).toHaveBeenCalledWith('Saved', expect.objectContaining({
167+
position: 'top-center',
168+
}));
169+
});
170+
});
136171
});

packages/app-shell/src/chrome/notificationToast.tsx

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,13 @@
1414
*/
1515

1616
import { toast } from 'sonner';
17-
import type { NotificationItem, NotificationSeverityLevel } from '@object-ui/react';
18-
import { isLucideIconName, LazyIcon } from '@object-ui/components';
17+
import {
18+
resolveNotificationPosition,
19+
type NotificationItem,
20+
type NotificationSeverityLevel,
21+
type ResolvedNotificationConfig,
22+
} from '@object-ui/react';
23+
import { isLucideIconName, LazyIcon, TOASTER_POSITIONS } from '@object-ui/components';
1924

2025
/**
2126
* Typed as `Record<NotificationSeverityLevel, …>` so a new severity in the spec
@@ -35,8 +40,19 @@ const TOAST_BY_SEVERITY: Record<NotificationSeverityLevel, (message: string, dat
3540
* Duration follows the notification contract: `0` means persistent (sonner's
3641
* `Infinity`), and an absent duration defers to the `ConsoleToaster` default
3742
* rather than being invented here.
43+
*
44+
* Position follows the same rule, and it is the one that had to be settled: a
45+
* DECLARED position (the notification's own, else `config.defaultPosition`) is
46+
* passed per-toast so the contract always wins, and an UNDECLARED one omits the
47+
* key entirely so the sonner container keeps placing it. That container is host
48+
* chrome — it also serves the console's many direct `toast.*` calls, which are
49+
* not spec notifications — so it stays the fallback authority, never a
50+
* competing one.
3851
*/
39-
export function presentNotificationToast(notification: NotificationItem): void {
52+
export function presentNotificationToast(
53+
notification: NotificationItem,
54+
config?: Pick<ResolvedNotificationConfig, 'defaultPosition'>,
55+
): void {
4056
const show = TOAST_BY_SEVERITY[notification.severity] ?? TOAST_BY_SEVERITY.info;
4157
// A toast carries at most one action button — sonner has one `action` slot,
4258
// and a notification needing more than one belongs on a banner or an alert.
@@ -48,12 +64,15 @@ export function presentNotificationToast(notification: NotificationItem): void {
4864
const icon = isLucideIconName(notification.icon)
4965
? <LazyIcon name={notification.icon} className="h-4 w-4" />
5066
: undefined;
67+
const declared = resolveNotificationPosition(notification, config);
68+
const position = declared ? TOASTER_POSITIONS[declared] : undefined;
5169

5270
show(notification.title, {
5371
description: notification.message,
5472
duration: notification.duration === 0 ? Infinity : notification.duration,
5573
dismissible: notification.dismissible,
5674
...(icon ? { icon } : {}),
75+
...(position ? { position } : {}),
5776
...(action ? { action: { label: action.label, onClick: action.onClick } } : {}),
5877
});
5978
}

packages/app-shell/src/console/ConsoleShell.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,13 @@ function GlobalActionRuntimeProvider({ dataSource, children }: { dataSource: unk
132132
export function ConsoleShell({ children }: { children: ReactNode }) {
133133
return (
134134
<ThemeProvider defaultTheme="system" storageKey="object-ui-theme">
135-
{/* `defaultDuration` matches ConsoleToaster's 4s toast default, so a
136-
snackbar and a toast raised together disappear together. */}
135+
{/* `defaultDuration` matches ConsoleToaster's 4s toast default and
136+
`maxVisible` its `visibleToasts={4}`, so a snackbar and a toast raised
137+
together disappear together and the banner stack caps like the toast
138+
stack. No `defaultPosition`: nothing declared means the sonner
139+
container keeps placing toasts (it also serves the console's direct
140+
`toast.*` calls) and the snackbar keeps its own bottom anchor — a
141+
notification that DECLARES a position still overrides both. */}
137142
<NotificationProvider config={{ defaultDuration: 4000, maxVisible: 4 }} onToast={presentNotificationToast}>
138143
<NavigationProvider>
139144
<UserStateAdaptersProvider>

packages/components/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ export * from './custom';
5858

5959
// Export the notification surfaces — one per spec `displayType` (#3014).
6060
export * from './notifications';
61+
// Spec position → sonner position ids, shared with the console's toast bridge.
62+
export { TOASTER_POSITIONS } from './renderers/feedback/toaster';
6163

6264
// Export hooks
6365
export { useConfigDraft } from './hooks/use-config-draft';

packages/components/src/notifications/NotificationAlerts.tsx

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,18 @@ import {
3232
AlertDialogHeader,
3333
AlertDialogTitle,
3434
} from '../ui/alert-dialog';
35-
import { notificationIcon, notificationSeverityStyle } from './severity';
35+
import { notificationActionVariant, notificationIcon, notificationSeverityStyle } from './severity';
36+
37+
/**
38+
* Button-variant classes for the dialog footer. `AlertDialogAction` bakes in
39+
* `buttonVariants()` (the `default` look), so a variant is expressed as an
40+
* override rather than a prop — `primary` needs none.
41+
*/
42+
const ALERT_ACTION_CLASSES: Record<'default' | 'secondary' | 'link', string | undefined> = {
43+
default: undefined,
44+
secondary: 'bg-secondary text-secondary-foreground hover:bg-secondary/80',
45+
link: 'bg-transparent text-primary underline-offset-4 hover:bg-transparent hover:underline',
46+
};
3647

3748
export interface NotificationAlertsProps {
3849
/** Extra classes for the dialog content. */
@@ -90,14 +101,18 @@ export function NotificationAlerts({ className, acknowledgeLabel = 'OK' }: Notif
90101
{notification.actions?.map((action) => (
91102
<AlertDialogAction
92103
key={action.label}
93-
className={cn(action.variant === 'destructive' && 'bg-destructive text-destructive-foreground hover:bg-destructive/90')}
104+
// AlertDialogAction hard-codes the default Button styling, so the
105+
// spec variant is applied as an override on top of it.
106+
className={cn(ALERT_ACTION_CLASSES[notificationActionVariant(action.variant)])}
94107
onClick={() => { action.onClick(); acknowledge(); }}
95108
>
96109
{action.label}
97110
</AlertDialogAction>
98111
))}
99112
<AlertDialogAction
100-
className={cn(notification.actions?.length ? 'bg-secondary text-secondary-foreground hover:bg-secondary/80' : undefined)}
113+
// Declared actions are the primary ones; the bare acknowledge steps
114+
// back to secondary when it is not the only button.
115+
className={cn(notification.actions?.length ? ALERT_ACTION_CLASSES.secondary : undefined)}
101116
onClick={acknowledge}
102117
>
103118
{acknowledgeLabel}

packages/components/src/notifications/NotificationBanners.tsx

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,18 @@
1818

1919
import * as React from 'react';
2020
import { X } from 'lucide-react';
21-
import { useNotifications, useNotificationsByPresentation } from '@object-ui/react';
21+
import {
22+
useNotifications,
23+
useNotificationsByPresentation,
24+
visibleNotificationStack,
25+
} from '@object-ui/react';
2226
import { cn } from '../lib/utils';
2327
import { Button } from '../ui/button';
24-
import { notificationIcon, notificationSeverityStyle } from './severity';
28+
import { notificationActionVariant, notificationIcon, notificationSeverityStyle } from './severity';
2529

2630
export interface NotificationBannersProps {
2731
/** Extra classes for the banner stack container. */
2832
className?: string;
29-
/**
30-
* Cap on simultaneously rendered banners (newest first). Banners are
31-
* persistent, so an uncapped stack can swallow the viewport. Default 3.
32-
*/
33-
max?: number;
3433
}
3534

3635
/**
@@ -44,15 +43,19 @@ export interface NotificationBannersProps {
4443
* </main>
4544
* ```
4645
*/
47-
export function NotificationBanners({ className, max = 3 }: NotificationBannersProps) {
46+
export function NotificationBanners({ className }: NotificationBannersProps) {
4847
const banners = useNotificationsByPresentation('banner');
49-
const { dismiss } = useNotifications();
48+
const { dismiss, config } = useNotifications();
5049

5150
if (banners.length === 0) return null;
5251

52+
// `maxVisible` / `stackDirection` come from the provider config — the spec's
53+
// own knobs — instead of a cap this component invents for itself.
54+
const visible = visibleNotificationStack(banners, config);
55+
5356
return (
5457
<div className={cn('flex w-full flex-col', className)} data-notification-surface="banner">
55-
{banners.slice(0, max).map((notification) => {
58+
{visible.map((notification) => {
5659
const { tone } = notificationSeverityStyle(notification.severity);
5760
const Icon = notificationIcon(notification);
5861
return (
@@ -76,7 +79,7 @@ export function NotificationBanners({ className, max = 3 }: NotificationBannersP
7679
<Button
7780
key={action.label}
7881
size="sm"
79-
variant={action.variant ?? 'outline'}
82+
variant={notificationActionVariant(action.variant)}
8083
className="h-7 shrink-0"
8184
onClick={action.onClick}
8285
>

packages/components/src/notifications/NotificationInline.tsx

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,14 @@
1818

1919
import * as React from 'react';
2020
import { X } from 'lucide-react';
21-
import { useNotifications, useNotificationsByPresentation } from '@object-ui/react';
21+
import {
22+
useNotifications,
23+
useNotificationsByPresentation,
24+
visibleNotificationStack,
25+
} from '@object-ui/react';
2226
import { cn } from '../lib/utils';
2327
import { Button } from '../ui/button';
24-
import { notificationIcon, notificationSeverityStyle } from './severity';
28+
import { notificationActionVariant, notificationIcon, notificationSeverityStyle } from './severity';
2529

2630
export interface NotificationInlineProps {
2731
/**
@@ -46,17 +50,20 @@ export interface NotificationInlineProps {
4650
*/
4751
export function NotificationInline({ scope, className }: NotificationInlineProps) {
4852
const items = useNotificationsByPresentation('inline', scope);
49-
const { dismiss } = useNotifications();
53+
const { dismiss, config } = useNotifications();
5054

5155
if (items.length === 0) return null;
5256

57+
// Same `maxVisible` / `stackDirection` contract as the banner stack.
58+
const visible = visibleNotificationStack(items, config);
59+
5360
return (
5461
<div
5562
className={cn('flex w-full flex-col gap-2', className)}
5663
data-notification-surface="inline"
5764
data-scope={scope}
5865
>
59-
{items.map((notification) => {
66+
{visible.map((notification) => {
6067
const { tone } = notificationSeverityStyle(notification.severity);
6168
const Icon = notificationIcon(notification);
6269
return (
@@ -78,7 +85,7 @@ export function NotificationInline({ scope, className }: NotificationInlineProps
7885
<Button
7986
key={action.label}
8087
size="sm"
81-
variant={action.variant ?? 'outline'}
88+
variant={notificationActionVariant(action.variant)}
8289
className="h-7"
8390
onClick={action.onClick}
8491
>

0 commit comments

Comments
 (0)