fix(notifications): the spec icon is read instead of stored and ignored (#3014 follow-up) - #3076
Merged
Merged
Conversation
…ored (#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 (`presentNotificationToast`, now .tsx so it can build the icon element), so the override behaves identically on all five. The fallback is the interesting part. `getLazyIcon` degrades an unknown name to a `Database` glyph — right for a data-shaped schema slot, wrong here, where it would swap a meaningful icon for a meaningless one on an error notification. So the name is checked first via a new `isLucideIconName` export, and a typo costs the author their override and nothing more. The two `react-hooks/static-components` disables follow the existing repo convention for this rule (MetricCard / MetricWidget / NavigationRenderer): the factory is module-cached per name, so the component identity is stable and the rule is a false positive here. Verified in the running console: a toast declaring `icon: 'rocket'` renders the rocket instead of the success checkmark, while a snackbar declaring `icon: 'not-a-real-icon'` renders the info icon — not a Database glyph. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
github-actions
Bot
added
documentation
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…instead of forked or ignored (#3014 follow-up) (#3085) 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: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #3071 / #3075. Closes the gap I flagged when wiring the console:
NotificationSchema.icon— the spec's "Icon name override" — reachedNotificationItemand stopped there. Every surface drew the severity icon, so an author writingicon: 'rocket'got the success checkmark. Same shape as thedisplayTypecollapse #3071 fixed: a value that validates, is carried, and renders nothing.The fix
All five presentations resolve
iconthrough 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 (presentNotificationToast, now.tsxso it can build the icon element), so the override behaves identically on a toast, a banner, a snackbar, an alert and an inline message.The fallback is the part worth reviewing.
getLazyIcondegrades an unknown name to aDatabaseglyph — the right default for a data-shaped schema slot, and the wrong one here: on an error notification it swaps a meaningful icon for a meaningless one. So the name is checked first, via a newisLucideIconNameexport onlib/lazy-icon, and a typo costs the author their override and nothing more.The two
react-hooks/static-componentsdisables follow the repo's existing convention for this rule (MetricCard,MetricWidget,NavigationRenderer):getLazyIconis module-cached per name, so the component identity is stable across renders and the rule is a false positive here. Only the two direct component-body call sites trip it; the banner/inline ones sit inside.mapcallbacks and are not flagged, so they carry no disable rather than an unused one.Verification
In the running console — a toast declaring
icon: 'rocket'renders the rocket instead of the green success checkmark, and, side by side, a snackbar declaringicon: 'not-a-real-icon'renders the plain info icon rather than a stray database glyph. Both fallback directions proven in one frame; no console errors.Suites:
packages/components+packages/app-shell— 2478 tests, 301 files, all green (9 new: 4 pinning the resolver contract, 1 DOM smoke on a surface, 4 on the toast mapping).tsc --noEmitclean on both; eslint 0 errors on every touched file.Docs: the notifications guide gains an
iconsection, plus a line in the@object-ui/componentsREADME.Refs #3014, #2942.
🤖 Generated with Claude Code