Skip to content

fix(textarea)!: errors following a full review of the component (#DS-5482) - #1990

Draft
artembelik wants to merge 2 commits into
mainfrom
fix/textarea-signals
Draft

fix(textarea)!: errors following a full review of the component (#DS-5482)#1990
artembelik wants to merge 2 commits into
mainfrom
fix/textarea-signals

Conversation

@artembelik

Copy link
Copy Markdown
Contributor

What

A full review of textarea, in the same shape as the 20.3.0 component reviews.

What stays a plain accessor, and why

KbqTextarea implements KbqFormFieldControl, which declares value, id, placeholder, required, disabled, focused, empty and errorState as plain members. That interface is how the form field reads them, and it went through its own review in 20.3.0 — so those stay accessors. What moved are the four inputs the textarea owns.

canGrow disagreed with what was bound

get canGrow(): boolean {
    return !this.maxRowLimitReached && this._canGrow;
}

It reported false once the textarea hit maxRows, even though the consumer had asked for growth. The folded value is what drives the resize handle, so it became an internal growing computed; canGrow() reports what was bound. Same shape as KbqLink.tabIndex in the earlier review.

freeRowsHeight defaulted itself by writing its own input

this.freeRowsHeight = this.freeRowsHeight ?? this.lineHeight;

That is exactly why the automated migration skipped it. The fallback is a computed over the measured line height now, so binding [freeRowsHeight] later actually takes effect instead of being overwritten the next time ngOnInit runs — and it does run again, because the parent animation re-invokes it.

maxRows and freeRowsHeight lied about being required

Both were declared number while an unbound textarea held undefined. maxRowLimitReached compared rowsCount > undefined, which is false — that accident is what made unlimited growth work. They report number | undefined now and the comparison is explicit.

The row-limit class waited for someone else's change detection

rowsCount is written inside runOutsideAngular, so [class.kbq-textarea_max-row-limit-reached] only appeared on the next unrelated change detection pass. It is a signal now and the class follows it directly.

Also

  • The parent.animationDone subscription had no teardown; it is takeUntilDestroyed() now.
  • The generated id comes from the CDK _IdGenerator instead of a module counter, so the shape changes from kbq-textarea-1 to kbq-textarea-a1.
  • this.id = this.id in the constructor — a trick to force the setter — is replaced by initialising the backing field.

Migration

textarea-signals runs from ng update @koobiq/components@20. It rewrites the value-safe reads and reports the rest. canGrow is deliberately not rewritten: appending () would compile and hand back a different boolean at the row limit.

There is no template pass — kbqTextarea is an attribute on a native <textarea>, so a reference variable is not tied to an element name the schematic can match.

Documented in docs/guides/migration.{en,ru}.md, section 18.

Testing

  • textarea.component.spec.ts: 24 → 27 tests. New coverage for the valueless canGrow attribute, canGrow() reporting the bound value, and the unbound maxRows/freeRowsHeight defaults.
  • textarea-signals/index.spec.ts: 10 tests.
  • Full packages/components (5164 tests) and packages/schematics (492 tests) suites pass.
  • check-api is in sync.

The row-limit reactivity fix is not unit-testable here: jsdom reports no line height, so rowsCount never leaves 0.

BREAKING CHANGE

🤖 Generated with Claude Code

`KbqTextarea` implements `KbqFormFieldControl`, which declares `value`, `id`,
`placeholder`, `required`, `disabled`, `focused`, `empty` and `errorState` as
plain members — that interface is how the form field reads them, so they stay
plain accessors. The four inputs the textarea owns are signals now.

`canGrow` reported `!maxRowLimitReached && bound`, so it said `false` once the
textarea hit `maxRows` even though the consumer had asked for growth. The folded
value drives the resize handle and is internal; `canGrow()` reports what was
bound.

`freeRowsHeight` used to default itself by assigning its own input in `ngOnInit`,
which is why the automated migration skipped it. The fallback is a computed, so
binding it later takes effect instead of being overwritten on the next init.
`maxRows` and `freeRowsHeight` report `number | undefined`, which is what an
unbound textarea always held.

The row count is a signal, so the `kbq-textarea_max-row-limit-reached` class
follows it directly. It is written inside `runOutsideAngular`, so the class used
to wait for an unrelated change detection pass.

The parent animation subscription is torn down with the directive, and the
generated id comes from the CDK `_IdGenerator`.

BREAKING CHANGE: `KbqTextarea.canGrow`, `maxRows` and `freeRowsHeight` are signal
inputs, `maxRowLimitReached` is a computed, `canGrow` reports the bound value
rather than folding in the row limit, and generated ids changed shape. Reported
and partly rewritten by the `textarea-signals` schematic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added bug Something isn't working breaking changes labels Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit 54f91dd):

https://koobiq-next--prs-1990-tv3ykd3g.web.app

(expires Mon, 07 Sep 2026 07:06:42 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

Six members still carried `// TODO: Skipped for migration because:` comments from
the automated tooling. They stay plain because `KbqFormFieldControl` declares them
so, not because a tool gave up — the comments say that now.

`disabled` and `required` coerced by hand with `coerceBooleanProperty` while the
migrated inputs used `booleanAttribute`; both declare the transform on the input
now, so the coercion shows up in the API report instead of hiding in a setter.

`grow` was a per-instance arrow function; only the `setTimeout` needed the
binding. Injections that are never reassigned are `readonly`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking changes bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant