valor-software / ngx-bootstrap

Fast and reliable Bootstrap widgets in Angular (supports Ivy engine)
https://valor-software.com/ngx-bootstrap
MIT License
5.53k stars 1.69k forks source link

Timepicker incorrectly fires change event when value is written by external source #5630

Open jonr-elsewhen opened 4 years ago

jonr-elsewhen commented 4 years ago

Bug description:

The timepicker component incorrectly implements the ControlValueAccessor interface and fires change events when values are written externally.

A typical Angular form component can receive a new value in two ways:

1) The user makes a change via the component's UI, e.g. entering text into a field. 2) The app's code writes a new value using the writeValue(obj) method on the value accessor instance.

Only the first way should cause the the onChange handler to be called. As the Angular docs say regarding registerOnChange():

"Registers a callback function that is called when the control's value changes in the UI."

However, the ngx-bootstrap timepicker component calls onChange even when the value is written externally. This breaks a number of important Angular forms conventions and risks causing infinite loops if the app reacts in response to the change.

Plunker/StackBlitz that reproduces the issue:

https://stackblitz.com/edit/ngx-bootstrap-sswf9j

The example contains a timepicker and a regular text input both bound to same value. Clicking the button updates the value, which causes the new value to be written to both form components. Note, however, that only the timepicker fires a change event in response. The text input, which uses Angular's own implementation of ControlValueAccessor does not. The text input only fires a change when the user interacts directly with the component via the text field.

Versions of ngx-bootstrap, Angular, and Bootstrap:

ngx-bootstrap: 5.4

Angular: 8

Bootstrap: 3.3.7

Build system: Angular CLI, System.js, webpack, starter seed:

Expected behavior

The timepicker component should only emit a new value when the change has come from the component's own UI.

jonr-elsewhen commented 4 years ago

It looks like this would be fixed by https://github.com/valor-software/ngx-bootstrap/pull/5334

blazkovicz commented 4 months ago

Guys, we have encountered very inconvenient issue because of this bug, when are you going to fix this? This is very basic behaviour of control value accessor.