diff --git a/.changeset/notification-icon-override.md b/.changeset/notification-icon-override.md new file mode 100644 index 0000000000..36e5d5fc30 --- /dev/null +++ b/.changeset/notification-icon-override.md @@ -0,0 +1,23 @@ +--- +"@object-ui/components": minor +"@object-ui/app-shell": minor +--- + +fix(notifications): the spec `icon` is read instead of stored and ignored (#3014 follow-up) + +`NotificationSchema.icon` — "Icon name override" — reached `NotificationItem` and +stopped there. Every surface drew the severity icon, so an author writing +`icon: 'rocket'` got the success checkmark. Same shape as the `displayType` +collapse #3071 fixed: a value that validates, is carried, and renders nothing. + +All five presentations now resolve it through one rule (`notificationIcon`): a +declared Lucide name — kebab-case or PascalCase — replaces the severity icon; +anything else falls back to it. That includes the console's sonner toast, so the +override behaves identically on a toast, a banner, a snackbar, an alert and an +inline message. + +**The fallback is the interesting part.** `getLazyIcon` degrades an unknown name +to a `Database` glyph, which is right for a data-shaped schema slot and wrong +here — on an error notification it swaps a meaningful icon for a meaningless one. +So the name is checked first, via a new `isLucideIconName` export, and a typo +costs the author their override and nothing more. diff --git a/content/docs/guide/notifications.md b/content/docs/guide/notifications.md index c96d7d66a9..843d06e340 100644 --- a/content/docs/guide/notifications.md +++ b/content/docs/guide/notifications.md @@ -115,6 +115,21 @@ Omit `scope` on both ends for a page-level inline outlet. `scope` is renderer-lo routing metadata, not a spec field — the spec describes what a notification *is*, not which React subtree hosts it. +### `icon` + +Every surface — including the console's sonner toast — resolves `icon` through +the same rule: a declared Lucide name (kebab-case or PascalCase) replaces the +severity icon; anything else falls back to it. + +```tsx +notify({ title: 'Deploy finished', severity: 'success', displayType: 'banner', icon: 'rocket' }); +``` + +A name Lucide doesn't have costs the author their override and nothing more — +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. + ### `dismissible` Defaults to `true`. On the persistent presentations, `dismissible: false` removes diff --git a/packages/app-shell/src/chrome/notificationToast.test.ts b/packages/app-shell/src/chrome/notificationToast.test.tsx similarity index 70% rename from packages/app-shell/src/chrome/notificationToast.test.ts rename to packages/app-shell/src/chrome/notificationToast.test.tsx index 36b2854dc7..cb231bd8dd 100644 --- a/packages/app-shell/src/chrome/notificationToast.test.ts +++ b/packages/app-shell/src/chrome/notificationToast.test.tsx @@ -6,6 +6,7 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { isValidElement } from 'react'; import type { NotificationItem } from '@object-ui/react'; vi.mock('sonner', () => ({ @@ -16,6 +17,10 @@ vi.mock('sonner', () => ({ import { toast } from 'sonner'; import { presentNotificationToast } from './notificationToast'; +// Imported at module scope, not inside a test: `@object-ui/components` is a +// heavy barrel and resolving it mid-assertion would race the RTL timeouts +// (AGENTS.md § 测试纪律). +import '@object-ui/components'; function item(overrides: Partial = {}): NotificationItem { return { @@ -101,4 +106,31 @@ describe('presentNotificationToast', () => { dismissible: false, })); }); + + it('passes the spec icon override to sonner', () => { + presentNotificationToast(item({ icon: 'rocket' })); + const { icon } = vi.mocked(toast.success).mock.calls[0][1] as { icon?: unknown }; + expect(isValidElement(icon)).toBe(true); + expect((icon as { props: { name?: string } }).props.name).toBe('rocket'); + }); + + it('accepts a PascalCase icon name', () => { + presentNotificationToast(item({ icon: 'CircleCheck' })); + const { icon } = vi.mocked(toast.success).mock.calls[0][1] as { icon?: unknown }; + expect(isValidElement(icon)).toBe(true); + }); + + it('omits the icon key for a name Lucide does not have', () => { + // Passing it through would render LazyIcon's `Database` fallback — a + // meaningless glyph where ConsoleToaster's severity icon belongs. + presentNotificationToast(item({ icon: 'not-a-real-icon' })); + const options = vi.mocked(toast.success).mock.calls[0][1] as Record; + expect(options).not.toHaveProperty('icon'); + }); + + it('omits the icon key when none is declared', () => { + presentNotificationToast(item()); + const options = vi.mocked(toast.success).mock.calls[0][1] as Record; + expect(options).not.toHaveProperty('icon'); + }); }); diff --git a/packages/app-shell/src/chrome/notificationToast.ts b/packages/app-shell/src/chrome/notificationToast.tsx similarity index 81% rename from packages/app-shell/src/chrome/notificationToast.ts rename to packages/app-shell/src/chrome/notificationToast.tsx index 45aaa937df..4f4e175615 100644 --- a/packages/app-shell/src/chrome/notificationToast.ts +++ b/packages/app-shell/src/chrome/notificationToast.tsx @@ -15,6 +15,7 @@ import { toast } from 'sonner'; import type { NotificationItem, NotificationSeverityLevel } from '@object-ui/react'; +import { isLucideIconName, LazyIcon } from '@object-ui/components'; /** * Typed as `Record` so a new severity in the spec @@ -40,11 +41,19 @@ export function presentNotificationToast(notification: NotificationItem): void { // 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. const action = notification.actions?.[0]; + // The spec `icon` override, matching what the four surface components do. + // Only a REAL Lucide name is passed: `LazyIcon` degrades an unknown one to a + // `Database` glyph, whereas omitting the key leaves ConsoleToaster's severity + // icon in place — the better fallback for a typo. + const icon = isLucideIconName(notification.icon) + ? + : undefined; show(notification.title, { description: notification.message, duration: notification.duration === 0 ? Infinity : notification.duration, dismissible: notification.dismissible, + ...(icon ? { icon } : {}), ...(action ? { action: { label: action.label, onClick: action.onClick } } : {}), }); } diff --git a/packages/components/README.md b/packages/components/README.md index 240401c8df..eb9337fe2d 100644 --- a/packages/components/README.md +++ b/packages/components/README.md @@ -163,7 +163,9 @@ raised through `NotificationProvider` from `@object-ui/react`. One per spec | `` | `alert` | anywhere inside the provider — blocking dialog, FIFO queue | | `` | `inline` | in the surface that raises them | -`toast` stays with the host's `onToast` delegate (sonner in the console). See the +`toast` stays with the host's `onToast` delegate (sonner in the console). All of +them draw the notification's `severity` icon unless it declares an `icon` +override naming a real Lucide icon. See the [notifications guide](https://objectui.org/docs/guide/notifications). ```tsx diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index 16bdb802d5..ec7754b501 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -39,7 +39,7 @@ import './renderers'; export { cn } from './lib/utils'; export { renderChildren } from './lib/utils'; export { cva } from 'class-variance-authority'; -export { getLazyIcon, LazyIcon, toKebabIconName } from './lib/lazy-icon'; +export { getLazyIcon, isLucideIconName, LazyIcon, toKebabIconName } from './lib/lazy-icon'; // Export placeholder registration export { registerPlaceholders } from './renderers/placeholders'; diff --git a/packages/components/src/lib/lazy-icon.tsx b/packages/components/src/lib/lazy-icon.tsx index 92371f3182..5ce39b479f 100644 --- a/packages/components/src/lib/lazy-icon.tsx +++ b/packages/components/src/lib/lazy-icon.tsx @@ -40,6 +40,19 @@ function isLucideIcon(kebab: string): boolean { return VALID_ICON_NAMES.has(kebab); } +/** + * Whether `name` (kebab-case or PascalCase) resolves to a real Lucide icon. + * + * Exported because `getLazyIcon` degrades an unknown name to the `Database` + * icon, which is the right default for a data-shaped schema slot but wrong + * where a caller has a BETTER fallback of its own — a notification, for + * instance, would rather show its severity icon than a stray database glyph. + * Ask first, then choose. + */ +export function isLucideIconName(name?: string): boolean { + return !!name && isLucideIcon(toKebabIconName(name)); +} + const cache = new Map(); /** diff --git a/packages/components/src/notifications/NotificationAlerts.tsx b/packages/components/src/notifications/NotificationAlerts.tsx index dfc73b4177..107c85e35b 100644 --- a/packages/components/src/notifications/NotificationAlerts.tsx +++ b/packages/components/src/notifications/NotificationAlerts.tsx @@ -32,7 +32,7 @@ import { AlertDialogHeader, AlertDialogTitle, } from '../ui/alert-dialog'; -import { notificationSeverityStyle } from './severity'; +import { notificationIcon, notificationSeverityStyle } from './severity'; export interface NotificationAlertsProps { /** Extra classes for the dialog content. */ @@ -61,7 +61,8 @@ export function NotificationAlerts({ className, acknowledgeLabel = 'OK' }: Notif const notification = alerts[alerts.length - 1]; if (!notification) return null; - const { Icon, iconTone } = notificationSeverityStyle(notification.severity); + const { iconTone } = notificationSeverityStyle(notification.severity); + const Icon = notificationIcon(notification); const acknowledge = () => dismiss(notification.id); // `dismissible: false` removes the wave-it-away paths (Escape); it never // removes the acknowledge button — an alert nobody can acknowledge is a trap. @@ -77,6 +78,7 @@ export function NotificationAlerts({ className, acknowledgeLabel = 'OK' }: Notif > + {/* eslint-disable-next-line react-hooks/static-components -- notificationIcon returns a module-cached stable component per name, not one created during render */} diff --git a/packages/components/src/notifications/NotificationBanners.tsx b/packages/components/src/notifications/NotificationBanners.tsx index 8495fc1b7d..fb6476a13f 100644 --- a/packages/components/src/notifications/NotificationBanners.tsx +++ b/packages/components/src/notifications/NotificationBanners.tsx @@ -21,7 +21,7 @@ import { X } from 'lucide-react'; import { useNotifications, useNotificationsByPresentation } from '@object-ui/react'; import { cn } from '../lib/utils'; import { Button } from '../ui/button'; -import { notificationSeverityStyle } from './severity'; +import { notificationIcon, notificationSeverityStyle } from './severity'; export interface NotificationBannersProps { /** Extra classes for the banner stack container. */ @@ -53,7 +53,8 @@ export function NotificationBanners({ className, max = 3 }: NotificationBannersP return (
{banners.slice(0, max).map((notification) => { - const { Icon, tone } = notificationSeverityStyle(notification.severity); + const { tone } = notificationSeverityStyle(notification.severity); + const Icon = notificationIcon(notification); return (
{items.map((notification) => { - const { Icon, tone } = notificationSeverityStyle(notification.severity); + const { tone } = notificationSeverityStyle(notification.severity); + const Icon = notificationIcon(notification); return (
+ {/* eslint-disable-next-line react-hooks/static-components -- notificationIcon returns a module-cached stable component per name, not one created during render */}