Fix for "All" and "Last" history buttons #89
Conversation
…he first notice displayed. Note that there was a bug in the previous implementation because $.merge() returns a new instance so the captured notices_data variable refers to an array containing only the first opened notification.
…, not only the first notice ever opened. In addition, when using the "push": "top" stack option, the most recent notice is at the opposite end of the array so this should be accounted for. Note that there was a bug in the previous implementation because $.merge() returns a new instance so the captured notices_data variable refers to an array containing only the first opened notification. Using the defaults to get the stack value because in theory different notifications could be opened on different stacks, so we have to really on the default to determine if we are pushing to the top.
|
I don't seem to be having this problem. Which browser are you using to test? |
|
I tested this on Chrome, latest version. Steps to reproduce:
3b) Select "Last" |
|
Clarification: between steps 1 & 2, make sure you manually close or allow all of the notifications to time out. Before proceeding with step 2 no notifications should be displayed. |
|
It's possible that this is related to the implementation of $.merge and / or captured variables in different browsers. |
|
That's odd, I'm not seeing the same behavior in Chrome. I don't see how it could hurt though, so I can merge this next time I'm at my computer. |
|
Works for me now as well. My installation of chrome seems to have gotten pretty confused / corrupted lately so maybe that was what caused the problem. Only other relevant piece here then is the "push: top" ordering of the open last action. |
|
Oh ok. Awesome. Could you clean up the other parts to only include that? I'll merge it later today then. |
|
Sure thing. |
…t just the first notice displayed." This reverts commit 498e448.
…t notice, not only the first notice ever opened. In addition, when using the "push": "top" stack option, the most recent notice is at the opposite end of the array so this should be accounted for." This reverts commit 32c35e6.
… "top" stack option, the most recent notice is at the opposite end of the array so this should be accounted for.
Fix for "All" and "Last" history buttons
|
I’m writing to you because you’ve contributed to PNotify. As such, you own the copyrights to the code you contributed. I've decided to release PNotify under the Apache License, instead of the current triple license model. This simplifies PNotify's licensing terms, and provides the users of PNotify with more options while using it. It doesn’t remove any of the rights PNotify users have under the current license model. I need your permission to relicense the code you’ve contributed. Will you please reply to let me know that you agree to have your contributions released under the Apache License? If you don’t agree, or do not respond before the next release, your contributions may be removed from the code base. Thank you for your help in making PNotify awesome!! Best, |
|
@hperrin: I don't have a problem with that. You can include my changes under the new license. |
|
Thanks! |
The history buttons were not working properly. They were referencing a capture variable
notices_data, which got out of sync with the global collection of notices because $.merge returns a new instance on each pnotify call. Because of this, the buttons only worked on the first notification opened.