The idea of this code is to poll the server every 15 seconds to check for and update the notifications indicator. However, I'm not convinced I've done it right, as sometimes it seems to take an unusual amount of time to propagate across tabs and some users have reported that notifications "get stuck" and don't update at all. So here's the code, I think everything's right, but maybe I missed something.
$(function() {
var LSping = "notify_lastping",
LSresult = "notify_result",
poll, interval = 15*1000,
notif_icon = $("#notifs>a"),
notif_body = $("#notifications");
if( notif_icon.length < 1) return;
var notbox = null,
lastping = -1,
running = false;
function startPoll() {
var delay = interval;
if( lastping == -1) {
delay = (+localStorage[LSping]+interval || 0) - Date.now();
if( delay < 100) delay = 100;
}
lastping = localStorage[LSping] || 0;
if( poll) clearTimeout(poll);
poll = setTimeout(getNotifications,delay);
}
function getNotifications() {
// storage event should reset the poll and update notices for us,
// but just to be sure...
if( lastping != (localStorage[LSping] || 0)) {
updateNotifications(
JSON.parse(localStorage[LSresult] || '{"html":"","count":0}'),
true
);
return startPoll();
}
// inform other tabs that I'm on this
localStorage[LSping] = Date.now();
if( !running) {
running = true;
ajax("/notices").success(function(r) {
running = false;
localStorage[LSresult] = JSON.stringify(r);
updateNotifications(r);
}).failure(function(r) {
running = false;
return true; // use default error handler for AJAX callback
});
}
startPoll();
}
function updateNotifications(r,fromStorage) {
if( !fromStorage) {
var oldids = notif_body.find("[data-id]").map(function() {return this.getAttribute("data-id");}).get();
}
notif_icon.attr("data-count",r.count);
notif_body.html(r.html);
document.title = (r.count ? "["+r.count+"] " : "")+document.title.replace(/^\[\d+\]\s*/,"");
if( !fromStorage) {
var newids = notif_body.find("[data-id]").map(function() {return this.getAttribute("data-id");}).get();
var diff = $(newids).not(oldids).get(); // damn jQuery can do some weird stuff XD
if( diff.length > 0) {
// play notification sound if selected by user settings
}
}
}
getNotifications();
$(window).on("storage",function(e) {
e = e.originalEvent;
if( e.key == LSping) {
startPoll();
}
if( e.key == LSresult) {
updateNotifications(JSON.parse(localStorage[LSresult]),true);
}
});
});
As far as I can tell there's no reason for the "loop" to stop. getNotifications()
calls startPoll()
regardless of the AJAX result. But like I said, maybe I missed something.
1 Answer 1
I would try to also add a complete
callback function to set running
to false. It handles ( ("success", "notmodified", "nocontent", "error", "timeout", "abort", or "parsererror")) Those should be handled by error
and success
but something is going on, so give it a shot?
Other than, your code is quite solid, I ran JSHint over it, and it got very few hits.
- You never actually use
notbox
- You probably want to declare
oldids
before thatif
loop
Other than that
The naming is definitely a mixed bag, I would stick everywhere with lowerCamelCase (
lastping
->lastPing
,notif_body
->notificationsBody
( or considernotificationsElement
or perhaps justnotifications
)You are using one liner
if
s without curly braces.
Final thought, are you at any point rebuilding the DOM? If you rebuild the DOM (specifically any of the elements captured by notif_ico
and notif_body
), then your code stops working.
-
\$\begingroup\$
notbox
is used in the original source, an extra bit of presentation for the.error
handler. I trimmed out some of the extra bits before posting here, and must have missed it once. I see what you mean about variable names, they are a bit all over the place. And lastly, while there are plenty of DOM modifications, thenotif_icon
andnotif_body
elements don't change. Even so, I should probably add checks or re-evaluate the selectors... Thanks for your insights. \$\endgroup\$Niet the Dark Absol– Niet the Dark Absol2016年09月15日 15:25:02 +00:00Commented Sep 15, 2016 at 15:25