Skip to content

Check this please #49

Open
SaulBurgos wants to merge 2 commits intoalexbeletsky:masterfrom
SaulBurgos:master
Open

Check this please #49
SaulBurgos wants to merge 2 commits intoalexbeletsky:masterfrom
SaulBurgos:master

Conversation

@SaulBurgos
Copy link
Copy Markdown

I have fixed a problem with the remove of notificators

Sometimes if you show severals notifications like this
sometimes if you have show several notificators like this

notifications.showError({ message: '1111'});
notifications.showSuccess({ message: '22222'});
notifications.showWarning({ message: '3333' };

the autoHide is not working, because the index in the array is the same
@alexbeletsky
Copy link
Copy Markdown
Owner

@SaulBurgos thanks a lot! Very likely the root cause is in this line. It's actually quite bad implementation there.

Instead of id it's possible to use a return value of push(), since it gives a new length of array, correspondingly index of notification is newLenght - 1 is an index of inserted notification. Then, it could be simply removed by index, not by id.

Would you like to take care of that?

@GaryTowers
Copy link
Copy Markdown

Hi guys, are there any plans to merge this PR to fix this issue soon?

@lciolecki
Copy link
Copy Markdown

lciolecki commented Dec 13, 2016

@alexbeletsky are you plan to merge this PR?

@davewood
Copy link
Copy Markdown

bump?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants