feat: integrate sonner as brain entry by marcjulian · Pull Request #1252 · spartan-ng/spartan

marcjulian · 2026-03-05T11:15:00Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our
guidelines: https://github.com/spartan-ng/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Which package are you modifying?

Primitives

Others

trpc
nx
repo
cli

What is the current behavior?

Closes #1240

What is the new behavior?

added @spartan-ng/brain/sonner entry which replaces ngx-sonner
fix vanishing of custom icons
fix expand toasts groups by position
update modifiers

Does this PR introduce a breaking change?

Yes
No

@spartan-ng/brain/sonner entry which replaces ngx-sonner

Added healthcheck and migration to replace ngx-sonner with @spartan-ng/brain/sonner for the toast call.
Requires two steps:

regenerate sonner helm
run healthcheck

Other information

…sts grouped by position

…UMENT for window access * remove dir prop

greptile-apps · 2026-03-05T13:59:20Z

Greptile Summary

This PR replaces the third-party ngx-sonner package with a first-party @spartan-ng/brain/sonner entry, giving the spartan-ng team full control over the toast implementation. The brain library ports the Sonner API to Angular signals (linkedSignal, afterRenderEffect, viewChild), adds multi-position support, fixes custom icon rendering, and ships a CLI migration generator + healthcheck to help consumers upgrade. The helm layer (hlm-toaster) is regenerated to import from the new brain entry.

Key changes and issues found:

_expanded resets on every toast change (brn-toaster.ts:169) — linkedSignal's computation re-runs and overwrites the user's manual .set(true) call whenever any toast is added or dismissed. A user hovering over an expanded stack will see it collapse to the stacked view the moment any toast auto-dismisses, even while their mouse is stationary over the container.
--front-toast-height uses unfiltered global heights (brn-toaster.ts:189) — this._heights is the unfiltered toastState.heights signal. With multiple position groups active, this._heights()[0] picks up the height of whichever toast is first globally (regardless of position), and the same value is written to all <ol> elements, producing incorrect entrance/exit animations for all but the first position group.
_toasts filter excludes default-position toasts (brn-toast.ts:165) — previously flagged; the t.position === this.position() guard filters out toasts with position: undefined, making --z-index evaluate to a negative value for the common case of toasts created without an explicit position. The ToastFilterPipe handles this correctly with (!toast.position && index === 0) || toast.position === position — the same logic should be applied here.
The CLI generator and healthcheck look correct; previously reported issues (replace → replaceAll, JSDoc copy-paste, filename typo) have all been addressed.

Confidence Score: 3/5

The PR is functional for the common single-position use-case but contains two logic bugs that affect multi-position setups and the expanded hover UX.
The core toast() API, state management, and migration tooling are solid. However, the linkedSignal reset bug in _expanded will cause a visible UX regression (collapsing on auto-dismiss while hovering), and the unfiltered --front-toast-height will cause animation glitches for multi-position setups. Combined with the previously flagged _toasts filter issue that remains unresolved, three independent logic bugs are present in the core component files.
libs/brain/sonner/src/lib/brn-toaster.ts and libs/brain/sonner/src/lib/brn-toast.ts need attention before merging.

Important Files Changed

Filename	Overview
libs/brain/sonner/src/lib/brn-toaster.ts	Core toaster host component. Two logic bugs found: `linkedSignal` for `_expanded` resets on every toast change (collapsing the hover-expanded view when a toast auto-dismisses), and `--front-toast-height` reads from the global unfiltered heights array, giving wrong values for all but the first position group.
libs/brain/sonner/src/lib/brn-toast.ts	Individual toast component. The `_toasts` computed filter (`t.position === this.position()`) excludes default-position toasts (`position: undefined`), causing `--z-index` to be negative for the most common use-case where toasts are created without an explicit position. Noted in previous review threads.
libs/brain/sonner/src/lib/state.ts	Global toast state as a module-level singleton. Correctly implements create/dismiss/update semantics, promise toasts, and height tracking with sorted insertion.
libs/cli/src/generators/migrate-sonner/generator.ts	Migration generator correctly uses `replaceAll` to replace all `ngx-sonner` references and properly skips `hlm-toaster.ts`. Previously flagged issues (JSDoc, `replace` vs `replaceAll`) have been addressed.
libs/cli/src/generators/healthcheck/healthchecks/sonner.ts	Healthcheck correctly detects remaining `ngx-sonner` references and delegates the fix to `migrateSonnerGenerator`. Previously flagged typos have been corrected.
libs/brain/sonner/src/lib/brn-toaster.spec.ts	Good test coverage for core toast interactions, timer behavior, and custom icons. Tests are clear and well-structured. Missing coverage for multi-position scenarios and the `_toasts` position filter edge case.
libs/helm/sonner/src/lib/hlm-toaster.ts	Helm wrapper correctly migrated to use `@spartan-ng/brain/sonner`. Provides custom icon slots and default CSS variable bindings for themed toasts.
libs/brain/sonner/src/lib/pipes/toast-filter.pipe.ts	Correctly handles default-position toasts (`!toast.position && index === 0`) and explicit-position toasts, assigning them to the right `<ol>` group. This logic is unfortunately not mirrored in `brn-toast.ts`'s own `_toasts` computed.

Sequence Diagram

sequenceDiagram
    participant App as Application Code
    participant State as toastState (singleton)
    participant Toaster as BrnSonnerToaster
    participant Toast as BrnSonnerToast
    participant Pipe as ToastFilterPipe

    App->>State: toast('message') / toast.success() / toast.promise()
    State->>State: create() — adds ToastT to toasts signal

    State-->>Toaster: toasts signal changes
    Toaster->>Toaster: _possiblePositions() recomputed
    Toaster->>Pipe: _toasts() | toastFilter: $index : pos
    Pipe-->>Toaster: filtered ToastT[] per position group

    Toaster->>Toast: renders <brn-sonner-toast [position]="pos" ...>
    Toast->>Toast: ngAfterViewInit() — measures height
    Toast->>State: addHeight({ toastId, height, position })
    State-->>Toaster: heights signal changes → _toasterStyles() recomputed

    Note over Toast: afterRenderEffect — startTimer()
    Toast->>Toast: startTimer() — schedules deleteToast()
    Toast->>State: dismiss(id) after duration
    State-->>Toaster: toasts signal changes → _expanded recomputed

    Note over App,Toaster: User hover / keyboard
    App->>Toaster: mouseenter / mousemove → _expanded.set(true)
    App->>Toaster: Alt+T hotkey → _expanded.set(true), listRef.focus()
    App->>Toaster: Escape → _expanded.set(false)
    App->>Toast: pointerdown/move/up → swipe-to-dismiss

_{Last reviewed commit: 6e8bdc7}

libs/cli/src/generators/healthcheck/healthchecks/sonner.ts

greptile-apps · 2026-03-05T13:59:25Z

libs/cli/src/generators/migrate-sonner/generator.ts

+		}
+
+		if (content.includes('ngx-sonner')) {
+			content = content.replace('ngx-sonner', '@spartan-ng/brain/sonner');


String.prototype.replace with a plain string only replaces the first occurrence. A file with both a value import and a type import from ngx-sonner would only have the first one migrated, leaving a broken import behind.

Use replaceAll to ensure all occurrences are replaced:

Suggested change

content = content.replace('ngx-sonner', '@spartan-ng/brain/sonner');

content = content.replaceAll('ngx-sonner', '@spartan-ng/brain/sonner');

libs/cli/src/generators/migrate-sonner/generator.ts

greptile-apps · 2026-03-05T13:59:27Z

libs/cli/src/generators/migrate-sonner/generator.spec.ts

+		});
+	});
+
+	it('should add HlmContextMenuImports import (Standalone)', async () => {


The test description is a copy-paste from a different generator and does not describe what this test verifies. It should reflect the actual behavior being tested.

Suggested change

it('should add HlmContextMenuImports import (Standalone)', async () => {

it('should replace ngx-sonner imports with @spartan-ng/brain/sonner', async () => {

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

libs/cli/src/generators/healthcheck/healthchecks/sonner.ts

greptile-apps · 2026-03-05T13:59:29Z

libs/brain/sonner/src/lib/brn-toaster.ts

+	/** internal **/
+	public readonly direction = this._dir.valueSignal;
+
+	private readonly _listRef = viewChild<ElementRef<HTMLOListElement>>('listRef');


The template uses @for (pos of _possiblePositions()) to render multiple <ol #listRef> elements, one per position. However, viewChild always resolves to the first matching element in template order. When toasts appear at multiple positions, the keyboard handlers (Alt+T hotkey and Escape key) will only affect the first position group.

Consider using viewChildren instead to handle all position groups:

private readonly _listRefs = viewChildren<ElementRef<HTMLOListElement>>('listRef');

Then in handleKeydown, iterate over all lists or focus the most relevant one based on current context.

marcjulian · 2026-03-05T14:02:01Z

@greptileai review again

libs/brain/sonner/src/lib/brn-toast.ts

libs/brain/sonner/src/lib/brn-toaster.ts

libs/cli/src/generators/migrate-sonner/generator.ts

libs/brain/sonner/src/lib/brn-toaster.ts

marcjulian · 2026-03-05T19:11:30Z

@greptileai review again

libs/brain/sonner/src/lib/brn-toaster.ts

Dafnik · 2026-03-05T22:22:58Z

libs/brain/sonner/src/lib/internal/cn.ts

@@ -0,0 +1,3 @@
+export function cn(...classes: (string | undefined)[]) {


Is there a reason for not using the hlm function?

I copied over the code from ngx-sonner, which has this cn function. I also thought about using the helm version, but the brain package cannot rely on helm. Helm should consume the brain only. But the brain has a deps on clsx which we could use instead of this custom function. What do you think?

Ohh I missed that the hlm function is only available in hlm but that totally makes sense. My bad!

Using clsx directly is a great idea when its already available.

But if not I guess its also fine. I just wanted to make sure that this didn't slip your eyes when doing the integration.

No worries, I totally had the same thought as you to use hlm there! Thanks for the review :) I will go with clsx here!

MerlinMoos

LGTM thanks for moving that to our repo

marcjulian · 2026-03-09T09:38:27Z

libs/helm/sonner/src/lib/hlm-toaster.ts

-		/>
+		>
+			<ng-template #loadingIcon>
+				<ng-icon name="lucideLoader2" class="overflow-visible! text-base [&>svg]:motion-safe:animate-spin" />


@ashley-hunter the sonner styles moves the svg to the left or right, but because ng-icon has overflow hidden it looks like its cut of. I set overflow to visible. Do you think that is fine in this case?

marcjulian · 2026-03-09T13:51:33Z

@MerlinMoos we reference ngx-sonner on the sonner page and about page. Should we keep it or should we update the text?

MerlinMoos · 2026-03-09T13:55:23Z

@MerlinMoos we reference ngx-sonner on the sonner page and about page. Should we keep it or should we update the text?

I would say we can remove it, now we are maintaining that. We can keep it simple and remove the reference to the ngx-sonner

marcjulian added 11 commits March 5, 2026 10:50

feat(sonner): add brain sonner entry

e5d85aa

feat(sonner): remove ngx-sonner and use @spartan-ng/brain/sonner

87019e1

docs(sonner): add brain api section

ea400e2

fix(sonner): vanishing of custom icons

745a1e7

feat(sonner): add toaster spec and update snapshot

e11e6be

fix(sonner): filter toasts/heights by position in toast to expand toa…

b7f6421

…sts grouped by position

feat(sonner): add sonner toaster config

e1904b5

feat(sonner): use Directionality for dir, use afterNextRender and DOC…

b3b6433

…UMENT for window access * remove dir prop

feat(sonner): update modifiers and type imports

05c8539

feat(sonner): update snapshot

612ffef

feat(sonner): add sonner healthcheck and migration to migrate ngx-sonner

4133a9f

marcjulian marked this pull request as ready for review March 5, 2026 13:53

marcjulian requested review from ashley-hunter and goetzrobin March 5, 2026 13:53

greptile-apps bot reviewed Mar 5, 2026

View reviewed changes

marcjulian added 2 commits March 5, 2026 15:00

fix(sonner): update healthcheck guard

3b68cd6

feat(sonner): copy to helm

d6e2fb5

marcjulian added 4 commits March 5, 2026 15:02

fix(sonner): update comment

42080b2

fix(sonner): replace comment

c755159

fix(sonner): update migration test message

9e0c443

feat(sonner): replace all in migration

bc7ed2a

greptile-apps bot reviewed Mar 5, 2026

View reviewed changes

feat(sonner): use injected document, update comments

6e8bdc7

greptile-apps bot reviewed Mar 5, 2026

View reviewed changes

libs/brain/sonner/src/lib/brn-toaster.ts Show resolved Hide resolved

libs/brain/sonner/src/lib/brn-toaster.ts Show resolved Hide resolved

Dafnik reviewed Mar 5, 2026

View reviewed changes

marcjulian mentioned this pull request Mar 6, 2026

feat(sonner): add custom icons #1242

Closed

75 tasks

feat(sonner): use clsx

58d2f7e

feat(sonner): use computed for classes

e22c6e0

marcjulian requested a review from MerlinMoos March 9, 2026 09:20

MerlinMoos approved these changes Mar 9, 2026

View reviewed changes

marcjulian commented Mar 9, 2026

View reviewed changes

docs(sonner): update about

78ced54

	content = content.replace('ngx-sonner', '@spartan-ng/brain/sonner');
	content = content.replaceAll('ngx-sonner', '@spartan-ng/brain/sonner');

	it('should add HlmContextMenuImports import (Standalone)', async () => {
	it('should replace ngx-sonner imports with @spartan-ng/brain/sonner', async () => {

		@@ -0,0 +1,3 @@
		export function cn(...classes: (string \| undefined)[]) {

Conversation

marcjulian commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

Which package are you modifying?

Primitives

Others

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

Uh oh!

greptile-apps bot commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

greptile-apps bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

marcjulian commented Mar 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marcjulian commented Mar 5, 2026

Uh oh!

Uh oh!

Uh oh!

Dafnik Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

marcjulian Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Dafnik Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

marcjulian Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

MerlinMoos left a comment

Choose a reason for hiding this comment

Uh oh!

marcjulian Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

marcjulian commented Mar 9, 2026

Uh oh!

MerlinMoos commented Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marcjulian commented Mar 5, 2026 •

edited

Loading

greptile-apps bot commented Mar 5, 2026 •

edited

Loading

marcjulian Mar 6, 2026 •

edited

Loading