2
\$\begingroup\$

I am trying to keep the progression of the Review Queue Notifier moving forward. My next big move is to turn it into an extension for Chrome and FireFox and eventually Edge.

I would like the Javascript for the code reviewed again to get me back into the groove of coding this extension/script.

Here are the guts:

$( document ).ready(function() {
    //Public Key
    var publicKey = '?key=hyEwZ8*W*OF7tQ3KYgNjzg((';
    var sites;
    var ACTIVESITES;// = chrome.storage.sync.get(activeSites);

    GetSelectedSites();
    getAllTehSitez();

    function getAllTehSitez() {
        console.log("getAllTehSitez has been called");
        $.getJSON('http://api.stackexchange.com/2.2/sites' + publicKey + '&pagesize=100', function(data) {
            sites = data.items;
            isActiveSite();
        });
    }

    function isActiveSite() {
        console.log("isActiveSite has been called");
        chrome.runtime.sendMessage("getUrl", function(response) {
            var tabUrl = response.url;
            for (var site in sites) {
                if ((tabUrl == sites[site].site_url + '/review') 
                    && (ACTIVESITES.indexOf(sites[site].name.toLowerCase()) > -1)) {
                        runRQN();
                        return;
                }
            }
        }); 
    }   

    function GetSelectedSites () {
        console.log("GetSelectedSites has been called");
        chrome.storage.sync.get({
            activeSites: "Code Review"
        }, function(item) {
            ACTIVESITES = item.activeSites;
        });
    }

    function runRQN () {
        console.log("runRQN has been called")
        Notification.requestPermission();
        var DELAY =  300 * 1000; //120,000 milliseconds = 2 Minutes
        function getDelayAmount() { 
            chrome.storage.sync.get({
                refreshRate: 300000
            }, function(item){
                DELAY = item.refreshRate;
            });
        }
        getDelayAmount();

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



        console.log(DELAY);
        var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
        var reviewCount = 0;
        var reviewItems = document.getElementsByClassName('dashboard-num');

        for (var i = 0; i < reviewItems.length; i++){
            if (reviewItems[i].parentNode.className != 'dashboard-count dashboard-faded'){
                reviewCount += parseInt((reviewItems[i].getAttribute("title")).replace(',', ''), 10);
                console.log(reviewItems[i]);
            }
        }
        console.log(reviewCount);
        var image = chrome.extension.getURL('Icon2.jpg');
        if (reviewCount > 0) {
            var details = {
                body: reviewCount + ' Review Items',
                icon: image
            } 
            var n = new Notification(notificationTitle, details );
            setTimeout(n.close.bind(n), 100000); // Magic number is time to notification disappear      
        }
    }
}); 

Stack Apps Question

GitHub Repository

This is a follow-up to the following questions, kind of...

First Question

Second Question

\$\endgroup\$
2
\$\begingroup\$

Foreword

I know that this was posted nearly 1.5 years ago and the code appears to have changed dramatically and perhaps you've already learned about what I will mention below, but I feel like this question needs an answer...

Feedback

I like the good use of the Notification interface, as well as the radix passed to parseInt().

Suggestions

  • The code to check if the site is in the list of active sites, i.e.

    if ((tabUrl == sites[site].site_url + '/review') 
                && (ACTIVESITES.indexOf(sites[site].name.toLowerCase()) > -1)) {
    

    can be simplified using Array.prototype.includes(), which has been supported since FF 43 (released 12/15/2015) and Chrome 47 (released 12/01/2015):

    if ((tabUrl == sites[site].site_url + '/review') 
                && (ACTIVESITES.includes(sites[site].name.toLowerCase()))) {
    
  • The reload registration could be simplified from:

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

    To the following, which makes a partially applied function using Function.bind():

    setTimeout(window.location.reload.bind(), DELAY);
    

    that way there is no excess lambda/anonymous function, and two fewer lines (I get the impression you might like one-liners)!

  • The selection of dashboard elements could be simplified, using a more selective CSS selector:

    var reviewItems = document.getElementsByClassName('dashboard-num');
    for (var i = 0; i < reviewItems.length; i++){
        if (reviewItems[i].parentNode.className != 'dashboard-count dashboard-faded'){
    

    Instead, one could utilize the :not() selector and document.querySelectorAll() (or jQuery selector function, though that would return a jQuery collection instead of a NodeList):

    var reviewItems = document.querySelectorAll('.dashboard-num:not(.dashboard-count):not(.dashboard-faded)');
    
  • One could utilize promises for the asynchronous callbacks...
\$\endgroup\$

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.