From f32d0b23c62e1a3a23a70db804e1b2c20fa43d6d Mon Sep 17 00:00:00 2001 From: Artem Belik Date: Wed, 2 Sep 2026 19:05:53 +0300 Subject: [PATCH 1/2] fix(splitter)!: errors following a full review of the component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every input on the splitter and its gutter was an accessor with coercion in the setter, which is why the automated signal migration skipped all thirteen. They are signal inputs now, with `booleanAttribute` and `numberAttribute` doing the work. `KbqGutterGhostDirective` went the other way. Its `visible`, `x`, `y`, `direction` and `size` were `@Input()` in name only: the splitter renders `` with no bindings and drives them imperatively during a drag, outside the Angular zone. They are plain properties now. Three behavior fixes the review uncovered: - A `gutterSize` that is not a positive number falls back to the default 6. The old setter read its own getter, so an invalid value silently preserved the last valid one. - The gutter lays itself out reactively instead of once in `ngOnInit`, so changing `direction` after init re-applies the layout — and clears the dimension the other direction owns, which used to stay behind as a stale width or height. - A splitter area unsubscribes from `gutterPositionChange` when it is destroyed. An area removed from a long-lived splitter used to keep emitting `sizeChange` for every later drag. `resizing` is gone: nothing ever set it, so it always reported `false`. BREAKING CHANGE: every `KbqSplitterComponent` and `KbqGutterDirective` input is a signal, `resizing` is removed, `KbqGutterGhostDirective` declares no inputs, and the layout bookkeeping is private. A valueless `disabled`, `hideGutters` or `useGhost` attribute now means true. Reported and partly rewritten by the `splitter-signals` schematic. Co-Authored-By: Claude Opus 5 --- docs/guides/migration.en.md | 26 +- docs/guides/migration.ru.md | 26 +- .../splitter/splitter.component.html | 10 +- .../components/splitter/splitter.component.ts | 419 +++++++--------- packages/components/splitter/splitter.spec.ts | 95 +++- packages/schematics/src/collection.json | 5 + packages/schematics/src/migrations.json | 5 + .../src/migrations/splitter-signals/README.md | 58 +++ .../src/migrations/splitter-signals/data.ts | 107 ++++ .../migrations/splitter-signals/index.spec.ts | 303 ++++++++++++ .../src/migrations/splitter-signals/index.ts | 463 ++++++++++++++++++ .../migrations/splitter-signals/schema.json | 20 + .../src/migrations/splitter-signals/schema.ts | 6 + .../components/splitter.api.md | 131 ++--- 14 files changed, 1331 insertions(+), 343 deletions(-) create mode 100644 packages/schematics/src/migrations/splitter-signals/README.md create mode 100644 packages/schematics/src/migrations/splitter-signals/data.ts create mode 100644 packages/schematics/src/migrations/splitter-signals/index.spec.ts create mode 100644 packages/schematics/src/migrations/splitter-signals/index.ts create mode 100644 packages/schematics/src/migrations/splitter-signals/schema.json create mode 100644 packages/schematics/src/migrations/splitter-signals/schema.ts diff --git a/docs/guides/migration.en.md b/docs/guides/migration.en.md index 76f2bbaa45..fb80530dd2 100644 --- a/docs/guides/migration.en.md +++ b/docs/guides/migration.en.md @@ -1035,7 +1035,7 @@ for each option it deselected and reporting the shortened value to the form cont ### 18. Component review (20.3.0) -Ten components went through a full review in 20.3.0: notification-center, popover, search-expandable, select, split-button, title, toast, tooltip, tree and tree-select. Each review closed the members that were never part of the component's contract, moved inputs to signals where that was the point of it, and fixed the behavior it uncovered along the way. Only the changes that reach a consumer are listed here. +Components went through a full review in 20.3.0, in two waves. The first covered notification-center, popover, search-expandable, select, split-button, title, toast, tooltip, tree and tree-select; the second is the one each subsection below belongs to. Each review closed the members that were never part of the component's contract, moved inputs to signals where that was the point of it, and fixed the behavior it uncovered along the way. Only the changes that reach a consumer are listed here. Every schematic named below runs automatically: @@ -1093,6 +1093,30 @@ A `` with no projected button no longer throws outside dev mod Reported by `split-button-optional-disabled`. +#### Splitter + +Every input on the splitter and its gutter was an accessor with coercion in the setter, which is why the automated signal migration skipped all thirteen. They are signal inputs now, with `booleanAttribute` and `numberAttribute` doing the coercion. + +`KbqGutterGhostDirective` went the other way. Its `visible`, `x`, `y`, `direction` and `size` were `@Input()` in name only: the splitter renders `` with no bindings and drives them imperatively during a drag, outside the Angular zone. They are plain properties now, so a template binding on them stops compiling — there was never a supported way to place the ghost yourself. + +| Pattern | Manual migration | +| ------------------------------------------------------------------------------------- | ------------------------------------------------------------ | +| `.hideGutters` / `.direction` / `.disabled` / `.useGhost` / `.gutterSize` | Read as calls — rewritten for you | +| `.isDragging` / `.isVertical` | Read as calls — rewritten for you | +| `gutter.direction` / `.order` / `.size` / `.isVertical` / `.dragged` | Read as calls — rewritten for you | +| `.resizing` | Removed — it was dead and always `false`; use `isDragging()` | +| `.elementRef` / `.changeDetectorRef` / `.areas` / `.areaRefs` / `.gutters` / `.ghost` | Closed layout bookkeeping | + +**`hideGutters`, `disabled` and `useGhost` are `booleanAttribute` inputs now.** A valueless attribute means `true`; `coerceBooleanProperty` treated the empty string as `false`. + +**A `gutterSize` that is not a positive number falls back to the default 6** instead of keeping whatever the previous value happened to be. The old setter read its own getter, so an invalid value silently preserved the last valid one. + +**The gutter lays itself out reactively** instead of once in `ngOnInit`, so changing `direction` after init re-applies the layout — and clears the dimension the other direction owns, which used to stay behind as a stale `width` or `height`. + +**A splitter area unsubscribes from `gutterPositionChange` when it is destroyed.** An area removed from a long-lived splitter used to keep emitting `sizeChange` for every later drag. + +Handled by `splitter-signals`: the reads and the `dragged` write are rewritten, the rest is reported. + #### Title `kbq-title` measures its host and opens a tooltip when the text is truncated. The review kept that surface — the `kbq-title` input and the tooltip it opens — and closed the measurement machinery behind it. diff --git a/docs/guides/migration.ru.md b/docs/guides/migration.ru.md index a7d545361d..f11668cbc3 100644 --- a/docs/guides/migration.ru.md +++ b/docs/guides/migration.ru.md @@ -1039,7 +1039,7 @@ ng g @koobiq/components:list-tree-multiple-input --project ### 18. Ревью компонентов (20.3.0) -В 20.3.0 полное ревью прошли десять компонентов: notification-center, popover, search-expandable, select, split-button, title, toast, tooltip, tree и tree-select. Каждое ревью закрывало члены, которые никогда не были частью контракта компонента, переводило входы на сигналы там, где в этом и был его смысл, и попутно исправляло найденные ошибки поведения. Ниже перечислено только то, что доходит до потребителя. +В 20.3.0 полное ревью прошло в две волны. В первую вошли notification-center, popover, search-expandable, select, split-button, title, toast, tooltip, tree и tree-select; ко второй относится каждый из подразделов ниже. Каждое ревью закрывало члены, которые никогда не были частью контракта компонента, переводило входы на сигналы там, где в этом и был его смысл, и попутно исправляло найденные ошибки поведения. Ниже перечислено только то, что доходит до потребителя. Все схематики, названные ниже, запускаются автоматически: @@ -1097,6 +1097,30 @@ if (splitButton.disabled === false) { Сообщает `split-button-optional-disabled`. +#### Splitter + +Все входы сплиттера и его разделителя были геттерами и сеттерами с приведением типов внутри, поэтому автоматическая миграция на сигналы пропустила все тринадцать. Теперь это сигнальные входы, а приведением занимаются `booleanAttribute` и `numberAttribute`. + +С `KbqGutterGhostDirective` произошло обратное. Его `visible`, `x`, `y`, `direction` и `size` были `@Input()` только по названию: сплиттер отрисовывает `` вообще без привязок и задаёт их императивно во время перетаскивания, вне зоны Angular. Теперь это обычные свойства, поэтому привязка в шаблоне перестанет компилироваться — поддерживаемого способа расположить «призрак» самому никогда и не было. + +| Что было | Как мигрировать вручную | +| ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------ | +| `.hideGutters` / `.direction` / `.disabled` / `.useGhost` / `.gutterSize` | Читать как вызовы — переписывается за вас | +| `.isDragging` / `.isVertical` | Читать как вызовы — переписывается за вас | +| `gutter.direction` / `.order` / `.size` / `.isVertical` / `.dragged` | Читать как вызовы — переписывается за вас | +| `.resizing` | Удалён — он был мёртвым и всегда возвращал `false`; используйте `isDragging()` | +| `.elementRef` / `.changeDetectorRef` / `.areas` / `.areaRefs` / `.gutters` / `.ghost` | Закрытая внутренняя кухня раскладки | + +**`hideGutters`, `disabled` и `useGhost` стали `booleanAttribute`-входами.** Атрибут без значения теперь означает `true`; `coerceBooleanProperty` считал пустую строку ложью. + +**`gutterSize`, который не является положительным числом, откатывается к значению по умолчанию — 6**, а не сохраняет предыдущее значение. Старый сеттер читал собственный геттер, поэтому некорректное значение молча оставляло последнее корректное. + +**Разделитель пересчитывает свою раскладку реактивно**, а не один раз в `ngOnInit`, поэтому смена `direction` после инициализации теперь применяется — и очищает размер, принадлежащий другому направлению, который раньше оставался залипшим `width` или `height`. + +**Область сплиттера отписывается от `gutterPositionChange` при уничтожении.** Область, удалённая из долгоживущего сплиттера, раньше продолжала отправлять `sizeChange` при каждом последующем перетаскивании. + +Закрывается схематиком `splitter-signals`: чтения и запись в `dragged` переписываются, остальное сообщается в отчёте. + #### Title `kbq-title` измеряет свой хост и открывает тултип, когда текст обрезан. Ревью сохранило эту поверхность — вход `kbq-title` и открываемый тултип — и закрыло стоящий за ней механизм измерения. diff --git a/packages/components/splitter/splitter.component.html b/packages/components/splitter/splitter.component.html index ff8c86be4b..8a11087583 100644 --- a/packages/components/splitter/splitter.component.html +++ b/packages/components/splitter/splitter.component.html @@ -3,16 +3,16 @@ @for (area of areas; track area) { @if (!$last) { } } -@if (useGhost) { +@if (useGhost()) { } diff --git a/packages/components/splitter/splitter.component.ts b/packages/components/splitter/splitter.component.ts index f56fba231c..4e1c46af1d 100644 --- a/packages/components/splitter/splitter.component.ts +++ b/packages/components/splitter/splitter.component.ts @@ -1,28 +1,33 @@ -import { coerceBooleanProperty, coerceCssPixelValue, coerceNumberProperty } from '@angular/cdk/coercion'; +import { coerceCssPixelValue } from '@angular/cdk/coercion'; import { Platform } from '@angular/cdk/platform'; import { AfterContentInit, AfterViewInit, + booleanAttribute, ChangeDetectionStrategy, ChangeDetectorRef, Component, + computed, ContentChildren, Directive, - ElementRef, - Input, + effect, + forwardRef, + inject, + input, NgZone, + numberAttribute, OnDestroy, - OnInit, + output, + OutputRefSubscription, QueryList, Renderer2, - ViewEncapsulation, - forwardRef, - inject, - output, + Signal, + signal, viewChild, - viewChildren + viewChildren, + ViewEncapsulation } from '@angular/core'; -import { KBQ_WINDOW } from '@koobiq/components/core'; +import { KBQ_WINDOW, kbqInjectNativeElement } from '@koobiq/components/core'; import { Subscription } from 'rxjs'; interface IArea { @@ -54,94 +59,99 @@ const enum StyleProperty { Cursor = 'cursor' } +/** Width or height of a gutter, in pixels. */ +const DEFAULT_GUTTER_SIZE = 6; + +/** Coerces a gutter size, falling back to the default for anything that is not a positive number. */ +const gutterSizeAttribute = (value: unknown): number => { + const size = numberAttribute(value, DEFAULT_GUTTER_SIZE); + + return size > 0 ? size : DEFAULT_GUTTER_SIZE; +}; + +/** Axis the splitter lays its areas out along. */ export enum Direction { Horizontal = 'horizontal', Vertical = 'vertical' } +/** + * Draggable divider rendered by the splitter between two areas. + * + * @docs-private + */ @Directive({ selector: 'kbq-gutter', host: { class: 'kbq-gutter', - '[class.kbq-gutter_vertical]': 'isVertical', - '[class.kbq-gutter_dragged]': 'dragged', - '(mousedown)': 'dragged = true' + '[class.kbq-gutter_vertical]': 'isVertical()', + '[class.kbq-gutter_dragged]': 'dragged()', + '(mousedown)': 'dragged.set(true)' } }) -export class KbqGutterDirective implements OnInit { - private elementRef = inject>(ElementRef); - private renderer = inject(Renderer2); - - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() - get direction(): Direction { - return this._direction; - } - - set direction(direction: Direction) { - this._direction = direction; - } - - private _direction: Direction = Direction.Vertical; - - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() - get order(): number { - return this._order; - } - - set order(order: number) { - this._order = coerceNumberProperty(order); - } - - private _order: number = 0; - - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() - get size(): number { - return this._size; - } - - set size(size: number) { - this._size = coerceNumberProperty(size); - } - - private _size: number = 6; - - get isVertical(): boolean { - return this._direction === Direction.Vertical; - } - - dragged: boolean = false; - - ngOnInit(): void { - this.setStyle(StyleProperty.FlexBasis, coerceCssPixelValue(this.size)); - this.setStyle(this.isVertical ? StyleProperty.Height : StyleProperty.Width, coerceCssPixelValue(this.size)); - this.setStyle(StyleProperty.Order, this.order); - - if (!this.isVertical) { - this.setStyle(StyleProperty.Height, '100%'); - } - - // fix IE issue with gutter icon. flex-direction is required for flex alignment options - this.setStyle(StyleProperty.FlexDirection, this.isVertical ? 'row' : 'column'); +export class KbqGutterDirective { + private readonly nativeElement = kbqInjectNativeElement(); + private readonly renderer = inject(Renderer2); + + /** Axis the gutter divides. */ + readonly direction = input(Direction.Vertical); + + /** Flex order of the gutter among the areas. */ + readonly order = input(0, { transform: numberAttribute }); + + /** Thickness of the gutter, in pixels. */ + readonly size = input(DEFAULT_GUTTER_SIZE, { transform: gutterSizeAttribute }); + + /** Whether the gutter divides a vertical stack. */ + readonly isVertical = computed(() => this.direction() === Direction.Vertical); + + /** Whether the gutter is currently held down. */ + readonly dragged = signal(false); + + constructor() { + // The gutters are rendered inside an `@for` and keep their instance across reorders, so the layout + // has to follow the inputs rather than run once on init. + effect(() => { + const size = this.size(); + const isVertical = this.isVertical(); + + this.setStyle(StyleProperty.FlexBasis, coerceCssPixelValue(size)); + this.setStyle(StyleProperty.Order, this.order()); + + // fix IE issue with gutter icon. flex-direction is required for flex alignment options + this.setStyle(StyleProperty.FlexDirection, isVertical ? 'row' : 'column'); + + // Clear the dimension the other direction owns: the layout runs again when `direction` changes, + // and a leftover width would keep a vertical gutter as wide as it was while horizontal. + if (isVertical) { + this.renderer.removeStyle(this.nativeElement, StyleProperty.Width); + this.setStyle(StyleProperty.Height, coerceCssPixelValue(size)); + } else { + this.setStyle(StyleProperty.Width, coerceCssPixelValue(size)); + this.setStyle(StyleProperty.Height, '100%'); + } + }); } + /** Offset of the gutter within the splitter. */ getPosition(): IPoint { return { - x: this.elementRef.nativeElement.offsetLeft, - y: this.elementRef.nativeElement.offsetTop + x: this.nativeElement.offsetLeft, + y: this.nativeElement.offsetTop }; } private setStyle(property: StyleProperty, value: string | number): void { - this.renderer.setStyle(this.elementRef.nativeElement, property, value); + this.renderer.setStyle(this.nativeElement, property, value); } } +/** + * Placeholder the splitter drags in place of the gutter while `useGhost` is on. It is rendered by the + * splitter with no bindings and driven entirely from `KbqSplitterComponent`. + * + * @docs-private + */ @Directive({ selector: 'kbq-gutter-ghost', host: { @@ -151,16 +161,11 @@ export class KbqGutterDirective implements OnInit { } }) export class KbqGutterGhostDirective { - private elementRef = inject>(ElementRef); - private renderer = inject(Renderer2); + private readonly nativeElement = kbqInjectNativeElement(); + private readonly renderer = inject(Renderer2); - // TODO: Skipped for migration because: - // Your application code writes to the input. This prevents migration. - @Input() visible: boolean; + visible: boolean = false; - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() get x(): number { return this._x; } @@ -172,9 +177,6 @@ export class KbqGutterGhostDirective { private _x: number = 0; - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() get y(): number { return this._y; } @@ -186,9 +188,6 @@ export class KbqGutterGhostDirective { private _y: number = 0; - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() get direction(): Direction { return this._direction; } @@ -200,19 +199,16 @@ export class KbqGutterGhostDirective { private _direction: Direction = Direction.Vertical; - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() get size(): number { return this._size; } set size(size: number) { - this._size = coerceNumberProperty(size); + this._size = gutterSizeAttribute(size); this.updateDimensions(); } - private _size: number = 6; + private _size: number = DEFAULT_GUTTER_SIZE; get isVertical(): boolean { return this.direction === Direction.Vertical; @@ -224,10 +220,11 @@ export class KbqGutterGhostDirective { } private setStyle(property: StyleProperty, value: string | number): void { - this.renderer.setStyle(this.elementRef.nativeElement, property, value); + this.renderer.setStyle(this.nativeElement, property, value); } } +/** Component that lays out resizable areas separated by draggable gutters. */ @Component({ selector: 'kbq-splitter', imports: [KbqGutterDirective, KbqGutterGhostDirective], @@ -241,130 +238,76 @@ export class KbqGutterGhostDirective { exportAs: 'kbqSplitter', preserveWhitespaces: false }) -export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy { - elementRef = inject>(ElementRef); - changeDetectorRef = inject(ChangeDetectorRef); - private ngZone = inject(NgZone); - private renderer = inject(Renderer2); +export class KbqSplitterComponent implements AfterContentInit, OnDestroy { + private readonly nativeElement = kbqInjectNativeElement(); + private readonly changeDetectorRef = inject(ChangeDetectorRef); + private readonly ngZone = inject(NgZone); + private readonly renderer = inject(Renderer2); - readonly gutterPositionChange = output(); + private readonly gutters = viewChildren(KbqGutterDirective); + private readonly ghost = viewChild.required(KbqGutterGhostDirective); - areas: IArea[] = []; + @ContentChildren(forwardRef(() => KbqSplitterAreaDirective)) + private areaRefs: QueryList; - readonly gutters = viewChildren(KbqGutterDirective); - readonly ghost = viewChild.required(KbqGutterGhostDirective); - - @ContentChildren(forwardRef(() => KbqSplitterAreaDirective)) areaRefs: QueryList; - - get isDragging(): boolean { - return this._isDragging; - } - private _isDragging: boolean = false; + private readonly dragging = signal(false); private readonly areaPositionDivider: number = 2; private readonly listeners: (() => void)[] = []; private areasChangeSubscription: Subscription = Subscription.EMPTY; - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() - get hideGutters(): boolean { - return this._hideGutters; - } - - set hideGutters(value: boolean) { - this._hideGutters = coerceBooleanProperty(value); - } - - private _hideGutters: boolean = false; - - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() - get direction(): Direction { - return this._direction; - } - - set direction(direction: Direction) { - this._direction = direction; - } - - private _direction: Direction; - - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() - get disabled(): boolean { - return this._disabled; - } - - set disabled(disabled: boolean) { - this._disabled = coerceBooleanProperty(disabled); - } + /** Emitted once a gutter drag has finished. */ + readonly gutterPositionChange = output(); - private _disabled: boolean = false; + /** Whether the gutters are hidden. The areas stay resizable. */ + readonly hideGutters = input(false, { transform: booleanAttribute }); - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() - get useGhost(): boolean { - return this._useGhost; - } + /** Axis the areas are laid out along. */ + readonly direction = input(Direction.Horizontal); - set useGhost(useGhost: boolean) { - this._useGhost = coerceBooleanProperty(useGhost); - } + /** Whether dragging is disabled. */ + readonly disabled = input(false, { transform: booleanAttribute }); - private _useGhost: boolean = false; + /** Whether a drag moves a ghost divider and applies the new sizes on release. */ + readonly useGhost = input(false, { transform: booleanAttribute }); - // TODO: Skipped for migration because: - // Accessor inputs cannot be migrated as they are too complex. - @Input() - get gutterSize(): number { - return this._gutterSize; - } + /** Thickness of a gutter, in pixels. Anything that is not a positive number falls back to the default. */ + readonly gutterSize = input(DEFAULT_GUTTER_SIZE, { transform: gutterSizeAttribute }); - set gutterSize(gutterSize: number) { - const size = coerceNumberProperty(gutterSize); + /** Whether a gutter is currently being dragged. */ + readonly isDragging: Signal = this.dragging.asReadonly(); - this._gutterSize = size > 0 ? size : this.gutterSize; - } + /** Whether the areas are stacked vertically. */ + readonly isVertical = computed(() => this.direction() === Direction.Vertical); - private _gutterSize: number = 6; + /** @docs-private */ + protected areas: IArea[] = []; - get resizing(): boolean { - return this._resizing; + constructor() { + effect(() => this.setStyle(StyleProperty.FlexDirection, this.isVertical() ? 'column' : 'row')); } - private _resizing: boolean = false; - + /** @docs-private */ addArea(area: KbqSplitterAreaDirective): void { this.areas.push(this.mapAndOrderArea(area, this.areas.length)); this.changeDetectorRef.detectChanges(); } - ngOnInit(): void { - if (!this.direction) { - this.direction = Direction.Horizontal; - } - - this.setStyle(StyleProperty.FlexDirection, this.isVertical() ? 'column' : 'row'); - } - - ngAfterContentInit() { + ngAfterContentInit(): void { this.areasChangeSubscription = this.areaRefs.changes.subscribe((data: QueryList) => { this.areas = data.map(this.mapAndOrderArea); this.changeDetectorRef.markForCheck(); }); } - ngOnDestroy() { + ngOnDestroy(): void { this.areasChangeSubscription.unsubscribe(); } - onMouseDown(event: MouseEvent, leftAreaIndex: number, rightAreaIndex: number) { - if (this.disabled) { + /** @docs-private */ + protected onMouseDown(event: MouseEvent, leftAreaIndex: number, rightAreaIndex: number): void { + if (this.disabled()) { return; } @@ -383,24 +326,24 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy let currentGutter: KbqGutterDirective | undefined; - if (this.useGhost) { + if (this.useGhost()) { const gutterOrder = leftAreaIndex * 2 + 1; - currentGutter = this.gutters().find((gutter: KbqGutterDirective) => gutter.order === gutterOrder); + currentGutter = this.gutters().find((gutter: KbqGutterDirective) => gutter.order() === gutterOrder); if (currentGutter) { const gutterPosition = currentGutter.getPosition(); const ghost = this.ghost(); - ghost.direction = currentGutter.direction; - ghost.size = currentGutter.size; + ghost.direction = currentGutter.direction(); + ghost.size = currentGutter.size(); ghost.x = gutterPosition.x; ghost.y = gutterPosition.y; ghost.visible = true; this.setStyle( StyleProperty.Cursor, - currentGutter.direction === Direction.Vertical ? 'row-resize' : 'col-resize' + currentGutter.direction() === Direction.Vertical ? 'row-resize' : 'col-resize' ); } } else { @@ -424,9 +367,10 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy ); }); - this._isDragging = true; + this.dragging.set(true); } + /** @docs-private */ removeArea(area: KbqSplitterAreaDirective): void { let indexToRemove: number = -1; @@ -447,10 +391,6 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy this.areas.splice(indexToRemove, 1); } - isVertical(): boolean { - return this.direction === Direction.Vertical; - } - private mapAndOrderArea = (area: KbqSplitterAreaDirective, index: number): IArea => { const order = index * this.areaPositionDivider; @@ -466,8 +406,8 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy private updateGutter(): void { this.gutters().forEach((gutter) => { - if (gutter.dragged) { - gutter.dragged = false; + if (gutter.dragged()) { + gutter.dragged.set(false); this.changeDetectorRef.detectChanges(); } @@ -480,8 +420,8 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy leftArea: IArea, rightArea: IArea, currentGutter: KbqGutterDirective | undefined - ) { - if (!this.isDragging || this.disabled) { + ): void { + if (!this.isDragging() || this.disabled()) { return; } @@ -492,7 +432,7 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy const offset = this.isVertical() ? startPoint.y - endPoint.y : startPoint.x - endPoint.x; - if (this.useGhost && currentGutter) { + if (this.useGhost() && currentGutter) { const gutterPosition = currentGutter.getPosition(); const leftPos = leftArea.area.getPosition(); const rightPos = rightArea.area.getPosition(); @@ -502,7 +442,7 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy const key = this.isVertical() ? 'y' : 'x'; const minPos = leftPos[key] - leftMin; - const maxPos = rightPos[key] + (rightArea.area.getSize() || 0) - rightMin - currentGutter.size; + const maxPos = rightPos[key] + (rightArea.area.getSize() || 0) - rightMin - currentGutter.size(); const newPos = gutterPosition[key] - offset; this.ghost()[key] = newPos < minPos ? minPos : Math.min(newPos, maxPos); @@ -532,7 +472,7 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy } } - private onMouseUp(leftArea: IArea, rightArea: IArea, currentGutter: KbqGutterDirective | undefined) { + private onMouseUp(leftArea: IArea, rightArea: IArea, currentGutter: KbqGutterDirective | undefined): void { while (this.listeners.length > 0) { const unsubscribe = this.listeners.pop(); @@ -541,18 +481,18 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy } } - if (this.useGhost && currentGutter) { + if (this.useGhost() && currentGutter) { const gutterPosition = currentGutter.getPosition(); const ghost = this.ghost(); const offset = ghost.direction === Direction.Vertical ? gutterPosition.y - ghost.y : gutterPosition.x - ghost.x; this.resizeAreas(leftArea, rightArea, offset); - this.ghost().visible = false; + ghost.visible = false; this.setStyle(StyleProperty.Cursor, 'unset'); } - this._isDragging = false; + this.dragging.set(false); this.updateGutter(); @@ -562,11 +502,12 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy this.changeDetectorRef.markForCheck(); } - private setStyle(property: StyleProperty, value: string | number) { - this.renderer.setStyle(this.elementRef.nativeElement, property, value); + private setStyle(property: StyleProperty, value: string | number): void { + this.renderer.setStyle(this.nativeElement, property, value); } } +/** Directive that marks a resizable area of a splitter. */ @Directive({ selector: '[kbq-splitter-area]', host: { @@ -575,29 +516,31 @@ export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy } }) export class KbqSplitterAreaDirective implements AfterViewInit, OnDestroy { - private elementRef = inject>(ElementRef); - private renderer = inject(Renderer2); - private splitter = inject(KbqSplitterComponent); - - readonly sizeChange = output(); - + private readonly nativeElement = kbqInjectNativeElement(); + private readonly renderer = inject(Renderer2); + private readonly splitter = inject(KbqSplitterComponent); private readonly window = inject(KBQ_WINDOW); private readonly platform = inject(Platform); - isResizing(): boolean { - return this.splitter.isDragging; - } + private gutterPositionSubscription: OutputRefSubscription | null = null; + /** Emitted with the new size once a drag that changed this area has finished. */ + readonly sizeChange = output(); + + /** @docs-private */ + protected readonly isResizing = computed(() => this.splitter.isDragging()); + + /** @docs-private */ disableFlex(): void { - this.renderer.removeStyle(this.elementRef.nativeElement, 'flex'); + this.renderer.removeStyle(this.nativeElement, StyleProperty.Flex); } - ngAfterViewInit() { + ngAfterViewInit(): void { this.splitter.addArea(this); this.removeStyle(StyleProperty.MaxWidth); - if (this.splitter.direction === Direction.Vertical) { + if (this.splitter.isVertical()) { this.setStyle(StyleProperty.Width, '100%'); this.removeStyle(StyleProperty.Height); } else { @@ -605,66 +548,68 @@ export class KbqSplitterAreaDirective implements AfterViewInit, OnDestroy { this.removeStyle(StyleProperty.Width); } - this.splitter.gutterPositionChange.subscribe(this.emitSizeChange); + this.gutterPositionSubscription = this.splitter.gutterPositionChange.subscribe(this.emitSizeChange); } ngOnDestroy(): void { + this.gutterPositionSubscription?.unsubscribe(); this.splitter.removeArea(this); } + /** @docs-private */ setOrder(order: number): void { this.setStyle(StyleProperty.Order, order); } + /** @docs-private */ setSize(size: number): void { if (isNaN(size)) { return; } - this.setStyle(this.getSizeProperty(), coerceCssPixelValue(coerceNumberProperty(size))); + this.setStyle(this.getSizeProperty(), coerceCssPixelValue(numberAttribute(size, 0))); } + /** @docs-private */ getSize(): number { if (!this.platform.isBrowser) return 0; - return this.elementRef.nativeElement[this.getOffsetSizeProperty()]; + return this.nativeElement[this.getOffsetSizeProperty()]; } + /** @docs-private */ getPosition(): IPoint { return { - x: this.elementRef.nativeElement.offsetLeft, - y: this.elementRef.nativeElement.offsetTop + x: this.nativeElement.offsetLeft, + y: this.nativeElement.offsetTop }; } + /** @docs-private */ getMinSize(): number { - const styles = this.window.getComputedStyle(this.elementRef.nativeElement); + const styles = this.window.getComputedStyle(this.nativeElement); return parseFloat(styles[this.getMinSizeProperty()]); } - private isVertical(): boolean { - return this.splitter.direction === Direction.Vertical; - } - private getMinSizeProperty(): StyleProperty { - return this.isVertical() ? StyleProperty.MinHeight : StyleProperty.MinWidth; + return this.splitter.isVertical() ? StyleProperty.MinHeight : StyleProperty.MinWidth; } private getOffsetSizeProperty(): StyleProperty { - return this.isVertical() ? StyleProperty.OffsetHeight : StyleProperty.OffsetWidth; + return this.splitter.isVertical() ? StyleProperty.OffsetHeight : StyleProperty.OffsetWidth; } private getSizeProperty(): StyleProperty { - return this.isVertical() ? StyleProperty.Height : StyleProperty.Width; + return this.splitter.isVertical() ? StyleProperty.Height : StyleProperty.Width; } - private setStyle(style: StyleProperty, value: string | number) { - this.renderer.setStyle(this.elementRef.nativeElement, style, value); + private setStyle(style: StyleProperty, value: string | number): void { + this.renderer.setStyle(this.nativeElement, style, value); } - private removeStyle(style: StyleProperty) { - this.renderer.removeStyle(this.elementRef.nativeElement, style); + private removeStyle(style: StyleProperty): void { + this.renderer.removeStyle(this.nativeElement, style); } private emitSizeChange = () => { diff --git a/packages/components/splitter/splitter.spec.ts b/packages/components/splitter/splitter.spec.ts index 87d7495936..190dfbc66a 100644 --- a/packages/components/splitter/splitter.spec.ts +++ b/packages/components/splitter/splitter.spec.ts @@ -122,9 +122,9 @@ class KbqSplitterGhost { template: ` @if (isFirstRendered) { -
first
+
first
} -
second
+
second
` }) @@ -275,17 +275,98 @@ describe('KbqSplitter', () => { update(); + const orderOf = (ref: 'areaA' | 'areaB'): number => + +fixture.debugElement.query(By.css(`[kbq-splitter-area]#${ref}`)).nativeElement.style.order; + expect(componentInstance.areaA()).toBeTruthy(); - expect(+(componentInstance.areaA() as any).elementRef.nativeElement.style.order).toBe(0); - const areaBInitialOrder = +(componentInstance.areaB() as any).elementRef.nativeElement.style.order; + expect(orderOf('areaA')).toBe(0); + + const areaBInitialOrder = orderOf('areaB'); componentInstance.isFirstRendered = false; update(); expect(componentInstance.areaA()).toBeFalsy(); - expect(+(componentInstance.areaB() as any).elementRef.nativeElement.style.order).not.toEqual( - areaBInitialOrder - ); + expect(orderOf('areaB')).not.toEqual(areaBInitialOrder); })); }); + describe('gutterSize', () => { + it('should fall back to the default for a non-positive size', () => { + const fixture = createTestComponent(KbqSplitterGutterSize); + + fixture.detectChanges(); + + checkDirection(fixture, Direction.Horizontal, 2, EXPECTED_GUTTER_SIZE); + + fixture.componentInstance.gutterSize = 12; + fixture.detectChanges(); + + checkDirection(fixture, Direction.Horizontal, 2, 12); + }); + }); + + describe('valueless attributes', () => { + it('should treat disabled, hideGutters and useGhost as true', () => { + const fixture = createTestComponent(KbqSplitterValuelessAttributes); + + fixture.detectChanges(); + + const splitter = fixture.debugElement.query(By.directive(KbqSplitterComponent)) + .componentInstance as KbqSplitterComponent; + const gutter = fixture.debugElement.query(By.directive(KbqGutterDirective)); + + expect(splitter.disabled()).toBe(true); + expect(splitter.hideGutters()).toBe(true); + expect(splitter.useGhost()).toBe(true); + expect(gutter.nativeElement.style.display).toBe('none'); + expect(fixture.debugElement.query(By.directive(KbqGutterGhostDirective))).not.toBeNull(); + }); + }); + + describe('reactive layout', () => { + it('should re-apply the gutter layout when the direction changes after init', () => { + const fixture = createTestComponent(KbqSplitterDirection); + + fixture.componentInstance.direction = Direction.Horizontal; + fixture.detectChanges(); + + checkDirection(fixture, Direction.Horizontal, 2, EXPECTED_GUTTER_SIZE); + + fixture.componentInstance.direction = Direction.Vertical; + fixture.detectChanges(); + + checkDirection(fixture, Direction.Vertical, 2, EXPECTED_GUTTER_SIZE); + }); + }); }); + +@Component({ + selector: 'kbq-demo-splitter', + imports: [ + KbqSplitterModule + ], + template: ` + +
first
+
second
+
third
+
+ ` +}) +class KbqSplitterGutterSize { + gutterSize = 0; +} + +@Component({ + selector: 'kbq-demo-splitter', + imports: [ + KbqSplitterModule + ], + template: ` + +
first
+
second
+
+ ` +}) +class KbqSplitterValuelessAttributes {} diff --git a/packages/schematics/src/collection.json b/packages/schematics/src/collection.json index c2cc44abcb..a57118404d 100644 --- a/packages/schematics/src/collection.json +++ b/packages/schematics/src/collection.json @@ -143,6 +143,11 @@ "description": "Reports the KbqSplitButton members whose type changed in the split-button review. disabled reports boolean | undefined instead of boolean: the backing field has no initializer, so a control with no [disabled] binding always returned undefined behind a non-nullable type, and the call sites that assigned it to a boolean or compared it against false were quietly wrong. The protected buttons content query moved from QueryList to a signal query, so a subclass reading buttons.changes or buttons.length has to read buttons() instead. Warn-only: narrowing boolean | undefined back to boolean is a decision the call site owns. Notes the silent part too — an empty no longer throws outside dev mode.", "factory": "./migrations/split-button-optional-disabled/index", "schema": "./migrations/split-button-optional-disabled/schema.json" + }, + "splitter-signals": { + "description": "Migrates KbqSplitterComponent and KbqGutterDirective consumers to the finished signal-based API. Every input on both was an accessor with coercion in the setter, which is why the automated signal migration skipped all of them. Rewrites programmatic reads of hideGutters, direction, disabled, useGhost, gutterSize, isDragging, isVertical, order, size and dragged to calls on receivers typed KbqSplitterComponent or KbqGutterDirective, including reads via template reference variables on , and rewrites writes to the gutter's dragged signal. Warns on the removed resizing getter, which was dead and always reported false, on the layout bookkeeping that left the public surface (elementRef, changeDetectorRef, areas, areaRefs, gutters, ghost), on KbqGutterGhostDirective losing inputs it never actually accepted, and on view/content queries that return the instance. Reports the booleanAttribute transforms, the gutterSize fallback, the reactive gutter layout and the sizeChange teardown on a destroyed area.", + "factory": "./migrations/splitter-signals/index", + "schema": "./migrations/splitter-signals/schema.json" } } } diff --git a/packages/schematics/src/migrations.json b/packages/schematics/src/migrations.json index 152d6b710a..9ce3e27959 100644 --- a/packages/schematics/src/migrations.json +++ b/packages/schematics/src/migrations.json @@ -105,6 +105,11 @@ "version": "20.3.0-0", "description": "Reports the KbqSplitButton members whose type changed in the split-button review. disabled reports boolean | undefined instead of boolean: the backing field has no initializer, so a control with no [disabled] binding always returned undefined behind a non-nullable type, and the call sites that assigned it to a boolean or compared it against false were quietly wrong. The protected buttons content query moved from QueryList to a signal query, so a subclass reading buttons.changes or buttons.length has to read buttons() instead. Warn-only: narrowing boolean | undefined back to boolean is a decision the call site owns. Notes the silent part too — an empty no longer throws outside dev mode.", "factory": "./migrations/split-button-optional-disabled/index" + }, + "splitter-signals": { + "version": "20.3.0-0", + "description": "Migrates KbqSplitterComponent and KbqGutterDirective consumers to the finished signal-based API. Every input on both was an accessor with coercion in the setter, which is why the automated signal migration skipped all of them. Rewrites programmatic reads of hideGutters, direction, disabled, useGhost, gutterSize, isDragging, isVertical, order, size and dragged to calls on receivers typed KbqSplitterComponent or KbqGutterDirective, including reads via template reference variables on , and rewrites writes to the gutter's dragged signal. Warns on the removed resizing getter, which was dead and always reported false, on the layout bookkeeping that left the public surface (elementRef, changeDetectorRef, areas, areaRefs, gutters, ghost), on KbqGutterGhostDirective losing inputs it never actually accepted, and on view/content queries that return the instance. Reports the booleanAttribute transforms, the gutterSize fallback, the reactive gutter layout and the sizeChange teardown on a destroyed area.", + "factory": "./migrations/splitter-signals/index" } } } diff --git a/packages/schematics/src/migrations/splitter-signals/README.md b/packages/schematics/src/migrations/splitter-signals/README.md new file mode 100644 index 0000000000..0e5224e86a --- /dev/null +++ b/packages/schematics/src/migrations/splitter-signals/README.md @@ -0,0 +1,58 @@ +# splitter-signals + +Migration schematic invoked automatically by `ng update @koobiq/components@20` (registered for +`20.3.0-0`). Migrates `KbqSplitterComponent` and `KbqGutterDirective` consumers to the finished +signal-based API. + +## Background + +Every input on the splitter and its gutter was an accessor with coercion in the setter, which is why +the automated signal migration skipped all thirteen. They are signal inputs now, with +`booleanAttribute` and `numberAttribute` doing the coercion. + +`KbqGutterGhostDirective` went the other way. Its `visible`, `x`, `y`, `direction` and `size` were +`@Input()` in name only: the splitter renders `` with **no bindings** and drives +them imperatively during a drag, outside the Angular zone. They are plain properties now. + +## What it rewrites + +| Before | After | +| --------------------------------------------------------------------------------- | ----------------------- | +| `splitter.hideGutters` / `.direction` / `.disabled` / `.useGhost` / `.gutterSize` | calls | +| `splitter.isDragging` / `.isVertical` | calls | +| `gutter.direction` / `.order` / `.size` / `.isVertical` / `.dragged` | calls | +| `gutter.dragged = …` | `gutter.dragged.set(…)` | + +On receivers explicitly typed `KbqSplitterComponent` or `KbqGutterDirective`, and through template +reference variables on ``. Already-migrated reads are left alone, so the schematic is +idempotent. + +## What it does _not_ do + +| Pattern | Manual migration | +| ------------------------------------------------------------------------------------- | ------------------------------------------------------------ | +| `.resizing` | Removed — it was dead and always `false`; use `isDragging()` | +| `.elementRef` / `.changeDetectorRef` / `.areas` / `.areaRefs` / `.gutters` / `.ghost` | Closed layout bookkeeping | +| `KbqGutterGhostDirective` bindings | It never accepted them; there is nothing to bind | +| `viewChild(KbqSplitterComponent)` | The query returns the instance, so a read is a double call | + +## Notes with no call site to point at + +- **`hideGutters`, `disabled` and `useGhost` are `booleanAttribute` inputs.** A valueless attribute + means `true` now; `coerceBooleanProperty` treated the empty string as `false`. +- **A `gutterSize` that is not a positive number falls back to the default 6** instead of keeping + whatever the previous value happened to be. The old setter read its own getter, so an invalid + value silently preserved the last valid one. +- **The gutter lays itself out reactively** instead of once in `ngOnInit`, so changing `direction` + after init re-applies the layout — and clears the dimension the other direction owns, which used + to stay behind as a stale `width` or `height`. +- **A splitter area unsubscribes from `gutterPositionChange` when it is destroyed.** An area removed + from a long-lived splitter used to keep emitting `sizeChange` for every later drag. + +## Running it manually + +``` +ng generate @koobiq/components:splitter-signals --project my-app +``` + +Pass `--fix=false` to see what it would change without writing. diff --git a/packages/schematics/src/migrations/splitter-signals/data.ts b/packages/schematics/src/migrations/splitter-signals/data.ts new file mode 100644 index 0000000000..cc68e32354 --- /dev/null +++ b/packages/schematics/src/migrations/splitter-signals/data.ts @@ -0,0 +1,107 @@ +/** + * Data for the `splitter-signals` migration. + * + * Every input on the splitter and its gutter was an accessor with coercion in the setter, which is why the + * automated signal migration skipped all of them. + * + * - `hideGutters` / `direction` / `disabled` / `useGhost` / `gutterSize` on the splitter → calls (auto-fixed) + * - `direction` / `order` / `size` / `isVertical` / `dragged` on the gutter → calls (auto-fixed) + * - `isDragging` / `isVertical` → signals (auto-fixed) + * - `resizing` → removed; it was dead, always `false` (warn) + * - `elementRef` / `changeDetectorRef` / `areas` / `areaRefs` / `gutters` / `ghost` → closed (warn) + * + * `KbqGutterGhostDirective` kept its plain properties: the splitter drives them imperatively during a drag, + * outside the Angular zone, and nothing ever bound them — they were `@Input()` in name only. + */ + +/** Members whose value is unchanged; a read must become a call. Auto-fixed. */ +export const SIGNAL_MEMBERS: readonly string[] = [ + 'hideGutters', + 'direction', + 'disabled', + 'useGhost', + 'gutterSize', + 'isDragging', + 'isVertical', + 'order', + 'size', + 'dragged' +]; + +/** + * Signal members that are writable via `.set(...)`. `dragged` on the gutter is the one writable signal; + * everything else is an `input()` or a read-only `computed`. + */ +export const WRITABLE_MEMBERS: ReadonlySet = new Set(['dragged']); + +/** TypeScript type annotation that marks a receiver as a splitter. */ +export const SPLITTER_TYPE = 'KbqSplitterComponent'; + +/** Every type whose members this migration rewrites. */ +export const RECEIVER_TYPES: readonly string[] = ['KbqSplitterComponent', 'KbqGutterDirective']; + +/** Element selector whose template reference variables (`#ref`) point at a splitter. */ +export const SPLITTER_ELEMENT = 'kbq-splitter'; + +/** Import specifier that marks a file as a splitter consumer. */ +export const SPLITTER_PACKAGE = '@koobiq/components/splitter'; + +/** Members that left the public surface and can no longer be reached from outside. */ +export const PROTECTED_MEMBERS: readonly string[] = [ + 'resizing', + 'elementRef', + 'changeDetectorRef', + 'areas', + 'areaRefs', + 'gutters', + 'ghost' +]; + +/** Appended to the protected-members warning. */ +export const PROTECTED_HINT = + '`resizing` was dead — nothing ever set it, so it always reported false; read `isDragging()` instead. ' + + 'The rest is the layout bookkeeping: bind the inputs and listen to `gutterPositionChange` / `sizeChange`.'; + +export interface WarnPattern { + /** Owner of the member. The pattern is only evaluated for files that also name it. */ + anchor: string; + pattern: string; + message: string; +} + +const SPLITTER_ANCHOR = '\\bKbqSplitterComponent\\b'; + +export const warnPatterns: WarnPattern[] = [ + { + anchor: '\\bKbqGutterGhostDirective\\b', + pattern: '\\bKbqGutterGhostDirective\\b', + message: + 'KbqGutterGhostDirective no longer declares inputs. Its `visible`, `x`, `y`, `direction` and `size` ' + + 'were `@Input()` in name only — the splitter renders with no bindings and drives ' + + 'them imperatively during a drag. They are plain properties now, so a template binding on them ' + + 'stops compiling; there was never a supported way to place the ghost yourself.' + }, + { + anchor: SPLITTER_ANCHOR, + pattern: '(?:viewChild|ViewChild|contentChild|ContentChild)[^\\n;]*\\bKbqSplitterComponent\\b', + message: + 'A KbqSplitterComponent view/content query returns the component instance, whose inputs are now ' + + 'signals — reading one is a double call, e.g. `this.splitter().disabled()`. Verify query reads ' + + 'manually.' + } +]; + +/** Printed once per project, after the per-file reports. */ +export const SUMMARY = [ + ' `hideGutters`, `disabled` and `useGhost` are `booleanAttribute` inputs, and `gutterSize` is a ' + + 'numeric one. A valueless attribute means true now, where `coerceBooleanProperty` treated the empty ' + + 'string as false.', + ' A `gutterSize` that is not a positive number falls back to the default 6 instead of keeping whatever ' + + 'the previous value happened to be. The old setter read its own getter, so an invalid value silently ' + + 'preserved the last valid one.', + ' The gutter lays itself out reactively instead of once in ngOnInit, so changing `direction` after init ' + + 'now re-applies the layout — and clears the dimension the other direction owns, which used to stay ' + + 'behind as a stale width or height.', + ' A splitter area unsubscribes from `gutterPositionChange` when it is destroyed. An area removed from a ' + + 'long-lived splitter used to keep emitting `sizeChange` for every later drag.' +]; diff --git a/packages/schematics/src/migrations/splitter-signals/index.spec.ts b/packages/schematics/src/migrations/splitter-signals/index.spec.ts new file mode 100644 index 0000000000..6ad0b208c2 --- /dev/null +++ b/packages/schematics/src/migrations/splitter-signals/index.spec.ts @@ -0,0 +1,303 @@ +import { workspaces } from '@angular-devkit/core'; +import { Tree } from '@angular-devkit/schematics'; +import { SchematicTestRunner } from '@angular-devkit/schematics/testing'; +import { getWorkspace } from '@schematics/angular/utility/workspace'; +import * as path from 'path'; +import { createTestApp } from '../../utils/testing'; +import { Schema } from './schema'; + +const collectionPath = path.join(__dirname, '../../collection.json'); +const SCHEMATIC_NAME = 'splitter-signals'; + +describe(SCHEMATIC_NAME, () => { + let runner: SchematicTestRunner; + let appTree: Tree; + let projects: workspaces.ProjectDefinitionCollection; + let messages: string[]; + + beforeEach(async () => { + runner = new SchematicTestRunner('schematics', collectionPath); + appTree = await createTestApp(runner, { style: 'scss' }); + const workspace = await getWorkspace(appTree); + + projects = workspace.projects as unknown as workspaces.ProjectDefinitionCollection; + + messages = []; + runner.logger.subscribe((entry) => messages.push(entry.message)); + }); + + function paths(project: workspaces.ProjectDefinition) { + const root = `/${project.root}/src/app`; + const ts = appTree.exists(`${root}/app.ts`) ? `${root}/app.ts` : `${root}/app.component.ts`; + const html = appTree.exists(`${root}/app.html`) ? `${root}/app.html` : `${root}/app.component.html`; + + return { ts, html }; + } + + async function run(fix: boolean = true): Promise { + const [first] = projects.keys(); + + return runner.runSchematic(SCHEMATIC_NAME, { project: first, fix } satisfies Schema, appTree); + } + + function firstTsPath(): string { + const [first] = projects.keys(); + + return paths(projects.get(first)!).ts; + } + + function firstHtmlPath(): string { + const [first] = projects.keys(); + + return paths(projects.get(first)!).html; + } + + it('rewrites disabled reads on a parameter typed KbqSplitterComponent (incl. optional chain) to calls', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqSplitterComponent } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' read(splitter: KbqSplitterComponent) {\n' + + ' return splitter.disabled ?? splitter?.disabled;\n' + + ' }\n' + + '}\n' + ); + + const updated = (await run()).readText(ts); + + expect(updated).toContain('splitter.disabled() ?? splitter?.disabled()'); + }); + + it('rewrites reads on a @ViewChild field (this.splitter)', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { ViewChild } from '@angular/core';\n" + + "import { KbqSplitterComponent } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' @ViewChild(KbqSplitterComponent) splitter: KbqSplitterComponent;\n' + + ' read() {\n' + + ' return this.splitter.disabled;\n' + + ' }\n' + + '}\n' + ); + + expect((await run()).readText(ts)).toContain('return this.splitter.disabled();'); + }); + + it('leaves reads on a receiver of an unrelated type alone', async () => { + const ts = firstTsPath(); + const source = + "import { KbqSplitterComponent } from '@koobiq/components/splitter';\n" + + 'class Other {\n' + + ' disabled = false;\n' + + '}\n' + + 'class Demo {\n' + + ' read(other: Other) {\n' + + ' return other.disabled;\n' + + ' }\n' + + '}\n'; + + appTree.overwrite(ts, source); + + expect((await run()).readText(ts)).toBe(source); + }); + + it('is idempotent — an already migrated read is left alone', async () => { + const ts = firstTsPath(); + const source = + "import { KbqSplitterComponent } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' read(splitter: KbqSplitterComponent) {\n' + + ' return splitter.disabled();\n' + + ' }\n' + + '}\n'; + + appTree.overwrite(ts, source); + + expect((await run()).readText(ts)).toBe(source); + }); + + it('leaves a programmatic write alone — the input is read-only', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqSplitterComponent } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' write(splitter: KbqSplitterComponent) {\n' + + ' splitter.disabled = true;\n' + + ' }\n' + + '}\n' + ); + + expect((await run()).readText(ts)).toContain('splitter.disabled = true;'); + }); + + it('rewrites template reference reads in an external template', async () => { + const html = firstHtmlPath(); + + appTree.overwrite(html, '\n{{ splitter.disabled }}\n'); + + expect((await run()).readText(html)).toContain('{{ splitter.disabled() }}'); + }); + + it('rewrites template reference reads inside an inline template', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { Component } from '@angular/core';\n" + + "@Component({ template: '{{ splitter.disabled }}' })\n" + + 'class Demo {}\n' + ); + + expect((await run()).readText(ts)).toContain('{{ splitter.disabled() }}'); + }); + + it('leaves a template reference on an unrelated element alone', async () => { + const html = firstHtmlPath(); + const source = '\n{{ splitter.disabled }}\n'; + + appTree.overwrite(html, source); + + expect((await run()).readText(html)).toBe(source); + }); + + it('warns about the layout bookkeeping that left the public surface', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqSplitterComponent } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' read(splitter: KbqSplitterComponent) {\n' + + ' return splitter.resizing + splitter.areas + splitter.elementRef;\n' + + ' }\n' + + '}\n' + ); + + await run(); + + const logged = messages.join('\n'); + + expect(logged).toContain('resizing'); + expect(logged).toContain('areas'); + expect(logged).toContain('elementRef'); + }); + + it('warns about a view query returning the instance', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { viewChild } from '@angular/core';\n" + + "import { KbqSplitterComponent } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' readonly splitter = viewChild(KbqSplitterComponent);\n' + + '}\n' + ); + + await run(); + + expect(messages.join('\n')).toContain('double call'); + }); + + it('rewrites gutter reads on a KbqGutterDirective receiver', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqGutterDirective } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' read(gutter: KbqGutterDirective) {\n' + + ' return gutter.order + gutter.size + gutter.direction;\n' + + ' }\n' + + '}\n' + ); + + expect((await run()).readText(ts)).toContain('gutter.order() + gutter.size() + gutter.direction()'); + }); + + it('rewrites a write to the gutter dragged signal', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqGutterDirective } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' reset(gutter: KbqGutterDirective) {\n' + + ' gutter.dragged = false;\n' + + ' }\n' + + '}\n' + ); + + expect((await run()).readText(ts)).toContain('gutter.dragged.set(false);'); + }); + + it('warns about the ghost directive losing its inputs', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqGutterGhostDirective } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' place(ghost: KbqGutterGhostDirective) {\n' + + ' ghost.x = 10;\n' + + ' }\n' + + '}\n' + ); + + await run(); + + expect(messages.join('\n')).toContain('in name only'); + }); + + it('reports the attribute, fallback, layout and teardown notes once per project', async () => { + const ts = firstTsPath(); + + appTree.overwrite( + ts, + "import { KbqSplitterComponent } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' read(splitter: KbqSplitterComponent) {\n' + + ' return splitter.disabled;\n' + + ' }\n' + + '}\n' + ); + + await run(); + + const summary = messages.join('\n'); + + expect(summary).toContain('booleanAttribute'); + expect(summary).toContain('gutterSize'); + expect(summary).toContain('gutterPositionChange'); + expect(summary.match(/booleanAttribute/g)!.length).toBe(1); + }); + + it('stays silent for a workspace that does not use the splitter', async () => { + await run(); + + expect(messages.join('\n')).not.toContain(`[${SCHEMATIC_NAME}]`); + }); + + it('does not write when fix is false', async () => { + const ts = firstTsPath(); + const source = + "import { KbqSplitterComponent } from '@koobiq/components/splitter';\n" + + 'class Demo {\n' + + ' read(splitter: KbqSplitterComponent) {\n' + + ' return splitter.disabled;\n' + + ' }\n' + + '}\n'; + + appTree.overwrite(ts, source); + + expect((await run(false)).readText(ts)).toBe(source); + expect(messages.join('\n')).toContain('would update'); + }); +}); diff --git a/packages/schematics/src/migrations/splitter-signals/index.ts b/packages/schematics/src/migrations/splitter-signals/index.ts new file mode 100644 index 0000000000..4da7c96ee6 --- /dev/null +++ b/packages/schematics/src/migrations/splitter-signals/index.ts @@ -0,0 +1,463 @@ +import { Path } from '@angular-devkit/core'; +import { Rule, SchematicContext, Tree } from '@angular-devkit/schematics'; +import ts from 'typescript'; +import { visitAll, Visitor } from '../../utils/ast'; +import { logMessage } from '../../utils/messages'; +import { setupOptions } from '../../utils/package-config'; +import { forEachClass, parseTemplate } from '../../utils/typescript'; +import { + PROTECTED_HINT, + PROTECTED_MEMBERS, + RECEIVER_TYPES, + SIGNAL_MEMBERS, + SPLITTER_ELEMENT, + SPLITTER_PACKAGE, + SUMMARY, + warnPatterns, + WRITABLE_MEMBERS +} from './data'; +import { Schema } from './schema'; + +const LABEL = '[splitter-signals]'; +const TS_EXT = '.ts'; +const HTML_EXT = '.html'; + +/** A text-span edit on the original file content. Applied right-to-left so offsets stay valid. */ +interface Edit { + start: number; + end: number; + text: string; +} + +/** A receiver whose static type is a splitter, valid within `[start, end]` of the source. */ +interface Receiver { + /** Source text of the receiver expression, e.g. `splitter` or `this.splitter`. */ + text: string; + start: number; + end: number; +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** Applies text-span edits to `content`, right-to-left, so earlier edits don't shift later offsets. */ +function applyEdits(content: string, edits: Edit[]): string { + const sorted = [...edits].sort((a, b) => b.start - a.start || b.end - a.end); + let result = content; + + for (const { start, end, text } of sorted) { + result = result.slice(0, start) + text + result.slice(end); + } + + return result; +} + +const isFunctionLike = (node: ts.Node): boolean => + ts.isFunctionDeclaration(node) || + ts.isMethodDeclaration(node) || + ts.isConstructorDeclaration(node) || + ts.isArrowFunction(node) || + ts.isFunctionExpression(node) || + ts.isGetAccessorDeclaration(node) || + ts.isSetAccessorDeclaration(node); + +/** Walks up from `node` to the nearest ancestor matching `predicate`. */ +function findAncestor(node: ts.Node, predicate: (node: ts.Node) => boolean): ts.Node | undefined { + let current = node.parent; + + while (current) { + if (predicate(current)) return current; + current = current.parent; + } + + return undefined; +} + +/** Whether a type annotation refers to `typeName`. */ +function isTypeReference(type: ts.TypeNode | undefined, typeName: string): boolean { + return !!type && ts.isTypeReferenceNode(type) && ts.isIdentifier(type.typeName) && type.typeName.text === typeName; +} + +const FIELD_MODIFIERS = new Set([ + ts.SyntaxKind.PrivateKeyword, + ts.SyntaxKind.PublicKeyword, + ts.SyntaxKind.ProtectedKeyword, + ts.SyntaxKind.ReadonlyKeyword +]); + +/** + * Collects the receivers annotated with `typeName`, by explicit annotation only (no cross-package type + * resolution): method/function params, class fields (incl. `@ViewChild(KbqSplitterComponent) x: KbqSplitterComponent` and constructor + * parameter-properties) and typed locals. + */ +function collectReceivers(sourceFile: ts.SourceFile, typeName: string): Receiver[] { + const receivers: Receiver[] = []; + const add = (text: string, scope: ts.Node) => + receivers.push({ text, start: scope.getStart(sourceFile), end: scope.getEnd() }); + + const visit = (node: ts.Node): void => { + if (ts.isParameter(node) && ts.isIdentifier(node.name) && isTypeReference(node.type, typeName)) { + add(node.name.text, findAncestor(node, isFunctionLike) ?? sourceFile); + + // A constructor parameter-property is also a class field, reachable as `this.`. + if (node.modifiers?.some((modifier) => FIELD_MODIFIERS.has(modifier.kind))) { + const owner = findAncestor(node, ts.isClassDeclaration); + + if (owner) add(`this.${node.name.text}`, owner); + } + } else if ( + ts.isPropertyDeclaration(node) && + ts.isIdentifier(node.name) && + isTypeReference(node.type, typeName) + ) { + const owner = findAncestor(node, ts.isClassDeclaration); + + if (owner) add(`this.${node.name.text}`, owner); + } else if ( + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + isTypeReference(node.type, typeName) + ) { + add(node.name.text, findAncestor(node, isFunctionLike) ?? sourceFile); + } + + node.forEachChild(visit); + }; + + visit(sourceFile); + + return receivers; +} + +/** Whether a property access on a receiver is within one of the receiver's scopes. */ +function inReceiverScope(node: ts.PropertyAccessExpression, sourceFile: ts.SourceFile, receivers: Receiver[]): boolean { + const receiverText = node.expression.getText(sourceFile); + const start = node.getStart(sourceFile); + const end = node.getEnd(); + + return receivers.some((r) => r.text === receiverText && start >= r.start && end <= r.end); +} + +/** Classifies a matched property access and appends the resulting edit(s). */ +function classifyAccess(node: ts.PropertyAccessExpression, sourceFile: ts.SourceFile, edits: Edit[]): void { + const parent = node.parent; + + // Already migrated: a call, or a `.set(...)` write — leave alone (idempotent). + if (ts.isCallExpression(parent) && parent.expression === node) return; + if (ts.isPropertyAccessExpression(parent) && parent.expression === node && parent.name.text === 'set') return; + + // Write target: `x.member = RHS`. Every KbqSplitterComponent signal member is `input()` (read-only), so there is no + // writable member — leave the write untouched (it becomes a compile error the consumer fixes by hand). + if ( + ts.isBinaryExpression(parent) && + parent.left === node && + parent.operatorToken.kind === ts.SyntaxKind.EqualsToken + ) { + if (WRITABLE_MEMBERS.has(node.name.text)) { + const rhs = parent.right; + + edits.push({ start: node.getEnd(), end: rhs.getStart(sourceFile), text: '.set(' }); + edits.push({ start: rhs.getEnd(), end: rhs.getEnd(), text: ')' }); + } + + return; + } + + // Read (incl. optional chain `x?.compact`): append `()`. + edits.push({ start: node.getEnd(), end: node.getEnd(), text: '()' }); +} + +/** Collects edits for every read/write of a value-safe signal member on a known splitter receiver. */ +function collectAccessEdits(sourceFile: ts.SourceFile, receivers: Receiver[]): Edit[] { + const edits: Edit[] = []; + + const visit = (node: ts.Node): void => { + if ( + ts.isPropertyAccessExpression(node) && + ts.isIdentifier(node.name) && + SIGNAL_MEMBERS.includes(node.name.text) && + inReceiverScope(node, sourceFile, receivers) + ) { + classifyAccess(node, sourceFile, edits); + } + + node.forEachChild(visit); + }; + + visit(sourceFile); + + return edits; +} + +/** Collects the members read on a splitter receiver that no consumer can keep reading as-is. */ +function collectProtectedAccess(sourceFile: ts.SourceFile, receivers: Receiver[]): Set { + const protectedAccess = new Set(); + + const visit = (node: ts.Node): void => { + if ( + ts.isPropertyAccessExpression(node) && + ts.isIdentifier(node.name) && + PROTECTED_MEMBERS.includes(node.name.text) && + inReceiverScope(node, sourceFile, receivers) + ) { + protectedAccess.add(node.name.text); + } + + node.forEachChild(visit); + }; + + visit(sourceFile); + + return protectedAccess; +} + +/** Pass A — rewrite value-safe programmatic reads of splitter signal members in TypeScript code. */ +function migrateTsExpressions(content: string, fileName: string): string { + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS); + const receivers = RECEIVER_TYPES.flatMap((type) => collectReceivers(sourceFile, type)); + + if (receivers.length === 0) return content; + + const edits = collectAccessEdits(sourceFile, receivers); + + return edits.length > 0 ? applyEdits(content, edits) : content; +} + +/** Emits precise, receiver-scoped warnings for the members that can't be auto-fixed. */ +function warnReceiverMembers(context: SchematicContext, filePath: string, content: string): void { + const sourceFile = ts.createSourceFile(filePath, content, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS); + const receivers = RECEIVER_TYPES.flatMap((type) => collectReceivers(sourceFile, type)); + + if (receivers.length === 0) return; + + const protectedAccess = collectProtectedAccess(sourceFile, receivers); + + if (protectedAccess.size > 0) { + logMessage(context.logger, [ + `${LABEL} ${filePath}`, + ` These KbqSplitterComponent members are now \`protected\` and can't be read from outside the ` + + `component: ${[...protectedAccess].join(', ')}. ${PROTECTED_HINT}` + ]); + } +} + +/** Collects template reference variable names bound to a `` element. */ +class SplitterComponentRefCollector implements Visitor { + readonly refs = new Set(); + + visitElement(element: any): void { + if (element.name === SPLITTER_ELEMENT) { + for (const attr of element.attrs ?? []) { + if (typeof attr.name !== 'string') continue; + + if (attr.name.startsWith('#')) this.refs.add(attr.name.slice(1)); + else if (attr.name.startsWith('ref-')) this.refs.add(attr.name.slice(4)); + } + } + + this.visitChildren(element); + } + + visitBlock(block: any): void { + this.visitChildren(block); + } + + private visitChildren(node: any): void { + for (const child of node.children ?? []) { + child.visit(this); + } + } + + visitAttribute(): void {} + visitText(): void {} + visitComment(): void {} + visitExpansion(): void {} + visitExpansionCase(): void {} + visitBlockParameter(): void {} + visitLetDeclaration(): void {} +} + +/** Rewrites `ref.member` reads to `ref.member()` for the given refs, scoped to those exact identifiers. */ +function rewriteRefReads(template: string, refs: string[]): { content: string; changed: boolean } { + const members = SIGNAL_MEMBERS.join('|'); + let content = template; + let changed = false; + + for (const ref of refs) { + // `\bref\.(member)\b(?!\s*\()` — skip anything already invoked, so the rewrite is idempotent. + const pattern = new RegExp(`\\b(${escapeRegExp(ref)})\\.(${members})\\b(?!\\s*\\()`, 'g'); + const next = content.replace(pattern, '$1.$2()'); + + if (next !== content) { + content = next; + changed = true; + } + } + + return { content, changed }; +} + +/** Pass B (core) — parse a template, discover splitter refs, rewrite their value-safe signal reads. */ +async function migrateTemplate(template: string): Promise<{ content: string; changed: boolean }> { + if (!template.includes(SPLITTER_ELEMENT)) return { content: template, changed: false }; + + const parsed = await parseTemplate(template); + + if (!parsed.tree) return { content: template, changed: false }; + + const collector = new SplitterComponentRefCollector(); + + visitAll(collector, (parsed.tree as { rootNodes: unknown[] }).rootNodes); + + if (collector.refs.size === 0) return { content: template, changed: false }; + + return rewriteRefReads(template, [...collector.refs]); +} + +/** Interior `[start, end]` ranges of inline `@Component({ template: '…' })` string literals. */ +function collectInlineTemplateRanges(sourceFile: ts.SourceFile): Array<{ start: number; end: number }> { + const ranges: Array<{ start: number; end: number }> = []; + + forEachClass(sourceFile, (node) => { + const decorator = ts + .getDecorators(node) + ?.find( + (dec) => + ts.isCallExpression(dec.expression) && + ts.isIdentifier(dec.expression.expression) && + dec.expression.expression.text === 'Component' + ); + + if (!decorator || !ts.isCallExpression(decorator.expression)) return; + + const [arg] = decorator.expression.arguments; + + if (!arg || !ts.isObjectLiteralExpression(arg)) return; + + for (const prop of arg.properties) { + if ( + ts.isPropertyAssignment(prop) && + (ts.isIdentifier(prop.name) || ts.isStringLiteralLike(prop.name)) && + prop.name.text === 'template' && + ts.isStringLiteralLike(prop.initializer) && + prop.initializer.text + ) { + // +1 / -1 to exclude the opening/closing quote characters. + ranges.push({ start: prop.initializer.getStart(sourceFile) + 1, end: prop.initializer.getEnd() - 1 }); + } + } + }); + + return ranges; +} + +/** Pass B (inline) — rewrite splitter ref reads inside inline component templates. */ +async function migrateInlineTemplates(content: string, fileName: string): Promise { + const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS); + const ranges = collectInlineTemplateRanges(sourceFile).sort((a, b) => b.start - a.start); + let result = content; + + for (const { start, end } of ranges) { + const { content: rewritten, changed } = await migrateTemplate(result.slice(start, end)); + + if (changed) { + result = result.slice(0, start) + rewritten + result.slice(end); + } + } + + return result; +} + +function logWarnings(context: SchematicContext, filePath: string, content: string): void { + for (const { anchor, pattern, message } of warnPatterns) { + if (!new RegExp(anchor).test(content) || !new RegExp(pattern).test(content)) continue; + + logMessage(context.logger, [`${LABEL} ${filePath}`, ` ${message}`]); + } +} + +/** + * A `.ts` file is a splitter consumer if it names any of the exported symbols, imports the package, or renders + * the element in an inline template — a component that only imports `KbqSplitterComponentModule` names no type. + */ +function referencesSplitterComponent(content: string): boolean { + return ( + /\bKbqSplitterComponent\w*\b/.test(content) || + content.includes(SPLITTER_PACKAGE) || + content.includes(`<${SPLITTER_ELEMENT}`) + ); +} + +export default function splitterSignals(options: Schema): Rule { + return async (tree: Tree, context: SchematicContext) => { + const { project, fix } = options; + const projectDefinition = await setupOptions(project, tree); + const root = projectDefinition?.root ?? ''; + const rootDir = root ? tree.getDir(root as Path) : tree.root; + + const tsPaths: string[] = []; + const htmlPaths: string[] = []; + + rootDir.visit((filePath) => { + if (filePath.includes('node_modules') || filePath.includes('/dist/')) return; + + if (filePath.endsWith(TS_EXT)) tsPaths.push(filePath); + else if (filePath.endsWith(HTML_EXT)) htmlPaths.push(filePath); + }); + + let touched = 0; + let consumers = 0; + + const commit = (filePath: string, original: string, updated: string) => { + if (updated === original) return; + + touched++; + + if (fix) { + tree.overwrite(filePath, updated); + } else { + logMessage(context.logger, [`${LABEL} would update ${filePath} (run with --fix to apply)`]); + } + }; + + for (const filePath of tsPaths) { + const original = tree.read(filePath)?.toString(); + + if (!original || !referencesSplitterComponent(original)) continue; + + consumers++; + + logWarnings(context, filePath, original); + warnReceiverMembers(context, filePath, original); + + let content = migrateTsExpressions(original, filePath); + + content = await migrateInlineTemplates(content, filePath); + + commit(filePath, original, content); + } + + for (const filePath of htmlPaths) { + const original = tree.read(filePath)?.toString(); + + if (!original) continue; + + const { content, changed } = await migrateTemplate(original); + + if (changed) { + consumers++; + commit(filePath, original, content); + } + } + + // Nothing here uses the splitter, so the summary would only be noise. + if (consumers === 0) return; + + logMessage(context.logger, [ + `${LABEL} processed tree under "${root || ''}", ` + + `${fix ? 'updated' : 'would update'} ${touched} file(s).`, + ...SUMMARY + ]); + }; +} diff --git a/packages/schematics/src/migrations/splitter-signals/schema.json b/packages/schematics/src/migrations/splitter-signals/schema.json new file mode 100644 index 0000000000..26a5943ff7 --- /dev/null +++ b/packages/schematics/src/migrations/splitter-signals/schema.json @@ -0,0 +1,20 @@ +{ + "$schema": "http://json-schema.org/schema", + "$id": "koobiq-components-splitter-signals", + "title": "Koobiq components splitter signals migration", + "type": "object", + "properties": { + "project": { + "type": "string", + "description": "Name of the project to migrate. If omitted, the migration runs over the whole tree.", + "$default": { + "$source": "projectName" + } + }, + "fix": { + "type": "boolean", + "default": true, + "description": "When true, applies all auto-fix replacements. When false, prints what would change without writing." + } + } +} diff --git a/packages/schematics/src/migrations/splitter-signals/schema.ts b/packages/schematics/src/migrations/splitter-signals/schema.ts new file mode 100644 index 0000000000..b9f56acac5 --- /dev/null +++ b/packages/schematics/src/migrations/splitter-signals/schema.ts @@ -0,0 +1,6 @@ +export interface Schema { + /** Name of the project to migrate. */ + project?: string; + /** When true, applies replacements; when false, only logs what would change. */ + fix: boolean; +} diff --git a/tools/public_api_guard/components/splitter.api.md b/tools/public_api_guard/components/splitter.api.md index cefaf98d29..5d11d7e6ae 100644 --- a/tools/public_api_guard/components/splitter.api.md +++ b/tools/public_api_guard/components/splitter.api.md @@ -6,15 +6,12 @@ import { AfterContentInit } from '@angular/core'; import { AfterViewInit } from '@angular/core'; -import { ChangeDetectorRef } from '@angular/core'; -import { ElementRef } from '@angular/core'; -import * as i0 from '@angular/core'; +import * as _angular_core from '@angular/core'; import * as i1 from '@koobiq/components/icon'; import { OnDestroy } from '@angular/core'; -import { OnInit } from '@angular/core'; -import { QueryList } from '@angular/core'; +import { Signal } from '@angular/core'; -// @public (undocumented) +// @public export enum Direction { // (undocumented) Horizontal = "horizontal", @@ -22,34 +19,23 @@ export enum Direction { Vertical = "vertical" } -// @public (undocumented) -export class KbqGutterDirective implements OnInit { - // (undocumented) - get direction(): Direction; - set direction(direction: Direction); - // (undocumented) - dragged: boolean; +// @public +export class KbqGutterDirective { + constructor(); + readonly direction: _angular_core.InputSignal; + readonly dragged: _angular_core.WritableSignal; // Warning: (ae-forgotten-export) The symbol "IPoint" needs to be exported by the entry point index.d.ts - // - // (undocumented) getPosition(): IPoint; + readonly isVertical: Signal; + readonly order: _angular_core.InputSignalWithTransform; + readonly size: _angular_core.InputSignalWithTransform; // (undocumented) - get isVertical(): boolean; - // (undocumented) - ngOnInit(): void; + static ɵdir: _angular_core.ɵɵDirectiveDeclaration; // (undocumented) - get order(): number; - set order(order: number); - // (undocumented) - get size(): number; - set size(size: number); - // (undocumented) - static ɵdir: i0.ɵɵDirectiveDeclaration; - // (undocumented) - static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵfac: _angular_core.ɵɵFactoryDeclaration; } -// @public (undocumented) +// @public export class KbqGutterGhostDirective { // (undocumented) get direction(): Direction; @@ -68,104 +54,65 @@ export class KbqGutterGhostDirective { get y(): number; set y(y: number); // (undocumented) - static ɵdir: i0.ɵɵDirectiveDeclaration; + static ɵdir: _angular_core.ɵɵDirectiveDeclaration; // (undocumented) - static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵfac: _angular_core.ɵɵFactoryDeclaration; } -// @public (undocumented) +// @public export class KbqSplitterAreaDirective implements AfterViewInit, OnDestroy { - // (undocumented) disableFlex(): void; - // (undocumented) getMinSize(): number; - // (undocumented) getPosition(): IPoint; - // (undocumented) getSize(): number; - // (undocumented) - isResizing(): boolean; + protected readonly isResizing: Signal; // (undocumented) ngAfterViewInit(): void; // (undocumented) ngOnDestroy(): void; - // (undocumented) setOrder(order: number): void; - // (undocumented) setSize(size: number): void; + readonly sizeChange: _angular_core.OutputEmitterRef; // (undocumented) - readonly sizeChange: i0.OutputEmitterRef; + static ɵdir: _angular_core.ɵɵDirectiveDeclaration; // (undocumented) - static ɵdir: i0.ɵɵDirectiveDeclaration; - // (undocumented) - static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵfac: _angular_core.ɵɵFactoryDeclaration; } -// @public (undocumented) -export class KbqSplitterComponent implements OnInit, AfterContentInit, OnDestroy { - // (undocumented) +// @public +export class KbqSplitterComponent implements AfterContentInit, OnDestroy { + constructor(); addArea(area: KbqSplitterAreaDirective): void; - // (undocumented) - areaRefs: QueryList; // Warning: (ae-forgotten-export) The symbol "IArea" needs to be exported by the entry point index.d.ts - // - // (undocumented) - areas: IArea[]; - // (undocumented) - changeDetectorRef: ChangeDetectorRef; - // (undocumented) - get direction(): Direction; - set direction(direction: Direction); - // (undocumented) - get disabled(): boolean; - set disabled(disabled: boolean); - // (undocumented) - elementRef: ElementRef; - // (undocumented) - readonly ghost: i0.Signal; - // (undocumented) - readonly gutterPositionChange: i0.OutputEmitterRef; - // (undocumented) - readonly gutters: i0.Signal; - // (undocumented) - get gutterSize(): number; - set gutterSize(gutterSize: number); - // (undocumented) - get hideGutters(): boolean; - set hideGutters(value: boolean); - // (undocumented) - get isDragging(): boolean; - // (undocumented) - isVertical(): boolean; + protected areas: IArea[]; + readonly direction: _angular_core.InputSignal; + readonly disabled: _angular_core.InputSignalWithTransform; + readonly gutterPositionChange: _angular_core.OutputEmitterRef; + readonly gutterSize: _angular_core.InputSignalWithTransform; + readonly hideGutters: _angular_core.InputSignalWithTransform; + readonly isDragging: Signal; + readonly isVertical: Signal; // (undocumented) ngAfterContentInit(): void; // (undocumented) ngOnDestroy(): void; - // (undocumented) - ngOnInit(): void; - // (undocumented) - onMouseDown(event: MouseEvent, leftAreaIndex: number, rightAreaIndex: number): void; - // (undocumented) + protected onMouseDown(event: MouseEvent, leftAreaIndex: number, rightAreaIndex: number): void; removeArea(area: KbqSplitterAreaDirective): void; + readonly useGhost: _angular_core.InputSignalWithTransform; // (undocumented) - get resizing(): boolean; - // (undocumented) - get useGhost(): boolean; - set useGhost(useGhost: boolean); - // (undocumented) - static ɵcmp: i0.ɵɵComponentDeclaration; + static ɵcmp: _angular_core.ɵɵComponentDeclaration; // (undocumented) - static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵfac: _angular_core.ɵɵFactoryDeclaration; } // @public (undocumented) export class KbqSplitterModule { // (undocumented) - static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵfac: _angular_core.ɵɵFactoryDeclaration; // (undocumented) - static ɵinj: i0.ɵɵInjectorDeclaration; + static ɵinj: _angular_core.ɵɵInjectorDeclaration; // (undocumented) - static ɵmod: i0.ɵɵNgModuleDeclaration; + static ɵmod: _angular_core.ɵɵNgModuleDeclaration; } // (No @packageDocumentation comment for this package) From 22f8d06830ef3ff68a75c17d40a771f81804bdbe Mon Sep 17 00:00:00 2001 From: Artem Belik Date: Thu, 3 Sep 2026 18:13:42 +0300 Subject: [PATCH 2/2] fix(splitter): address the review of the component review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `updateGutter` ran a synchronous change-detection pass for each gutter it reset. `dragged` is a signal read from the gutter's own host binding now, so writing it marks the view dirty on its own, and `onMouseUp` calls `markForCheck()` right after. The class had no test at all, which is why the leftover went unnoticed. The schematic never counted a template-only consumer, so the booleanAttribute note — the change that flips what such a template does — went unprinted. `SPLITTER_TYPE` was left exported with nothing importing it. Co-Authored-By: Claude Opus 5 --- .../components/splitter/splitter.component.ts | 8 +------ packages/components/splitter/splitter.spec.ts | 24 +++++++++++++++++++ .../src/migrations/splitter-signals/data.ts | 3 --- .../migrations/splitter-signals/index.spec.ts | 10 ++++++++ .../src/migrations/splitter-signals/index.ts | 5 ++-- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/packages/components/splitter/splitter.component.ts b/packages/components/splitter/splitter.component.ts index 4e1c46af1d..8cb9db451f 100644 --- a/packages/components/splitter/splitter.component.ts +++ b/packages/components/splitter/splitter.component.ts @@ -405,13 +405,7 @@ export class KbqSplitterComponent implements AfterContentInit, OnDestroy { }; private updateGutter(): void { - this.gutters().forEach((gutter) => { - if (gutter.dragged()) { - gutter.dragged.set(false); - - this.changeDetectorRef.detectChanges(); - } - }); + this.gutters().forEach((gutter) => gutter.dragged.set(false)); } private onMouseMove( diff --git a/packages/components/splitter/splitter.spec.ts b/packages/components/splitter/splitter.spec.ts index 190dfbc66a..50533e0012 100644 --- a/packages/components/splitter/splitter.spec.ts +++ b/packages/components/splitter/splitter.spec.ts @@ -237,6 +237,30 @@ describe('KbqSplitter', () => { expect(ghost.nativeElement.classList.contains('kbq-gutter-ghost_visible')).toBe(false); })); + it('should toggle the dragged class on the gutter', fakeAsync(() => { + const fixture = createTestComponent(KbqSplitterGhost); + + fixture.detectChanges(); + + tick(); + + const gutter = fixture.debugElement.query(By.directive(KbqGutterDirective)); + + expect(gutter.nativeElement.classList.contains('kbq-gutter_dragged')).toBe(false); + + gutter.nativeElement.dispatchEvent(new MouseEvent('mousedown', { screenX: 0, screenY: 0 })); + + fixture.detectChanges(); + + expect(gutter.nativeElement.classList.contains('kbq-gutter_dragged')).toBe(true); + + document.dispatchEvent(new Event('mouseup')); + + fixture.detectChanges(); + + expect(gutter.nativeElement.classList.contains('kbq-gutter_dragged')).toBe(false); + })); + it('should not resize areas while ghost is being dragged', fakeAsync(() => { const fixture = createTestComponent(KbqSplitterGhost); diff --git a/packages/schematics/src/migrations/splitter-signals/data.ts b/packages/schematics/src/migrations/splitter-signals/data.ts index cc68e32354..7673205e76 100644 --- a/packages/schematics/src/migrations/splitter-signals/data.ts +++ b/packages/schematics/src/migrations/splitter-signals/data.ts @@ -34,9 +34,6 @@ export const SIGNAL_MEMBERS: readonly string[] = [ */ export const WRITABLE_MEMBERS: ReadonlySet = new Set(['dragged']); -/** TypeScript type annotation that marks a receiver as a splitter. */ -export const SPLITTER_TYPE = 'KbqSplitterComponent'; - /** Every type whose members this migration rewrites. */ export const RECEIVER_TYPES: readonly string[] = ['KbqSplitterComponent', 'KbqGutterDirective']; diff --git a/packages/schematics/src/migrations/splitter-signals/index.spec.ts b/packages/schematics/src/migrations/splitter-signals/index.spec.ts index 6ad0b208c2..496baef48b 100644 --- a/packages/schematics/src/migrations/splitter-signals/index.spec.ts +++ b/packages/schematics/src/migrations/splitter-signals/index.spec.ts @@ -279,6 +279,16 @@ describe(SCHEMATIC_NAME, () => { expect(summary.match(/booleanAttribute/g)!.length).toBe(1); }); + it('reports the summary for a template-only consumer with nothing to rewrite', async () => { + const html = firstHtmlPath(); + + appTree.overwrite(html, '
\n'); + + await run(); + + expect(messages.join('\n')).toContain('booleanAttribute'); + }); + it('stays silent for a workspace that does not use the splitter', async () => { await run(); diff --git a/packages/schematics/src/migrations/splitter-signals/index.ts b/packages/schematics/src/migrations/splitter-signals/index.ts index 4da7c96ee6..b0916f93b9 100644 --- a/packages/schematics/src/migrations/splitter-signals/index.ts +++ b/packages/schematics/src/migrations/splitter-signals/index.ts @@ -441,12 +441,13 @@ export default function splitterSignals(options: Schema): Rule { for (const filePath of htmlPaths) { const original = tree.read(filePath)?.toString(); - if (!original) continue; + if (!original || !original.includes(`<${SPLITTER_ELEMENT}`)) continue; + + consumers++; const { content, changed } = await migrateTemplate(original); if (changed) { - consumers++; commit(filePath, original, content); } }