-
Notifications
You must be signed in to change notification settings - Fork 13
fix(DB Switch): double event firing with Angular signals #4779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ssue Co-authored-by: mfranzke <[email protected]>
Co-authored-by: mfranzke <[email protected]>
@copilot I don't see a reason to modify |
Co-authored-by: mfranzke <[email protected]>
You're absolutely right. I've removed the unrelated |
Co-authored-by: mfranzke <[email protected]>
🦋 Changeset detectedLatest commit: b238811 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a double event firing issue in the DB Switch component when using Angular signals. The root cause was improper implementation of the Angular ControlValueAccessor pattern in the handleFrameworkEventAngular
function, which was calling both propagateChange
and writeValue
for user-initiated events.
Key changes:
- Removed
writeValue
call fromhandleFrameworkEventAngular
function to follow proper Angular ControlValueAccessor pattern - Added documentation explaining the distinction between
propagateChange
(view → model) andwriteValue
(model → view) - Added test showcase demonstrating the fix with Angular signals
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/components/src/utils/form-components.ts | Core fix removing writeValue call and adding documentation for proper ControlValueAccessor pattern |
showcases/angular-showcase/src/app/components/form/form.component.ts | Test component setup with signal state and change handler |
showcases/angular-showcase/src/app/components/form/form.component.html | Test template demonstrating switch with Angular signals |
showcases/angular-showcase/src/app/components/form/form.component.ts
Outdated
Show resolved
Hide resolved
showcases/angular-showcase/src/app/components/form/form.component.ts
Outdated
Show resolved
Hide resolved
Pull request was converted to draft
Problem Solved
The DB Switch component was triggering
change
events twice when using Angular signals. The root cause was inhandleFrameworkEventAngular
which incorrectly called bothpropagateChange
andwriteValue
during user-initiated events.Solution Evolution
Initial approach: Only skipped
writeValue
for signal-based properties (checked
,disabled
)Final approach: Completely removed
writeValue
call for all user-initiated events, following proper Angular ControlValueAccessor pattern:propagateChange
: For view → model updates (user interactions) ✅writeValue
: Only for model → view updates (programmatic changes) ✅This architectural fix resolves the double event issue for all affected components while maintaining proper Angular forms integration.
Impact
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
fixes #4599