Desktop Notifications for flags and queue items

Question

After our recent election, I got elected as a moderator here on Code Review (Thank you, community members!). One of the first things I missed among the moderator tools was desktop notifications for when there were any flags.

As no current tool existed, I figured that I just have to create one...

Following the approach of "Disassemble your problems so you can assemble your solutions" (which I believe that I normally follow whether I am aware of it or not) I figured that I am gonna need the following things:

• Desktop Notifications
• Automatically reload the page (could also possibly work with AJAX requests)
• jQuery
• It should be functional on Stack Exchange
• Load/Save local data

At first I used some advice from SO and a general Stack Exchange userscript template, but later I realized I didn't need those things (and they just prevented me from using some GM_* functions).

The script adds a link on the /review URL (of any Stack Exchange site) to /review/desktop-notifications, which technically leads to a 404 page but gets some added information thanks to the user-script.

The /review link looks like this:

and /review/desktop-notifications looks like:

A desktop notification looks like the following:

The source code is available as a part of my SE-Scripts GitHub repository. Any feature-requests and pull requests are appreciated.

The code has been tested with Tampermonkey 3.11 for Google Chrome, but I believe it should work with other user-script handlers as well.

Code

// ==UserScript==
// @name Moderator Flag Notification
// @author Simon Forsberg
// @description Shows a desktop notification when there are flags or review items to be handled.
// @namespace https://github.com/Zomis/SE-Scripts
// @grant GM_getValue
// @grant GM_setValue
// @grant GM_notification
// @match *://*.stackexchange.com/review*
// @match *://*.stackoverflow.com/review*
// @match *://*.superuser.com/review*
// @match *://*.serverfault.com/review*
// @match *://*.askubuntu.com/review*
// @match *://*.stackapps.com/review*
// @match *://*.mathoverflow.net/review*
// ==/UserScript==

if (window.location.href.indexOf('/desktop-notifications') === -1) {
    $('.tools-rev h1').append('<span class="lsep">|</span><a href="/review/desktop-notifications">Desktop Notifications</a></h1>');
} else {
    var KEY_NEXT = 'NextReload';
    var DELAY = 60 * 1000;
    var currentTime = Date.now ? Date.now() : new Date().getTime();
    var lastTime = GM_getValue(KEY_NEXT, 0);
    var nextTime = currentTime + DELAY;
    GM_setValue(KEY_NEXT, nextTime);

    var timeDiff = Math.abs(lastTime - currentTime);
    setTimeout(function(){ window.location.reload(); }, DELAY);

    $('.subheader h1').html('Desktop Notifications');
    $('.leftcol').html('Keep your browser open on this page and it will be automatically reloaded and alert you when something wants your attention.').removeClass('leftcol');
    $('.rightcol').remove();

    var title = document.title.split(' - '); // keep the site name
    document.title = 'Desktop Notifications - ' + title[1];

    // a way to detect that the script is being executed because of an automatic script reload, not by the user.
    if (timeDiff <= DELAY * 2) {
        var notifications = [];

        var topbarFlag = $('.topbar-links .mod-only .icon-flag .flag-count').html();
        var topbarFlagCount = parseInt(topbarFlag);
        if (topbarFlagCount > 0) {
            notifications.push(topbarFlagCount + ' Flags');
        }

        var reviewItems = $('.icon-tools-flag span');
        var reviewCount = 0;
        if (reviewItems.length > 0) {
            reviewCount = parseInt(reviewItems.html());
            if (reviewCount > 0) {
                notifications.push(reviewCount + ' Review Items');
            }
        }

        if (notifications.length > 0) {
            var details = {
                title: document.title,
                text: notifications.join('\n')
            };
            GM_notification(details, null);
        }
    }
}

Primary Concerns

• Is my code readable?
• Am I following best-practices?
• Am I using the GM_* features in a good way?
• Any other comments also appreciated.

Minor update:

With the new Stack Exchange topbar, change a line to this:

var topbarFlag = $('.top-bar .indicator-badge').html();

Answer 1

Magic Numbers

You did a very nice job of eliminating all magic numbers/values in the top of your code. However, there is still this one line:

var DELAY = 60 * 1000;

Where did 60 come from? And 1000?

These are still magic numbers, but they are not worthy of new constants; I would just add a simple comment explaining what DELAY is being set to.

Thanks to Mast for this:

var DELAY = 60 * 1000; usually gets a comment in my code indicating nature should be blamed if they ever need to be changed. Also read 100 is a magic number`. Explanation of what the 60 and 1000 stand for is good (even required), but giving them their own constants...

The radix parameter

You call the function parseInt a few times in your code. Like here:

var topbarFlagCount = parseInt(topbarFlag);

and here:

reviewCount = parseInt(reviewItems.html());

Although it is not commonly known, the parseInt function actually takes a second parameter called the radix. This parameter is used to specify which numerical base the first parameter is in.

Most commonly, this number is 10 (base 10, or decimal). You can read more about this parameter here.

So, for example, you might write this as one of your lines:

var topbarFlagCount = parseInt(topbarFlag, 10);

---

Going back to your old post

Quite a while ago, you posted another UserScript question.

In an answer to that question, the answered mentioned that, to help compatibility with FireFox, you should encase the beginning part of your UserScript like this:

/** @preserve
// ==UserScript==
.....
// ==/UserScript==
*/

There are a number of UserScript conventions you are failing to adhere to.

First up, for Firefox, you need to wrap the header section in to a "preserved" comment block:

...

The preserve is required to keep the comment block after the javascript is processed. This allows the 'compiled' version to keep the references, and for FireFox to know what it is about still.

Other browsers may not have the same requirements.

---

Compressed lines

This line here:

setTimeout(function(){ window.location.reload(); }, DELAY);

Took me a few times to read before I could match up the {}s and the ()s. I think this would be much more readable if you expanded it, like this:

setTimeout(function() {
    window.location.reload(); 
}, DELAY);

---

Wrap your code

This is just a small thing that I wanted to point out.

You should encase your code in this:

(function (window, undefined) {
    (function ($) {
        [code]
    })(window.jQuery);
})(Function("return this")());

The reason for this is because the names window, undefined and $ are all valid variable names that could easily be used in any variable.

Only do this if you aren't writing anything important.

For example, open up your console and type:

var $ = 3;
console.log($);

You will get the output

3

As for window and undefined, the same will not happen for them: if you were to try:

var window = 5;
console.log(window);

The actual WOM object will be outputted. This is because for these variables, overriding them will only change their values locally; they will not be changed on a global level.

Read more in this chat conversation.

Basically, by wrapping your code in these anonymous functions, you are ensuring that no matter what other scripts do with those variable names, those variables will always be the way you want them.

And, the reason why the function for setting $ is not combined with the first anonymous function is because the one for setting $ needs to use the window object, which the first anonymous function ensures is valid.

Some more reasoning why there is an anonymous function for $ here.

Revert the $ alias and then create and execute a function to provide the $ as a jQuery alias inside the function's scope. Inside the function the original $ object is not available. This works well for most plugins that don't rely on any other library.

EDIT:

The reason why you use the inner anonymous function is because of the function jQuery.noConflict.

If any code earlier in the document were to call this function, the alias $ would be stripped away from jQuery. This function restores the alias to jQuery, just in case this function was called.

Read more in this chat conversation.

Thanks to Ismael Miguel.

---

Split up your code

In your inner if statement of your larger one, you have two sections: one for getting all the flags, and one for getting all that is in the review queue.

To take some off the load of your main code, you should split these into respective functions, probably called getFlags and getReviewItems.

---

"use strict";

Your code should have "use strict";.

Answer 2

SirPython already gave a really nice review.

But there are some nitpicky points left behind. Just small stuff.

You have the following line:

var currentTime = Date.now ? Date.now() : new Date().getTime();

You could use any of these:

var currentTime = new Date().getTime();
var currentTime = +(new Date());

I actually prefer the later one, since it is faster. I saw a jsperf somewhere.

---

Also, you have ifs similar to this one:

if (notifications.length > 0) {

You don't need the > 0, since you can't have undefined elements or -1 elements. Just change it to:

if (notifications.length) {

Continuing to beat on your poor ifs, lets look at this one:

var reviewItems = $('.icon-tools-flag span');
var reviewCount = 0;
if (reviewItems.length > 0) {
    reviewCount = parseInt(reviewItems.html());
    if (reviewCount > 0) {
        notifications.push(reviewCount + ' Review Items');
    }
}

Why this whole mess when you can set reviewCound to the right value, right away?

Like this:

var reviewItems = $('.icon-tools-flag span');
if (reviewItems.length > 0) {
    var reviewCount = parseInt(reviewItems.html(), 10);
    if (reviewCount > 0) {
        notifications.push(reviewCount + ' Review Items');
    }
}

And if you really want to cut the fat:

var reviewCount = parseInt($('.icon-tools-flag span').html(), 10);
if (reviewCount > 0) {
    notifications.push(reviewCount + ' Review Items');
}

There, your whole block in 4 lines. And it is still readable!

---

Something to consider is that you are navigating to a page that triggers the error 404
I would recommend adding /404 to the URL, being it now /review/desktop-notifications/404, which will be registered as a real attempt to get a 404 page.

Or you could look for alternative method like creating an empty <iframe> with your code there or a popup.