Sandbox: Angular template fix form component #32502
Conversation
…Update form validation and success handling logic.
WalkthroughMigrates an Angular form component to signals-based outputs and state, marks it standalone with FormsModule, updates template for ngModel and native validation, and adjusts conditional rendering to use signal reads. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant T as Template (Form)
participant C as FormComponent
participant P as Parent Component
U->>T: Enter input + submit
T->>C: ngSubmit (validated via ngNativeValidate)
C->>C: complete.set(true)
C-->>P: onSuccess.emit(value) via output<string>()
Note over T,C: Template reads complete() to render "Completed!!"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
ndelangen
added
build
|
View your CI Pipeline Execution ↗ for commit a530612
☁️ Nx Cloud last updated this comment at |
ndelangen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
code/frameworks/angular/template/components/form.component.ts (4)
5-6: Standalone conversion looks good; add OnPush for perf and consistency.Signals work well with OnPush and reduce accidental change detection.
Apply:
@Component({ standalone: true, imports: [FormsModule], + changeDetection: ChangeDetectionStrategy.OnPush,And add import (outside this hunk):
import { ChangeDetectionStrategy } from '@angular/core';Also ensure stories use
moduleMetadata({ imports: [FormComponent] })(notdeclarations) after the standalone switch.
9-9: Prefer (ngSubmit) over (submit) with template‑driven forms.It integrates with
ngFormvalidity and mirrors Angular conventions.- <form id="interaction-test-form" ngNativeValidate (submit)="handleSubmit($event)"> + <form id="interaction-test-form" ngNativeValidate (ngSubmit)="handleSubmit($event)">
15-17: A11y: announce completion to assistive tech.Expose the status change for screen readers.
- <p>Completed!!</p> + <p role="status" aria-live="polite">Completed!!</p>
29-38: Avoid race/flicker on rapid submits; clear timers and clean up on destroy.Multiple quick submits schedule overlapping timeouts, causing inconsistent
completestate and minor leaks.Apply within this method:
handleSubmit(event: SubmitEvent) { event.preventDefault(); this.onSuccess.emit(this.value); - setTimeout(() => { - this.complete.set(true); - }, 500); - setTimeout(() => { - this.complete.set(false); - }, 1500); + this.clearTimers(); + this.complete.set(false); + this.showTimer = setTimeout(() => { + this.complete.set(true); + }, 500); + this.hideTimer = setTimeout(() => { + this.complete.set(false); + }, 1500); }Add supporting members (outside this hunk):
// import { OnDestroy } from '@angular/core'; export default class FormComponent implements OnDestroy { private showTimer?: ReturnType<typeof setTimeout>; private hideTimer?: ReturnType<typeof setTimeout>; private clearTimers() { if (this.showTimer) clearTimeout(this.showTimer); if (this.hideTimer) clearTimeout(this.hideTimer); this.showTimer = this.hideTimer = undefined; } ngOnDestroy() { this.clearTimers(); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
code/frameworks/angular/template/components/form.component.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: daily
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (4)
code/frameworks/angular/template/components/form.component.ts (4)
12-12: LGTM: Added name for ngModel.This unblocks
[(ngModel)]binding.
27-27: LGTM:signal(false)with template read viacomplete()is correct.
23-23: Public API shift: EventEmitter → output() may break .subscribe call-sitesRan the suggested search (rg -nP '\bonSuccess\s*.\s*subscribe\b|fixture.componentInstance.onSuccess\b' -C2) — no matches in the repo; absence of matches isn't proof. Confirm tests, other packages, and external consumers don't call component.onSuccess.subscribe. If they do, restore an EventEmitter or add an adapter (e.g., expose an Observable/subscribe wrapper or provide a test helper).
1-2: Versions verified — Angular 19 present; features supported.code/frameworks/angular/package.json pins @angular/core, @angular/common, @angular/forms to ^19.1.1 (peer range >=18 <21) and @storybook/angular is workspace:* (local); Angular 19 includes signals, the new control‑flow syntax (@if/@for), and the output() API — no CI break expected. (angular.dev)
What I did
Fix the form component for template for sandboxes
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
New Features
Refactor