primefaces / community

Community Forum
0 stars 0 forks source link

Multiselect's onChange event is not fired on Select all/Unselect all or range selection #1232

Closed jonmarozick closed 4 months ago

jonmarozick commented 4 months ago

I believe this is a bug. It was introduced in version 16.8.0. You can see it in the diff between version 16.7.2 and 16.8.0 in the src/app/components/multiselect/multiselect.ts. Also, the breaking change is not listed in the release notes for 16.8.0. Yes, you can look at the full change log but a breaking change should be called out and should be in a major version bump instead of a minor version bump.

v16.7.2 shows the updateModel and onChange functions fire every time.

v16.8.0 checks for this.selectAll to be explicitly null for updateModel to fire. And the call to onChanged was removed.

v17.1.0 updated the this.selectAll check to be less strict

In master, there are 2 place onChanged is called: onOptionsSelect and removeOption

updateModel is called in 6 places: onOptionsSelect, onOptionSelectRange, onKeydown, onToggleAll, clear, and removeOption

Should the onChange event be fired in all 6 cases where updateModel is called? The onSelectAllChange and clear events give hooks for those 2 cases but onOptionSelectRange and onKeydown don't provide events. I could, of course, tap into the control value accessor and use the forms system to watch the changes but then I'm forced to wrap each multselect in a form.

Originally posted by **edvard-mit** January 4, 2024 Hi team, with 17.3.0 version p-multiSelect neither onChange neither onSelectAllChange event fired on Select All/ Unselect All action. [Stackblitz](https://stackblitz.com/edit/pqrhxe?file=src%2Fapp%2Fdemo%2Fmulti-select-basic-demo.html) ` ` Is it a bug or feature?

Additional oddity

In the SlackBlitz reproducer below, setting the Input property selectAll to true causes the select all checkbox to always be checked and the onSelectAllChange event fires. Setting the Input property selectAll to false causes the select all checkbox to always be unchecked and the onSelectAllChange event fires. Setting the Input property selectAll to null causes the select all checkbox to be toggled and the onSelectAllChange event does not fire.

Environment

Windows 10, Edge Version 121.0.2277.106 (Official build) (64-bit)

Reproducer

https://stackblitz.com/edit/pqrhxe-2vxseb?file=src%2Fapp%2Fdemo%2Fmulti-select-basic-demo.html

Angular version

16.2.12, 17.2

PrimeNG version

16..8.0 and later

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

18.16.0

Browser(s)

No response

Steps to reproduce the behavior

Bind an onChange to a multiselect. Checking select all/ unselect all does not fire onChange

Expected behavior

Select all and unselect call onChange

jonmarozick commented 4 months ago

It appears that release 17.8.0 fixed this issue. The PR didn't add an onChange call to onKeydown or onOptionsSelectRange but the existing code never has done that. I'm curious if the is indented or not.

fixed https://github.com/primefaces/primeng/issues/14807 https://github.com/primefaces/primeng/issues/14252 https://github.com/primefaces/primeng/issues/14603 - updated onToggleAll method to call onChange and onSelectAllChange by @rosenthalj in https://github.com/primefaces/primeng/pull/14826