diff --git a/docs/guides/migration.en.md b/docs/guides/migration.en.md index ccb9a89267..4f93e87c45 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: @@ -1049,6 +1049,24 @@ Most of them report rather than rewrite: what replaces a removed member or a sig ng g @koobiq/components: --project ``` +#### Description list + +`KbqDlComponent` was already fully signal-based, so the review had nothing to migrate. What it found were the five inputs that never got a coercion transform, sitting next to siblings that had one: + +```ts +readonly verticalBreakpoint = input(400, { transform: numberAttribute }); +readonly minWidth = input(); // no transform +readonly wide = input(false); // no transform +``` + +**`` and `` used to do nothing.** A valueless attribute passes the empty string, which is falsy — while `` right next to it worked. Both are coerced now and the attribute means true. + +`vertical` is tri-state: `null` means "decide from `verticalBreakpoint`". `booleanAttribute` would have folded that into `false`, so it uses a transform that preserves `null`. + +**`minWidth`, `dtMinWidth` and `ddMinWidth` are numeric inputs** reporting `number | undefined`, which is what an unbound description list always held. A static attribute used to reach the layout arithmetic as a string, which coerced in a comparison but not in `Math.max`. + +Reported by `dl-attribute-coercion`. + #### Popover Hover mode was broken end to end by a dead expression. `this.leaveDelay ?? 500` looks like a default, but the base class sets the field to `0`, and `0 ?? 500` is `0` — so the panel closed before the pointer could cross the 8px gap to it, the documented interactive content was unreachable even for pointer users, and the auto-hide watchdog spun as an `interval(0)` for as long as the panel stayed open. diff --git a/docs/guides/migration.ru.md b/docs/guides/migration.ru.md index 73fd0cd51d..67f78f4396 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; ко второй относится каждый из подразделов ниже. Каждое ревью закрывало члены, которые никогда не были частью контракта компонента, переводило входы на сигналы там, где в этом и был его смысл, и попутно исправляло найденные ошибки поведения. Ниже перечислено только то, что доходит до потребителя. Все схематики, названные ниже, запускаются автоматически: @@ -1053,6 +1053,24 @@ ng update @koobiq/components@20 ng g @koobiq/components: --project ``` +#### Description list + +`KbqDlComponent` уже был полностью на сигналах, поэтому мигрировать в нём было нечего. Ревью нашло другое: пять входов, которым так и не досталось приведение типов, — рядом с соседями, у которых оно есть: + +```ts +readonly verticalBreakpoint = input(400, { transform: numberAttribute }); +readonly minWidth = input(); // без трансформации +readonly wide = input(false); // без трансформации +``` + +**`` и `` раньше ничего не делали.** Атрибут без значения передаёт пустую строку, которая ложна, — при том что `` по соседству работал. Теперь оба приводятся, и атрибут означает `true`. + +`vertical` трёхзначный: `null` означает «решай по `verticalBreakpoint`». `booleanAttribute` свернул бы это в `false`, поэтому используется трансформация, сохраняющая `null`. + +**`minWidth`, `dtMinWidth` и `ddMinWidth` стали числовыми входами** и возвращают `number | undefined` — именно это и содержал список без привязок. Статический атрибут раньше доходил до арифметики раскладки строкой, которая приводилась к числу в сравнении, но не в `Math.max`. + +Сообщается схематиком `dl-attribute-coercion`. + #### Popover Режим по наведению был сломан целиком из-за мёртвого выражения. `this.leaveDelay ?? 500` выглядит как значение по умолчанию, но базовый класс присваивает полю `0`, а `0 ?? 500` — это `0`. Панель закрывалась раньше, чем указатель успевал пересечь зазор в 8px до неё, задокументированное интерактивное содержимое было недостижимо даже для мыши, а таймер автоматического скрытия крутился как `interval(0)` всё время, пока панель открыта. diff --git a/packages/components/dl/dl.component.spec.ts b/packages/components/dl/dl.component.spec.ts index d7696c39fe..6131648893 100644 --- a/packages/components/dl/dl.component.spec.ts +++ b/packages/components/dl/dl.component.spec.ts @@ -1,11 +1,13 @@ import { FocusMonitor } from '@angular/cdk/a11y'; import { Directionality } from '@angular/cdk/bidi'; import { SharedResizeObserver } from '@angular/cdk/observers/private'; -import { Injectable, Provider, Type } from '@angular/core'; +import { Component, Injectable, Provider, Type } from '@angular/core'; import { ComponentFixture, fakeAsync, flush, TestBed, tick } from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; import { enUSLocaleData, kbqA11yLocaleConfigurationProvider } from '@koobiq/components/core'; import { BehaviorSubject, EMPTY, Observable } from 'rxjs'; import { KbqDlAlign, KbqDlComponent } from './dl.component'; +import { KbqDlModule } from './dl.module'; /** `SharedResizeObserver` stand-in: the real one never emits in jsdom, where `ResizeObserver` is a no-op stub. */ @Injectable() @@ -626,4 +628,123 @@ describe(KbqDlComponent.name, () => { expect(getDlElement(fixture).classList).toContain('kbq-dl_vertical'); flush(); })); + it('should re-evaluate the layout when the breakpoint changes', fakeAsync(() => { + const fixture = createComponent(KbqDlComponent); + + Object.defineProperty(getDlElement(fixture), 'getClientRects', { + configurable: true, + value: () => [{ width: 600 } as DOMRect] + }); + + tick(100); + fixture.detectChanges(); + + expect(getDlElement(fixture).classList).not.toContain('kbq-dl_vertical'); + + // The comparison used to run only on resize, so a new breakpoint left the old answer in place. + fixture.componentRef.setInput('verticalBreakpoint', 700); + fixture.detectChanges(); + + expect(getDlElement(fixture).classList).toContain('kbq-dl_vertical'); + flush(); + })); + + it('should hand the decision back to the breakpoint when vertical returns to null', fakeAsync(() => { + const fixture = createComponent(KbqDlComponent); + + Object.defineProperty(getDlElement(fixture), 'getClientRects', { + configurable: true, + value: () => [{ width: 600 } as DOMRect] + }); + + fixture.componentRef.setInput('verticalBreakpoint', 700); + tick(100); + fixture.detectChanges(); + + expect(getDlElement(fixture).classList).toContain('kbq-dl_vertical'); + + fixture.componentRef.setInput('vertical', false); + fixture.detectChanges(); + + expect(getDlElement(fixture).classList).not.toContain('kbq-dl_vertical'); + + fixture.componentRef.setInput('vertical', null); + fixture.detectChanges(); + + expect(getDlElement(fixture).classList).toContain('kbq-dl_vertical'); + flush(); + })); + + it('should treat a valueless wide attribute as true', () => { + const fixture = createComponent(DlWithValuelessAttributes); + + fixture.detectChanges(); + + const dl = fixture.debugElement.query(By.directive(KbqDlComponent)).nativeElement as HTMLElement; + + expect(dl.classList).toContain('kbq-dl_wide'); + }); + + it('should keep null as the vertical default', () => { + const fixture = createComponent(KbqDlComponent); + + fixture.detectChanges(); + + // `null` is the "decide from the breakpoint" state, so it must survive the transform. + expect(fixture.componentInstance.vertical()).toBeNull(); + }); + + it('should treat a valueless vertical attribute as true', () => { + const fixture = createComponent(DlWithValuelessAttributes); + + fixture.detectChanges(); + + const dl = fixture.debugElement.query(By.directive(KbqDlComponent)).nativeElement as HTMLElement; + + expect(dl.classList).toContain('kbq-dl_vertical'); + }); + + it('should coerce the numeric width attributes', () => { + const fixture = createComponent(DlWithStringWidths); + + fixture.detectChanges(); + + const dl = fixture.debugElement.query(By.directive(KbqDlComponent)).componentInstance as KbqDlComponent; + + expect(dl.minWidth()).toBe(700); + expect(dl.dtMinWidth()).toBe(120); + expect(dl.ddMinWidth()).toBe(80); + }); + + it('should leave the optional widths undefined when unbound', () => { + const fixture = createComponent(KbqDlComponent); + + fixture.detectChanges(); + + expect(fixture.componentInstance.minWidth()).toBeUndefined(); + expect(fixture.componentInstance.dtMinWidth()).toBeUndefined(); + expect(fixture.componentInstance.ddMinWidth()).toBeUndefined(); + }); }); + +@Component({ + imports: [KbqDlModule], + template: ` + + term + description + + ` +}) +class DlWithValuelessAttributes {} + +@Component({ + imports: [KbqDlModule], + template: ` + + term + description + + ` +}) +class DlWithStringWidths {} diff --git a/packages/components/dl/dl.component.ts b/packages/components/dl/dl.component.ts index 6677d36ad0..cef9bdc813 100644 --- a/packages/components/dl/dl.component.ts +++ b/packages/components/dl/dl.component.ts @@ -11,12 +11,15 @@ import { computed, contentChildren, DestroyRef, + effect, ElementRef, inject, + Injector, input, model, numberAttribute, signal, + untracked, viewChild, ViewEncapsulation } from '@angular/core'; @@ -25,6 +28,10 @@ import { KBQ_WINDOW, kbqInjectA11yLocaleConfiguration, kbqInjectNativeElement } import { KbqResizable, KbqResizer, KbqResizerDirection, KbqResizerSizeChangeEvent } from '@koobiq/components/resizer'; import { debounceTime, startWith } from 'rxjs/operators'; +/** Coerces an optional numeric input, keeping `undefined` distinguishable from `0`. */ +const optionalNumberAttribute = (value: unknown): number | undefined => + value == null ? undefined : numberAttribute(value); + /** Supported alignment values for description list items. */ export type KbqDlAlign = 'start' | 'center' | 'end'; @@ -110,10 +117,10 @@ export class KbqDlComponent { * @deprecated The name is misleading (it is a breakpoint, not a min width). Use `verticalBreakpoint` instead. * Will be removed in a future major release. When both are set, `minWidth` takes precedence. */ - readonly minWidth = input(); + readonly minWidth = input(undefined, { transform: optionalNumberAttribute }); /** Whether the list uses the wide two-column layout. */ - readonly wide = input(false); + readonly wide = input(false, { transform: booleanAttribute }); /** Whether the `kbq-dt` area can be resized by dragging the separator. */ readonly resizable = input(false, { transform: booleanAttribute }); @@ -122,10 +129,10 @@ export class KbqDlComponent { readonly dtWidth = model(null); /** Minimum width of the `kbq-dt` area in pixels; defaults to the rendered term width. */ - readonly dtMinWidth = input(undefined); + readonly dtMinWidth = input(undefined, { transform: optionalNumberAttribute }); /** Minimum width retained for the `kbq-dd` area in pixels; defaults to the rendered term width. */ - readonly ddMinWidth = input(undefined); + readonly ddMinWidth = input(undefined, { transform: optionalNumberAttribute }); /** Accessible name of the column resize separator; falls back to the localized default when omitted. */ readonly resizerAriaLabel = input(undefined); @@ -137,7 +144,10 @@ export class KbqDlComponent { readonly horizontalAlign = input('start'); /** Forces the vertical layout; `null` lets the list decide based on `verticalBreakpoint`. */ - readonly vertical = input(null); + readonly vertical = input(null, { + // Not `booleanAttribute`: it would fold `null` — the "decide for me" state — into `false`. + transform: (value) => (value == null ? null : booleanAttribute(value)) + }); /** @docs-private */ protected readonly resizeDirection = signal([1, 0]); @@ -194,6 +204,7 @@ export class KbqDlComponent { private readonly platform = inject(Platform); private readonly window = inject(KBQ_WINDOW); private readonly destroyRef = inject(DestroyRef); + private readonly injector = inject(Injector); private readonly resizeObserver = inject(SharedResizeObserver); private readonly directionality = inject(Directionality, { optional: true }); private readonly focusMonitor = inject(FocusMonitor); @@ -217,10 +228,34 @@ export class KbqDlComponent { afterNextRender(() => { this.measureDtWidth(); + // The resize subscription owns the first measurement: it is debounced, so the host has been + // laid out by the time it runs. The effect below defers to it rather than measuring a host + // that has no box yet. + let layoutMeasured = false; + this.resizeObserver .observe(this.nativeElement) .pipe(startWith(null), debounceTime(this.resizeDebounceInterval), takeUntilDestroyed(this.destroyRef)) - .subscribe(() => this.updateLayout()); + .subscribe(() => { + layoutMeasured = true; + this.updateLayout(); + }); + + // The breakpoint comparison was only ever re-run on resize, so changing any of its three + // inputs left the layout on the answer computed for the previous ones — including `vertical` + // going back to null, which hands the decision back to a stale `autoVertical`. + effect( + () => { + this.vertical(); + this.verticalBreakpoint(); + this.minWidth(); + + if (!layoutMeasured) return; + + untracked(() => this.updateLayout()); + }, + { injector: this.injector } + ); }); } diff --git a/packages/schematics/src/collection.json b/packages/schematics/src/collection.json index c5131fd7d8..ee8791144d 100644 --- a/packages/schematics/src/collection.json +++ b/packages/schematics/src/collection.json @@ -158,6 +158,11 @@ "description": "Reports the breaking parts of the toast review — the members KbqToastComponent lost when it moved from KbqToastService to the KBQ_TOAST_STACK token, the narrowed animation subject and showTemplate context — and notes that the data passed to show() is no longer written to, so its style and icon defaults are resolved inside the toast", "factory": "./migrations/toast-stack-and-defaults/index", "schema": "./migrations/toast-stack-and-defaults/schema.json" + }, + "dl-attribute-coercion": { + "description": "Reports the attributes whose coercion changed in the description list review. KbqDlComponent was already fully signal-based; five inputs never got a transform, so a static attribute reached them as a string. A valueless wide or vertical attribute passed the empty string, which is falsy, so it did nothing - both are coerced now and mean true. minWidth, dtMinWidth and ddMinWidth are numeric inputs reporting number | undefined. vertical keeps null as its default, the state that lets the list decide from verticalBreakpoint, so it uses a transform that preserves null rather than booleanAttribute. Warn-only: every one of these is a template change the call site owns.", + "factory": "./migrations/dl-attribute-coercion/index", + "schema": "./migrations/dl-attribute-coercion/schema.json" } } } diff --git a/packages/schematics/src/migrations.json b/packages/schematics/src/migrations.json index c4fe76dcc6..ae0d987719 100644 --- a/packages/schematics/src/migrations.json +++ b/packages/schematics/src/migrations.json @@ -120,6 +120,11 @@ "version": "20.3.0-0", "description": "Reports the breaking parts of the toast review. KbqToastComponent resolves its stack through the new KBQ_TOAST_STACK token instead of KbqToastService, so `service` is gone together with `elementRef`, `ttl`, `delay`, `isTemplateRef()`, `themePalette` and `toastStyle`, and everything the template renders became protected; KbqToastService.animation was narrowed from BehaviorSubject to Subject, so `.getValue()` / `.value` no longer exist; showTemplate() and `templates` return EmbeddedViewRef. Nothing is rewritten — a subclass has to be rewired by hand. Also notes the changes no call site can point at: the data passed to show() is no longer written to (the `style` and `icon` defaults are resolved inside the toast, so anything rendering the same object needs its own), hover and focus pause the shared countdown while a showTemplate() toast is not paused, every toast is a role=\"alert\"/\"status\" live region inside a labelled role=\"region\" stack named by the new `toastRegion` a11y locale key, and focus is handed on only for a keyboard dismissal.", "factory": "./migrations/toast-stack-and-defaults/index" + }, + "dl-attribute-coercion": { + "version": "20.3.0-0", + "description": "Reports the attributes whose coercion changed in the description list review. KbqDlComponent was already fully signal-based; five inputs never got a transform, so a static attribute reached them as a string. A valueless wide or vertical attribute passed the empty string, which is falsy, so it did nothing - both are coerced now and mean true. minWidth, dtMinWidth and ddMinWidth are numeric inputs reporting number | undefined. vertical keeps null as its default, the state that lets the list decide from verticalBreakpoint, so it uses a transform that preserves null rather than booleanAttribute. Warn-only: every one of these is a template change the call site owns.", + "factory": "./migrations/dl-attribute-coercion/index" } } } diff --git a/packages/schematics/src/migrations/dl-attribute-coercion/README.md b/packages/schematics/src/migrations/dl-attribute-coercion/README.md new file mode 100644 index 0000000000..9a2553bbc6 --- /dev/null +++ b/packages/schematics/src/migrations/dl-attribute-coercion/README.md @@ -0,0 +1,42 @@ +# dl-attribute-coercion + +Migration schematic invoked automatically by `ng update @koobiq/components@20` (registered for +`20.3.0-0`). Reports the `` attributes whose coercion changed. It never writes to the tree. + +## Background + +`KbqDlComponent` was already fully signal-based, so the review had nothing to migrate. What it found +were the five inputs that never got a coercion transform, next to siblings that had one: + +```ts +readonly verticalBreakpoint = input(400, { transform: numberAttribute }); +readonly minWidth = input(); // no transform +readonly wide = input(false); // no transform +``` + +So `` passed the empty string — falsy — and the attribute did nothing, while +`` right next to it worked. + +`vertical` is tri-state: `null` means "decide from `verticalBreakpoint`". `booleanAttribute` would +have folded that into `false`, so it uses a transform that preserves `null`. + +## What it does _not_ do + +Nothing is rewritten. Whether markup relied on a valueless `wide` being ignored is a decision the +call site owns, and so is what a non-numeric width was meant to say. + +| Pattern | Manual migration | +| ------------------------------------------------- | --------------------------------------------------------- | +| `` / `` | Remove the attribute if it was meant to do nothing | +| `` and the two `*MinWidth` | The value is a number now; a non-numeric one is undefined | + +## Notes with no call site to point at + +- `minWidth`, `dtMinWidth` and `ddMinWidth` report `number | undefined`, which is what an unbound + description list always held. + +## Running it manually + +``` +ng generate @koobiq/components:dl-attribute-coercion --project my-app +``` diff --git a/packages/schematics/src/migrations/dl-attribute-coercion/data.ts b/packages/schematics/src/migrations/dl-attribute-coercion/data.ts new file mode 100644 index 0000000000..307e1a8aa4 --- /dev/null +++ b/packages/schematics/src/migrations/dl-attribute-coercion/data.ts @@ -0,0 +1,53 @@ +/** + * Data for the `dl-attribute-coercion` migration. + * + * `KbqDlComponent` was already fully signal-based; the review found the five inputs that never got a + * coercion transform, so a static attribute reached them as a string. + * + * - `` passed the empty string, which is falsy — the attribute did nothing + * - `` did the same, and `vertical` is tri-state, so `booleanAttribute` alone would have + * folded its `null` "decide from the breakpoint" state into `false` + * - `minWidth`, `dtMinWidth` and `ddMinWidth` reached the layout arithmetic as strings + * + * Warn-only: every one of these is a template change, and which markup relied on the old reading is a + * decision the call site owns. + */ + +/** Import specifier that marks a file as a description list consumer. */ +export const DL_PACKAGE = '@koobiq/components/dl'; + +/** Element selector of the description list. */ +export const DL_ELEMENT = 'kbq-dl'; + +export interface WarnPattern { + /** Only evaluated for files that also render the element. */ + pattern: string; + message: string; +} + +export const warnPatterns: WarnPattern[] = [ + { + // A valueless `wide` or `vertical`: the attribute name with no `=` and no `[` prefix. + pattern: ']*\\s(?:wide|vertical)(?![\\w-]*\\s*=)', + message: + 'A valueless `wide` or `vertical` attribute on used to pass the empty string, which is ' + + 'falsy, so it did nothing. Both are coerced now and the attribute means true. Remove it if the ' + + 'markup was relying on it being ignored.' + }, + { + pattern: ']*\\s(?:minWidth|dtMinWidth|ddMinWidth)\\s*=\\s*"[^"{]', + message: + '`minWidth`, `dtMinWidth` and `ddMinWidth` on are numeric inputs now. A static attribute ' + + 'used to reach the layout arithmetic as a string, which happened to coerce in a comparison but ' + + 'not in `Math.max`. The value is a number now; a non-numeric one reads as undefined.' + } +]; + +/** Printed once per project, after the per-file reports. */ +export const SUMMARY = [ + ' `vertical` keeps `null` as its default — the state that lets the list decide from ' + + '`verticalBreakpoint` — so it is coerced with a transform that preserves null rather than with ' + + '`booleanAttribute`, which would have folded it into false.', + ' `minWidth`, `dtMinWidth` and `ddMinWidth` report `number | undefined`, which is what an unbound ' + + 'description list always held.' +]; diff --git a/packages/schematics/src/migrations/dl-attribute-coercion/index.spec.ts b/packages/schematics/src/migrations/dl-attribute-coercion/index.spec.ts new file mode 100644 index 0000000000..129b14af28 --- /dev/null +++ b/packages/schematics/src/migrations/dl-attribute-coercion/index.spec.ts @@ -0,0 +1,100 @@ +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 = 'dl-attribute-coercion'; + +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 firstHtmlPath(): string { + const [first] = projects.keys(); + const root = `/${projects.get(first)!.root}/src/app`; + + return appTree.exists(`${root}/app.html`) ? `${root}/app.html` : `${root}/app.component.html`; + } + + async function run(): Promise { + const [first] = projects.keys(); + + return runner.runSchematic(SCHEMATIC_NAME, { project: first } satisfies Schema, appTree); + } + + it('reports a valueless wide attribute without touching the file', async () => { + const html = firstHtmlPath(); + const source = 'ab\n'; + + appTree.overwrite(html, source); + + expect((await run()).readText(html)).toBe(source); + expect(messages.join('\n')).toContain('valueless `wide` or `vertical`'); + }); + + it('reports a valueless vertical attribute', async () => { + const html = firstHtmlPath(); + + appTree.overwrite(html, '\n'); + + await run(); + + expect(messages.join('\n')).toContain('valueless `wide` or `vertical`'); + }); + + it('reports a static numeric width attribute', async () => { + const html = firstHtmlPath(); + + appTree.overwrite(html, '\n'); + + await run(); + + expect(messages.join('\n')).toContain('numeric inputs now'); + }); + + it('leaves a bound wide alone', async () => { + const html = firstHtmlPath(); + + appTree.overwrite(html, '\n'); + + await run(); + + expect(messages.join('\n')).not.toContain('valueless `wide` or `vertical`'); + }); + + it('prints the summary once for a consumer', async () => { + const html = firstHtmlPath(); + + appTree.overwrite(html, '\n'); + + await run(); + + const summary = messages.join('\n'); + + expect(summary).toContain('number | undefined'); + expect(summary.match(/number \| undefined/g)!.length).toBe(1); + }); + + it('stays silent for a workspace that does not use the description list', async () => { + await run(); + + expect(messages.join('\n')).not.toContain(`[${SCHEMATIC_NAME}]`); + }); +}); diff --git a/packages/schematics/src/migrations/dl-attribute-coercion/index.ts b/packages/schematics/src/migrations/dl-attribute-coercion/index.ts new file mode 100644 index 0000000000..5b00eec352 --- /dev/null +++ b/packages/schematics/src/migrations/dl-attribute-coercion/index.ts @@ -0,0 +1,61 @@ +import { Path } from '@angular-devkit/core'; +import { Rule, SchematicContext, Tree } from '@angular-devkit/schematics'; +import { logMessage } from '../../utils/messages'; +import { setupOptions } from '../../utils/package-config'; +import { DL_ELEMENT, DL_PACKAGE, SUMMARY, warnPatterns } from './data'; +import { Schema } from './schema'; + +const LABEL = '[dl-attribute-coercion]'; +const EXTENSIONS = ['.ts', '.html']; + +/** A file is a description list consumer if it renders the element or imports the package. */ +function referencesDl(content: string): boolean { + return content.includes(`<${DL_ELEMENT}`) || content.includes(DL_PACKAGE); +} + +/** + * Reports the `` attributes whose coercion changed. Never writes: whether markup relied on a + * valueless `wide` being ignored is a decision the call site owns, and so is what a non-numeric width + * was meant to say. + * + * Both `.ts` and `.html` are visited, because the element is written in templates of either kind. + */ +export default function dlAttributeCoercion(options: Schema): Rule { + return async (tree: Tree, context: SchematicContext) => { + const { project } = options; + const projectDefinition = await setupOptions(project, tree); + const root = projectDefinition?.root ?? ''; + const rootDir = root ? tree.getDir(root as Path) : tree.root; + + let consumers = 0; + let reported = 0; + + rootDir.visit((filePath: Path, entry) => { + if (filePath.includes('node_modules') || filePath.includes('/dist/')) return; + if (!EXTENSIONS.some((extension) => filePath.endsWith(extension))) return; + + const content = entry?.content.toString(); + + if (!content || !referencesDl(content)) return; + + consumers++; + + for (const { pattern, message } of warnPatterns) { + if (!new RegExp(pattern).test(content)) continue; + + reported++; + + logMessage(context.logger, [`${LABEL} ${filePath}`, ` ${message}`]); + } + }); + + // Nothing here uses the description list, so the summary would only be noise. + if (consumers === 0) return; + + logMessage(context.logger, [ + `${LABEL} processed kbq-dl under "${root || ''}", ` + + `${consumers} file(s) reference the component, ${reported} call site(s) reported.`, + ...SUMMARY + ]); + }; +} diff --git a/packages/schematics/src/migrations/dl-attribute-coercion/schema.json b/packages/schematics/src/migrations/dl-attribute-coercion/schema.json new file mode 100644 index 0000000000..31adc456fc --- /dev/null +++ b/packages/schematics/src/migrations/dl-attribute-coercion/schema.json @@ -0,0 +1,15 @@ +{ + "$schema": "http://json-schema.org/schema", + "$id": "koobiq-components-dl-attribute-coercion", + "title": "Koobiq components description list review migration", + "type": "object", + "properties": { + "project": { + "type": "string", + "description": "Name of the project to inspect. If omitted, the migration runs over the whole tree.", + "$default": { + "$source": "projectName" + } + } + } +} diff --git a/packages/schematics/src/migrations/dl-attribute-coercion/schema.ts b/packages/schematics/src/migrations/dl-attribute-coercion/schema.ts new file mode 100644 index 0000000000..21f2e52bd6 --- /dev/null +++ b/packages/schematics/src/migrations/dl-attribute-coercion/schema.ts @@ -0,0 +1,4 @@ +export interface Schema { + /** Name of the project to inspect. If omitted, the whole tree is inspected. */ + project?: string; +} diff --git a/tools/public_api_guard/components/dl.api.md b/tools/public_api_guard/components/dl.api.md index a8aa09226b..fb78c942f3 100644 --- a/tools/public_api_guard/components/dl.api.md +++ b/tools/public_api_guard/components/dl.api.md @@ -26,8 +26,8 @@ export class KbqDlComponent { constructor(); // (undocumented) protected get currentDtWidth(): number; - readonly ddMinWidth: _angular_core.InputSignal; - readonly dtMinWidth: _angular_core.InputSignal; + readonly ddMinWidth: _angular_core.InputSignalWithTransform; + readonly dtMinWidth: _angular_core.InputSignalWithTransform; readonly dtWidth: _angular_core.ModelSignal; protected handleDtResize(input: KbqResizerSizeChangeEvent): void; protected handleResizeDblClick(event: MouseEvent): void; @@ -38,7 +38,7 @@ export class KbqDlComponent { // (undocumented) protected get maxDtWidth(): number; // @deprecated - readonly minWidth: _angular_core.InputSignal; + readonly minWidth: _angular_core.InputSignalWithTransform; protected readonly normalizedDtMinWidth: _angular_core.Signal; readonly resizable: _angular_core.InputSignalWithTransform; protected readonly resizeCursor: _angular_core.WritableSignal; @@ -46,10 +46,10 @@ export class KbqDlComponent { readonly resizerAriaLabel: _angular_core.InputSignal; protected readonly resizerVisible: _angular_core.Signal; protected readonly resolvedResizerAriaLabel: _angular_core.Signal; - readonly vertical: _angular_core.InputSignal; + readonly vertical: _angular_core.InputSignalWithTransform; readonly verticalAlign: _angular_core.InputSignal; readonly verticalBreakpoint: _angular_core.InputSignalWithTransform; - readonly wide: _angular_core.InputSignal; + readonly wide: _angular_core.InputSignalWithTransform; // (undocumented) static ɵcmp: _angular_core.ɵɵComponentDeclaration; // (undocumented)