diff --git a/.changeset/notification-config-contract.md b/.changeset/notification-config-contract.md new file mode 100644 index 000000000..5111a8b65 --- /dev/null +++ b/.changeset/notification-config-contract.md @@ -0,0 +1,52 @@ +--- +"@object-ui/react": minor +"@object-ui/components": minor +"@object-ui/app-shell": minor +--- + +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. + `maxVisible` and `stacking` were carried and ignored, while + `NotificationBanners` capped at a hard-coded `3` of its own; +- **its field names forked from `NotificationConfigSchema`** — `position` vs + `defaultPosition`, a renderer-local `stacking` boolean with no spec + counterpart, and no `pauseOnHover` at all; +- **a notification could not declare a `position`.** The #3008 parity guard + asserted the position *vocabulary* matched the spec while nothing positioned + anything by it — a guard passing over an unused value; +- **`NotificationActionButton.variant` was the shadcn Button vocabulary** + (`default | destructive | outline`) under a spec-shaped name, forking + `NotificationActionSchema.variant` (`primary | secondary | link`). + +**How positioning resolves now** — `notification.position ?? config.defaultPosition +?? nothing`, and "nothing" is a real answer: + +- **declared** → the surface pins itself there, always. `presentNotificationToast` + passes it per-toast so the contract wins over the container; +- **undeclared** → the surface keeps its own anchor (a snackbar's bottom edge) or + defers to the host's toast chrome. + +That asymmetry is the design decision. The host's sonner container also serves +toasts that are *not* spec notifications (the console action runtime's own +`toast.*` calls), so it stays the fallback authority for placement — never a +competing one. A declared position a component prop could silently override +would be the same "validates, then does nothing" shape this whole area is about. +Hence `defaultPosition` has no fabricated default: "the host didn't say" has to +be representable. + +Also: `maxVisible` / `stackDirection` now drive every stacking surface through +one shared `visibleNotificationStack` (cap keeps the NEWEST, stack grows in the +declared direction); `pauseOnHover` holds a transient notification's timer and +resumes it with the time it had left, which needed the provider to track live +timers rather than fire-and-forget `setTimeout`s. Legacy spellings still resolve: +`position` folds into `defaultPosition`, and `stacking: false` reads as +`maxVisible: 1` rather than being ignored. + +`onToast` now receives the resolved config as a second argument, so the delegate +can apply the parts of the contract only it can. Existing one-argument handlers +are unaffected. The spec-parity guard gained the action-variant vocabulary, the +one notification enum it did not cover. diff --git a/content/docs/guide/notifications.md b/content/docs/guide/notifications.md index 843d06e34..325ed0c50 100644 --- a/content/docs/guide/notifications.md +++ b/content/docs/guide/notifications.md @@ -96,6 +96,26 @@ notify({ title: 'Viewing a draft', severity: 'warning', displayType: 'banner' }) notify({ title: 'Session expired', severity: 'error', displayType: 'alert' }); ``` +### `actions` + +An action's `variant` is the spec vocabulary — `primary` (default) / `secondary` +/ `link` — describing the action's **role**. Each surface maps it onto its own +button styling; it is not the shadcn Button vocabulary, which is a look. + +```tsx +notify({ + title: 'Storage almost full', severity: 'warning', displayType: 'banner', + actions: [ + { label: 'Upgrade', onClick: upgrade }, // primary by default + { label: 'Learn more', onClick: openDocs, variant: 'link' }, + ], +}); +``` + +A `snackbar` and a `toast` render only the **first** action — both have one +action slot by nature. A `banner` and an `inline` render all of them; an `alert` +renders them beside its acknowledge button. + ### `inline` and `scope` 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 data-shaped schema slots, which on an error notification would replace a meaningful icon with a meaningless one. +### `position` + +Honored by the **floating** presentations — `toast` and `snackbar`. `banner`, +`inline` and `alert` are anchored by what they are (content top / in place / +centred modal) and ignore it. + +Resolution is `notification.position ?? config.defaultPosition ?? nothing`, and +"nothing" is a real answer rather than a missing one: + +- **declared** → the surface pins itself there, always; +- **undeclared** → the surface keeps its own anchor (a snackbar's bottom edge) + or defers to the host's toast chrome. + +That asymmetry is deliberate. The host's toast container is shared with toasts +that are *not* spec notifications (in the console, the action runtime's own +`toast.*` calls), so it stays the fallback authority for placement — but never a +competing one. A declared position that a component prop could silently override +would be the same "validates, then does nothing" shape this whole area is about. + +## Configuring the system + +`NotificationProvider`'s `config` is the spec `NotificationConfigSchema`: + +| Key | Default | Effect | +|---|---|---| +| `defaultPosition` | — | Fallback position for the floating presentations. Deliberately unset: see above. | +| `defaultDuration` | `5000` | Auto-dismiss for the transient presentations. | +| `maxVisible` | `5` | Cap on a stacking surface (banner, inline); the newest survive. | +| `stackDirection` | `down` | Which way a stack grows — `down` puts the newest below. | +| `pauseOnHover` | `true` | Hold a transient notification's timer while it is hovered. | + +The legacy spellings `position` and `stacking` are still accepted: +`defaultPosition` wins over `position`, and `stacking: false` reads as +`maxVisible: 1` ("show only the newest") rather than being ignored. + ### `dismissible` Defaults to `true`. On the persistent presentations, `dismissible: false` removes diff --git a/packages/app-shell/src/chrome/notificationToast.test.tsx b/packages/app-shell/src/chrome/notificationToast.test.tsx index cb231bd8d..cb8ced77e 100644 --- a/packages/app-shell/src/chrome/notificationToast.test.tsx +++ b/packages/app-shell/src/chrome/notificationToast.test.tsx @@ -7,7 +7,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { isValidElement } from 'react'; -import type { NotificationItem } from '@object-ui/react'; +import { resolveNotificationConfig, type NotificationItem } from '@object-ui/react'; vi.mock('sonner', () => ({ toast: Object.assign(vi.fn(), { @@ -133,4 +133,39 @@ describe('presentNotificationToast', () => { const options = vi.mocked(toast.success).mock.calls[0][1] as Record; expect(options).not.toHaveProperty('icon'); }); + + // Position is the half of the contract that had to be settled: declared wins, + // undeclared defers to the sonner container (host chrome, which also places + // the console's many direct `toast.*` calls). + describe('position', () => { + it('omits the key when neither the notification nor the config declares one', () => { + presentNotificationToast(item(), resolveNotificationConfig()); + const options = vi.mocked(toast.success).mock.calls[0][1] as Record; + expect(options).not.toHaveProperty('position'); + }); + + it('passes the configured default, translated to sonner ids', () => { + presentNotificationToast(item(), resolveNotificationConfig({ defaultPosition: 'top_right' })); + expect(toast.success).toHaveBeenCalledWith('Saved', expect.objectContaining({ + position: 'top-right', + })); + }); + + it("lets the notification's own position win", () => { + presentNotificationToast( + item({ position: 'bottom_left' }), + resolveNotificationConfig({ defaultPosition: 'top_right' }), + ); + expect(toast.success).toHaveBeenCalledWith('Saved', expect.objectContaining({ + position: 'bottom-left', + })); + }); + + it('works with no config at all', () => { + presentNotificationToast(item({ position: 'top_center' })); + expect(toast.success).toHaveBeenCalledWith('Saved', expect.objectContaining({ + position: 'top-center', + })); + }); + }); }); diff --git a/packages/app-shell/src/chrome/notificationToast.tsx b/packages/app-shell/src/chrome/notificationToast.tsx index 4f4e17561..bde4d3a9d 100644 --- a/packages/app-shell/src/chrome/notificationToast.tsx +++ b/packages/app-shell/src/chrome/notificationToast.tsx @@ -14,8 +14,13 @@ */ import { toast } from 'sonner'; -import type { NotificationItem, NotificationSeverityLevel } from '@object-ui/react'; -import { isLucideIconName, LazyIcon } from '@object-ui/components'; +import { + resolveNotificationPosition, + type NotificationItem, + type NotificationSeverityLevel, + type ResolvedNotificationConfig, +} from '@object-ui/react'; +import { isLucideIconName, LazyIcon, TOASTER_POSITIONS } from '@object-ui/components'; /** * Typed as `Record` so a new severity in the spec @@ -35,8 +40,19 @@ const TOAST_BY_SEVERITY: Record, +): void { const show = TOAST_BY_SEVERITY[notification.severity] ?? TOAST_BY_SEVERITY.info; // A toast carries at most one action button — sonner has one `action` slot, // and a notification needing more than one belongs on a banner or an alert. @@ -48,12 +64,15 @@ export function presentNotificationToast(notification: NotificationItem): void { const icon = isLucideIconName(notification.icon) ? : undefined; + const declared = resolveNotificationPosition(notification, config); + const position = declared ? TOASTER_POSITIONS[declared] : undefined; show(notification.title, { description: notification.message, duration: notification.duration === 0 ? Infinity : notification.duration, dismissible: notification.dismissible, ...(icon ? { icon } : {}), + ...(position ? { position } : {}), ...(action ? { action: { label: action.label, onClick: action.onClick } } : {}), }); } diff --git a/packages/app-shell/src/console/ConsoleShell.tsx b/packages/app-shell/src/console/ConsoleShell.tsx index eeb7c42f4..280613124 100644 --- a/packages/app-shell/src/console/ConsoleShell.tsx +++ b/packages/app-shell/src/console/ConsoleShell.tsx @@ -132,8 +132,13 @@ function GlobalActionRuntimeProvider({ dataSource, children }: { dataSource: unk export function ConsoleShell({ children }: { children: ReactNode }) { return ( - {/* `defaultDuration` matches ConsoleToaster's 4s toast default, so a - snackbar and a toast raised together disappear together. */} + {/* `defaultDuration` matches ConsoleToaster's 4s toast default and + `maxVisible` its `visibleToasts={4}`, so a snackbar and a toast raised + together disappear together and the banner stack caps like the toast + stack. No `defaultPosition`: nothing declared means the sonner + container keeps placing toasts (it also serves the console's direct + `toast.*` calls) and the snackbar keeps its own bottom anchor — a + notification that DECLARES a position still overrides both. */} diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index ec7754b50..e2537d8f2 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -58,6 +58,8 @@ export * from './custom'; // Export the notification surfaces — one per spec `displayType` (#3014). export * from './notifications'; +// Spec position → sonner position ids, shared with the console's toast bridge. +export { TOASTER_POSITIONS } from './renderers/feedback/toaster'; // Export hooks export { useConfigDraft } from './hooks/use-config-draft'; diff --git a/packages/components/src/notifications/NotificationAlerts.tsx b/packages/components/src/notifications/NotificationAlerts.tsx index 107c85e35..70a621318 100644 --- a/packages/components/src/notifications/NotificationAlerts.tsx +++ b/packages/components/src/notifications/NotificationAlerts.tsx @@ -32,7 +32,18 @@ import { AlertDialogHeader, AlertDialogTitle, } from '../ui/alert-dialog'; -import { notificationIcon, notificationSeverityStyle } from './severity'; +import { notificationActionVariant, notificationIcon, notificationSeverityStyle } from './severity'; + +/** + * Button-variant classes for the dialog footer. `AlertDialogAction` bakes in + * `buttonVariants()` (the `default` look), so a variant is expressed as an + * override rather than a prop — `primary` needs none. + */ +const ALERT_ACTION_CLASSES: Record<'default' | 'secondary' | 'link', string | undefined> = { + default: undefined, + secondary: 'bg-secondary text-secondary-foreground hover:bg-secondary/80', + link: 'bg-transparent text-primary underline-offset-4 hover:bg-transparent hover:underline', +}; export interface NotificationAlertsProps { /** Extra classes for the dialog content. */ @@ -90,14 +101,18 @@ export function NotificationAlerts({ className, acknowledgeLabel = 'OK' }: Notif {notification.actions?.map((action) => ( { action.onClick(); acknowledge(); }} > {action.label} ))} {acknowledgeLabel} diff --git a/packages/components/src/notifications/NotificationBanners.tsx b/packages/components/src/notifications/NotificationBanners.tsx index fb6476a13..93db39785 100644 --- a/packages/components/src/notifications/NotificationBanners.tsx +++ b/packages/components/src/notifications/NotificationBanners.tsx @@ -18,19 +18,18 @@ import * as React from 'react'; import { X } from 'lucide-react'; -import { useNotifications, useNotificationsByPresentation } from '@object-ui/react'; +import { + useNotifications, + useNotificationsByPresentation, + visibleNotificationStack, +} from '@object-ui/react'; import { cn } from '../lib/utils'; import { Button } from '../ui/button'; -import { notificationIcon, notificationSeverityStyle } from './severity'; +import { notificationActionVariant, notificationIcon, notificationSeverityStyle } from './severity'; export interface NotificationBannersProps { /** Extra classes for the banner stack container. */ className?: string; - /** - * Cap on simultaneously rendered banners (newest first). Banners are - * persistent, so an uncapped stack can swallow the viewport. Default 3. - */ - max?: number; } /** @@ -44,15 +43,19 @@ export interface NotificationBannersProps { * * ``` */ -export function NotificationBanners({ className, max = 3 }: NotificationBannersProps) { +export function NotificationBanners({ className }: NotificationBannersProps) { const banners = useNotificationsByPresentation('banner'); - const { dismiss } = useNotifications(); + const { dismiss, config } = useNotifications(); if (banners.length === 0) return null; + // `maxVisible` / `stackDirection` come from the provider config — the spec's + // own knobs — instead of a cap this component invents for itself. + const visible = visibleNotificationStack(banners, config); + return (
- {banners.slice(0, max).map((notification) => { + {visible.map((notification) => { const { tone } = notificationSeverityStyle(notification.severity); const Icon = notificationIcon(notification); return ( @@ -76,7 +79,7 @@ export function NotificationBanners({ className, max = 3 }: NotificationBannersP