REGRESSION 16.7.2: dropdown incorrectly implements NG_VALUE_ACCESSOR, writeValue is issuing change.

[pete-mcwilliams]

Describe the bug

writeValue on dropdown has had a call to this.updateModel(this.value); which in turn is calling this.onModelChange(value); which is the registered on change function for downdown. Issue introduced in 99a83d0

Working example in 16.6.0:
https://stackblitz.com/edit/hyecnx?file=src%2Fapp%2Fdemo%2Fdropdown-basic-demo.html,src%2Fapp%2Fdemo%2Fdropdown-basic-demo.ts

Issue in 16.7.2:
https://stackblitz.com/edit/hyecnx-mvfuaw?file=src%2Fapp%2Fdemo%2Fdropdown-basic-demo.html,src%2Fapp%2Fdemo%2Fdropdown-basic-demo.ts

Environment

N/A

Reproducer

https://stackblitz.com/edit/hyecnx-mvfuaw?file=src%2Fapp%2Fdemo%2Fdropdown-basic-demo.html,src%2Fapp%2Fdemo%2Fdropdown-basic-demo.ts

Angular version

16.2.7

PrimeNG version

16.7.2

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

v18.17.0

Browser(s)

No response

Steps to reproduce the behavior

use dropdown component see angular thinks it has been changed after writeValue has run.

Expected behavior

writeValue should not trigger function registered in registerOnChange

[roman-balzer]

Same problem for multiselect, it also fires onChange ol init of options.

As for dropdown, I'm not sure, but the onChange which is fired when the option are set, also sets the selected Value to the first in the Array, but only if I use a *ngIf or @if before hand. If I don't it does not trigger the onChange and doesnt set the selected Value, but it does not show the placeholder for some reason. (not working with signal and observable/async)

[cetincakiroglu]

16.7.0 bug metni

Hi,

With version 16.7.0, we have released the first part of the accessibility implementation for inputs that we have been working on for a long time. Based on the research we conducted during the implementation, we decided that many components needed changes in their structure and simplification, and these components were rewritten. Unfortunately, after extensive testing, there may be unexpected and unforeseen bugs that have emerged. We are grateful for your reports, and we are working to address them in the upcoming release!

Thanks a lot for reporting the issue, it's fixed by a refactor in a051243

[pete-mcwilliams]

@cetincakiroglu just a quick review and not tested but it looks like writevalue is now directly calling this.onModelChange(value);

writevalue should not be triggering (ngModelChange).

[cetincakiroglu]

@pete-mcwilliams

Before we refactored the components, dropdown was triggering ngModelChange in writeValue method by updateSelectedOption method, so it's not a new implementation. All changes have been attempted to be made without disrupting the previous functionality. So it was triggering the ngModelChange before and it triggers now, but if you have a better idea or solution to offer, we're open to any ideas to improve PrimeNG. Please feel free to open ideas or solutions to make us maintain PrimeNG better :)

You can check the line 988 of dropdown.ts in v16.4.1 to see the previous implementation.

[pete-mcwilliams]

Thanks for getting back to me, and I appreciate the prompt work on the issue.

The examples I gave, showed that the old version did not indicate there had been a change. I guess the problem I am having maybe that if nothing has been set then I am still getting a change back when it's been moved to writeValue.

[pete-mcwilliams]

One final try to give an example on 16.4.1.

https://stackblitz.com/edit/hyecnx-n4yisz?file=src%2Fapp%2Fdemo%2Fdropdown-basic-demo.html,src%2Fapp%2Fdemo%2Fdropdown-basic-demo.ts

having a value set though ngModel is not causing a change to be registered.

[pete-mcwilliams]

@pete-mcwilliams

Before we refactored the components, dropdown was triggering ngModelChange in writeValue method by updateSelectedOption method, so it's not a new implementation. All changes have been attempted to be made without disrupting the previous functionality. So it was triggering the ngModelChange before and it triggers now, but if you have a better idea or solution to offer, we're open to any ideas to improve PrimeNG. Please feel free to open ideas or solutions to make us maintain PrimeNG better :)

You can check the line 988 of dropdown.ts in v16.4.1 to see the previous implementation.

looking at updateSelectedOption it is not definitely going to issue a change.
if (this.autoDisplayFirst && !this.placeholder && !this.selectedOption && this.optionsToDisplay && this.optionsToDisplay.length && !this.editable)

if we want to autoDisplayFirst and we have no placeholder and the value passed in could not be found in the options so selectedOption is empty and we have some options and it is not editable then select the first value... which is fine to report as a change.

I think this issue is not resolved with the change, and it is breaking the NG_VALUE_ACCESSOR model.

[cetincakiroglu]

@pete-mcwilliams

Thanks for pointing me out I've just added that additional if check to the writeValue method.

[djay1977]

Hello @cetincakiroglu
I updated my apps to PrimeNG 16.9.1 and all my dropdowns no longer work properly.
My options are SelecItem[] and I have a null value with the label "Select". This value is no longer interpreted and selected.
Would it be possible to make the correction?
Thanks,
Jerry

[pete-mcwilliams]

Hello @cetincakiroglu I updated my apps to PrimeNG 16.9.1 and all my dropdowns no longer work properly. My options are SelecItem[] and I have a null value with the label "Select". This value is no longer interpreted and selected. Would it be possible to make the correction? Thanks, Jerry

created an issue, we are using empty strings and not matching. #14223