Primeng: Growl's onClose event references wrong message when message is closed manually

Created on 22 May 2017  路  4Comments  路  Source: primefaces/primeng

I'm submitting a ... (check one with "x")

[x] bug report => Search github for a similar issue or PR before submitting
[ ] feature request => Please check if request is not on the roadmap already https://github.com/primefaces/primeng/wiki/Roadmap
[ ] support request => Please do not submit support request here, instead see http://forum.primefaces.org/viewforum.php?f=35

Plunkr Case (Bug Reports)
http://plnkr.co/edit/0LOFUtsI2HU5ltoBozsh?p=preview

Current behavior
When the user manually closes a Growl message, the onClose callback receives an object containing the next message in the messages array instead of the message that was actually closed. Say we remove the second message on the array then the event is called with the third one. This does not happen when the message is automatically removed by the component.

Expected behavior
The callback should receive the exact message removed.

Minimal reproduction of the problem with instructions

  1. Go to the plunkr link provided above
  2. Wait a second for the 4 messages to pop up
  3. Close the 4 message in order
  4. Check the console output
    (It should log the four message in the correct order but instead it will log messages 2, 3, 4 and then undefined.)

What is the motivation / use case for changing the behavior?
It seems like unintended behaviour

Please tell us about your environment:

  • Operating system: Linux Mint 18.1
  • IDE: Visual Studio Code
  • package manager: NPM
  • Angular version: 4.0.3
  • Angular CLI version: 1.0.3
  • PrimeNG version: 4.0.1
  • Browser: Chromium 58 | Firefox 53
  • Language: TypeScript 2.2
defect

Most helpful comment

@bnazare I have found two issues. One is from you, the other needs to be resolved by the dev team.

@cagataycivici This will need fixed. See number 2. and below.

  1. The binding to the value property in the html needs to be two-way. You were only doing one way.
  2. In the source code of growl.ts, the remove method, that handles the manual removal of the message from the value array is causing the issue. The remove method has the following code:
remove(index: number, msgel: any) {        
        this.domHandler.fadeOut(msgel, 250);

        setTimeout(() => {
            this.value = this.value.filter((val,i) => i!=index);
            this.valueChange.emit(this.value);
            this.onClose.emit({message:this.value[index]});
        }, 250);        
    }

This code is setting the value array to a new value array minus the selected code and then emitting the wrong object later as the value array has changed.

The 'onClose.emit()' call should be placed before the 'this.value = ...' line. This should return the correct Message object.

All 4 comments

@bnazare I have found two issues. One is from you, the other needs to be resolved by the dev team.

@cagataycivici This will need fixed. See number 2. and below.

  1. The binding to the value property in the html needs to be two-way. You were only doing one way.
  2. In the source code of growl.ts, the remove method, that handles the manual removal of the message from the value array is causing the issue. The remove method has the following code:
remove(index: number, msgel: any) {        
        this.domHandler.fadeOut(msgel, 250);

        setTimeout(() => {
            this.value = this.value.filter((val,i) => i!=index);
            this.valueChange.emit(this.value);
            this.onClose.emit({message:this.value[index]});
        }, 250);        
    }

This code is setting the value array to a new value array minus the selected code and then emitting the wrong object later as the value array has changed.

The 'onClose.emit()' call should be placed before the 'this.value = ...' line. This should return the correct Message object.

A pull request has been entered to fix this.

@davisb10 Thanks for the speedy reply. I'll take your advice in the first point into account.

Oh, sorry PR already exists, :P

Was this page helpful?
0 / 5 - 0 ratings