Skip to content
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

Growl's onClose event references wrong message when message is closed manually #2846

Closed
bnazare opened this issue May 22, 2017 · 4 comments
Closed
Assignees
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@bnazare
Copy link

bnazare commented May 22, 2017

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
@davisb10
Copy link
Contributor

@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. <p-growl [(value)]="msgs"> 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.

@davisb10
Copy link
Contributor

A pull request has been entered to fix this.

@bnazare
Copy link
Author

bnazare commented May 22, 2017

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

@upmauro
Copy link
Contributor

upmauro commented May 23, 2017

Oh, sorry PR already exists, :P

cagataycivici added a commit that referenced this issue May 26, 2017
Fix #2846 (event onClose receiving wrong element)
@cagataycivici cagataycivici self-assigned this May 26, 2017
@cagataycivici cagataycivici added the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label May 26, 2017
@cagataycivici cagataycivici added this to the 4.0.2 milestone May 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

4 participants