8
\$\begingroup\$

I wasn't satisfied with Simon's alert on the orange alert. I wanted to know if even 1 review item was available, so I made some modifications to the userscript so that it runs on the Review page, and only alerts you when there are reviews that show a positive number.

This is Simon's version, and here is my version:

/** @preserve
// ==UserScript==
// @name Review Queue Notification
// @author Malachi Edited Simon Forsberg Created
// @description Shows a desktop notification when there review items in the queue.
// @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==
*/

var KEY_NEXT = 'NextReload';
var DELAY = 60 * 1000; //60,000 milliseconds
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);

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 reviewCount = 0;
    var reviewItems = document.getElementsByClassName('dashboard-num');

    for (var i = 0; i < reviewItems.length; i++){
        reviewCount += parseInt((reviewItems[i].getAttribute("title")).replace(',',''), 10);
        console.log(reviewItems[i]);
    }
    console.log(reviewCount);
    if (reviewCount > 0) {
        notifications.push(reviewCount + ' Review Items');
    }

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

This is also available on my GitHub as a userscript.

Follow-up question with new GitHub repo

\$\endgroup\$
7
+50
\$\begingroup\$

This is mostly personal preference, but, I usually stack the @matches in descending size:

// @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*

into:

// @match *://*.stackexchange.com/review*
// @match *://*.stackoverflow.com/review*
// @match *://*.mathoverflow.net/review*
// @match *://*.serverfault.com/review*
// @match *://*.askubuntu.com/review*
// @match *://*.stackapps.com/review*    
// @match *://*.superuser.com/review*

Just looks nicer, I suppose.


var title = document.title.split(' - ');:

You don't use title for anything other than title[1], so just use var title = document.title.split(' - ')[1] instead.


.replace(',',''): You should have a space after ','.


You could consider replacing the .length style for loop with a for (var i in reviewItems) style loop. If not, you can also declare reviewCount in the for loop like: for (var reviewCount = 0, i = 0;.


var details = {
        title: document.title,

Desktop Notifications - Code Review Stack Exchange sounds a little bulky and over-the-top. Review Items is a better name for it, and you could consider removing Stack Exchange, but, consider the effect that would have on MSE.


I can't see the point in using notifications as an array, as it should really just have the one notification.

if (reviewCount > 0) {
    notifications.push(reviewCount + ' Review Items');
}
if (notifications.length) {
    var details = {
        text: notifications.join('\n'),
if (reviewCount > 0) {
    notification = reviewCount + ' Review Items';
}
if (notification) {
    var details = {
        text: notification,

or even just move reviewCount + ' Review Items' into details, like:

if (reviewCount > 0) {
    var details = {
        text: reviewCount + ' Review Items',

As of ECMAScript 5, the default radix used in parseInt is supposed to be \$10\$, before that, \$0\$ would get parsed as a octal number instead of a decimal number, and, as it seems, you can just omit that optional radix argument entirely. Although, as @EthanBierlein pointed out in the comments; MDN recommends not to omit it.

\$\endgroup\$
  • 5
    \$\begingroup\$ While you can omit the radix argument from parseInt, MDN doesn't reccomend omitting it. \$\endgroup\$ – Ethan Bierlein Aug 4 '15 at 13:49
  • \$\begingroup\$ In JavaScript, while you can use for in for iterating through an array, it is very uncommon, and therefore confusing to the reader. \$\endgroup\$ – SirPython Aug 4 '15 at 13:51
  • \$\begingroup\$ Normally, I'd suggest forEach, but, it's an object, so the closest alternate to that is .each in jQuery, and also why I only presented it as an alternate solution, rather than expounding on it. \$\endgroup\$ – Quill Aug 4 '15 at 13:54
3
\$\begingroup\$

Removing some of the code where this would refresh, I forgot to alter the @match's and this made it kind of tricky to do a review (I changed the timer to 20 seconds)

I needed to remove the wildcard from the end

like this

// @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

In addition, I also changed the timeout on the popup, because if I had left the window open and left my computer for some time and there was a review left, it would populate a new popup and never dispose it, it was a PITA to close them all and I would rather check the review page when I get back to my desk than have all those glaring at me.

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

became this (with some changes that @Quill suggested)

if (reviewCount > 0) {
    var details = {
        title: document.title,
        text: reviewCount + ' Review Items',
        timeout: 15000
    }
    GM_notification(details, null);
}

and I also changed

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

to this

document.title = document.title.split(' - ')[1] + ' Review Queue'; // keep the site name

we already know that it is a desktop notification, there is no need to throw it in my face every time.

\$\endgroup\$
  • \$\begingroup\$ Did it ever occur to you that you don't need to change the document.title at all? \$\endgroup\$ – Simon Forsberg Aug 20 '15 at 10:43
  • \$\begingroup\$ @SimonAndréForsberg, I think I actually did think about that once. last night I was setting up a new repo so I could assign myself issues. \$\endgroup\$ – Malachi Aug 20 '15 at 12:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.