fix(textarea)!: errors following a full review of the component (#DS-5482) - #1990
Draft
artembelik wants to merge 2 commits into
Draft
fix(textarea)!: errors following a full review of the component (#DS-5482)#1990artembelik wants to merge 2 commits into
artembelik wants to merge 2 commits into
Conversation
`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>
|
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 |
🚨 E2E tests failedReview the report for details. 💡 Comment |
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>
🚨 E2E tests failedReview the report for details. 💡 Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A full review of
textarea, in the same shape as the 20.3.0 component reviews.What stays a plain accessor, and why
KbqTextareaimplementsKbqFormFieldControl, which declaresvalue,id,placeholder,required,disabled,focused,emptyanderrorStateas 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.canGrowdisagreed with what was boundIt reported
falseonce the textarea hitmaxRows, even though the consumer had asked for growth. The folded value is what drives the resize handle, so it became an internalgrowingcomputed;canGrow()reports what was bound. Same shape asKbqLink.tabIndexin the earlier review.freeRowsHeightdefaulted itself by writing its own inputThat is exactly why the automated migration skipped it. The fallback is a
computedover the measured line height now, so binding[freeRowsHeight]later actually takes effect instead of being overwritten the next timengOnInitruns — and it does run again, because the parent animation re-invokes it.maxRowsandfreeRowsHeightlied about being requiredBoth were declared
numberwhile an unbound textarea heldundefined.maxRowLimitReachedcomparedrowsCount > undefined, which isfalse— that accident is what made unlimited growth work. They reportnumber | undefinednow and the comparison is explicit.The row-limit class waited for someone else's change detection
rowsCountis written insiderunOutsideAngular, 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
parent.animationDonesubscription had no teardown; it istakeUntilDestroyed()now._IdGeneratorinstead of a module counter, so the shape changes fromkbq-textarea-1tokbq-textarea-a1.this.id = this.idin the constructor — a trick to force the setter — is replaced by initialising the backing field.Migration
textarea-signalsruns fromng update @koobiq/components@20. It rewrites the value-safe reads and reports the rest.canGrowis deliberately not rewritten: appending()would compile and hand back a different boolean at the row limit.There is no template pass —
kbqTextareais 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 valuelesscanGrowattribute,canGrow()reporting the bound value, and the unboundmaxRows/freeRowsHeightdefaults.textarea-signals/index.spec.ts: 10 tests.packages/components(5164 tests) andpackages/schematics(492 tests) suites pass.check-apiis in sync.The row-limit reactivity fix is not unit-testable here: jsdom reports no line height, so
rowsCountnever leaves 0.BREAKING CHANGE
🤖 Generated with Claude Code