20
\$\begingroup\$

I've created a UserScript for adding follow-up reminders to any post (question or answer) here on the Stack Exchange network. I did this in response to a stackoverflow meta post feature request which sparked my interest.

It adds a calendar icon into the vote cell which displays a datepicker where you can select a reminder date at which time you'll be notified via a notification dialog similar to the current inbox and achievements dialog.

post reminder screenshot

Reminders are displayed at the top of the screen in the navbar alongside your inbox/achievements and can be dismissed by clicking on them.

post reminder notification list

Everything works but feels sloppy/spaghetti-ish and I would like to get some feedback on how I can improve it, I'm sure I made a few mistakes.

reminders.js

var Reminder = function (reminderId, postId, postUrl, postTitle, postType, siteName, reminderDate) {
    this.reminderId = reminderId;
    this.postId = postId;
    this.postUrl = postUrl;
    this.postTitle = postTitle;
    this.postType = postType;
    this.siteName = siteName;
    this.reminderDate = reminderDate;
};

var Reminders = {

    Add(reminder) {
        reminders[reminder.reminderId] = {
            "reminderId": reminder.reminderId,
            "postId": reminder.postId,
            "postUrl": reminder.postUrl,
            "postTitle": reminder.postTitle,
            "postType": reminder.postType,
            "siteName": reminder.siteName,
            "reminderDate": reminder.reminderDate
        };
    },

    Clear() {
        reminders = {};
    },

    HasReminder() {
        return reminders.hasOwnProperty(reminderId);
    },

    Load() {
        if (GM_getValue('reminders', undefined) == undefined) {
            GM_setValue('reminders', JSON.stringify(reminders));
        } else {
            reminders = JSON.parse(GM_getValue('reminders'));
        }
    },

    Remove(reminderId) {
        delete reminders[reminderId];
    },

    Save() {
        GM_setValue('reminders', JSON.stringify(reminders));
    }

};

post-reminder.user.js

// ==UserScript==
// @name         SPR-DEV
// @version      1.0
// @namespace    https://stackoverflow.com/users/1454538/
// @author       enki
// @match        *://*.stackexchange.com/*
// @match        *://*.stackoverflow.com/*
// @match        *://*.superuser.com/*
// @match        *://*.serverfault.com/*
// @match        *://*.askubuntu.com/*
// @match        *://*.stackapps.com/*
// @match        *://*.mathoverflow.net/*
// @grant        GM_getValue
// @grant        GM_setValue
// @grant        GM_addValueChangeListener
// @require      https://code.jquery.com/ui/1.11.4/jquery-ui.min.js
// @require      https://rawgit.com/enki-code/UserScripts/master/reminders.js
// @run-at document-end
// ==/UserScript==

var reminders = {},
    sitename = window.location.hostname,
    title = $("#question-header h1 a").text();

$(function () {
    $("head").append("<link rel='stylesheet' href='https://maxcdn.bootstrapcdn.com/font-awesome/4.5.0/css/font-awesome.min.css'>")
             .append("<link rel='stylesheet' href='https://code.jquery.com/ui/1.11.4/themes/smoothness/jquery-ui.css'>")
             .append("<style>.reminder, #reminders {color: #999;} .active-reminder, #reminders.active-reminder { color: dodgerblue; }</style>");

    $("div.network-items").append("<a id='reminders'\
                                      class='topbar-icon'\
                                      style='background-image: none; padding: 10px 0 0 10px; font-size: 12px; '\
                                      title='Post Reminders'>\
                                      <i class=' fa fa-calendar-o'></i>\
                                  </a> '")
                          .on('click', '#reminders', function (e) {
                                 $("#reminder-dialog").toggle();
                             });

    $(".js-topbar-dialog-corral").append("<div id='reminder-dialog' class='topbar-dialog inbox-dialog dno' style='top: 34px; left: 236px; width: 377px; display: block; display:none;'>\
                                          <div class='header'>\
                                              <h3>post reminders</h3>\
                                          </div>\
                                          <div class='modal-content'>\
                                              <ul id='reminderlist'>\
                                              </ul>\
                                          </div>\
                                      </div>");

    Reminders.Load();

    notify();

    // listen for changes and reload reminders
    GM_addValueChangeListener("reminders", function () {
        console.log("reminder data has changed, updating reminder list now...");
        Reminders.Load();
        notify();
    });

    $(".vote").each(function () {
        // add calendar icon to each vote cell and add generate reminderId from postId and sitename since post ids are not unique across all sites
        var postId = $(this).find("input[name='_id_']").val(),
            reminderId = postId + sitename,
            type = $(this).parent().next().attr("class"),
            postType = (type == "postcell" ? "question" : "answer");

        $(this).append("<a class='reminder'\
                       data-reminderid='" + reminderId + "'\
                       title='Remind me of this post'\
                       style=' padding-left:1px;'>\
                            <i class='fa fa-calendar-plus-o fa-2x' style='padding-top:5px;'></i>\
                    </a>\
                    <input type='text' class='datepicker' data-reminderid='" + reminderId + "' data-posttype='" + postType + "' style='display:none;'>")
               .on('click', '.vote a.reminder', function (e) {
                   $(this).next().show().focus().hide();
               });
    });

    $(".datepicker").datepicker({
        minDate: 0,
        onSelect: function (dateText, inst) {
            // date selected, add new reminder and save changes.
            var postId = $(this).data("postid"),
                postUrl = $(this).closest("tr").find(".post-menu").find(".short-link").attr("href"),
                postType = $(this).data("posttype"),
                reminderId = $(this).data("reminderid"),
                reminderDate = new Date($(this).val()),
                calendar = $(this).prev(),
                rem = new Reminder(reminderId, postId, postUrl, title, postType, sitename, reminderDate.getTime());

            Reminders.Add(rem);
            Reminders.Save();
        },
        beforeShow: function (input, instance) {
            instance.dpDiv.css({
                marginTop: '-35px',
                marginLeft: '10px'
            });
        }
    });


    setTimeout(function () { // had to delay this or it wouldn't work, still need to investigate why.
        $('#reminder-dialog .modal-content #reminderlist li a').click(function (e) {
            //notification item clicked, remove item and open link in new tab
            e.preventDefault();
            var id = $(this).data("reminderid");
            Reminders.Remove(id);
            Reminders.Save();
            $(this).remove();
            $("#reminder-dialog").hide();
            window.open($(this).attr('href'), '_blank');
        });
    }, 600);
});

function notify() {
    setTimeout(function () {
        // remove active reminder class from any calendars and hide the notification dialog
        $("#reminders, a.reminder").removeClass("active-reminder");
        $("#reminder-dialog").hide();
        $("#reminderlist").empty();

        $.each(reminders, function (id, val) {
            // find calendar associated with reminder and highlight it
            var calendar = $("a.reminder[data-reminderid='" + id + "']"),
                time = reminders[id].reminderDate,
                currentTime = new Date().getTime();
            $(calendar).addClass("active-reminder").attr("title", "This post has a reminder set for " + new Date(time).toDateString());

            // check if it is time to display reminder notification
            if (new Date().getTime() > time) {
                var reminderDate = new Date(reminders[id].reminderDate).toDateString();
                $("#reminders").addClass("active-reminder");

                $("#reminderlist").append("<li class='inbox-item '>\
                                       <a href='https://" + reminders[id].siteName + reminders[id].postUrl + "' data-reminderid='" + id + "'>\
                                           <div class='site-icon fa fa-calendar-o' title='Post Reminder'></div>\
                                           <div class='item-content'>\
                                               <div class='item-header'>\
                                                   <span class='item-type'>Reminder &mdash; " + reminders[id].postType + "</span>\
                                                   <span class='item-creation'><span title='" + reminderDate + "'>" + reminderDate + "</span></span>\
                                               </div>\
                                               <div class='item-location'>" + reminders[id].postTitle + "</div>\
                                               <div class='item-summary'>" + reminders[id].siteName + "</div>\
                                           </div>\
                                       </a>\
                                     </li>");

            }//end if
        }); //end each
    }, 500);//end setTimeout
} //end Notify
\$\endgroup\$
3
+100
\$\begingroup\$

Overall concept

  • What about shared computers and users who have several devices? Could storage be made independent of a particular device (eg cloud storage) and somehow take account of StackOverflow login.

reminders.js

  • Needs an explanatory comment/link. I for one don't understand the pattern.
  • If you are looking for a more OO way of doing things then with a little thought, Reminder() instances could be full-blooded objects with methods, not just raw data. Reminders.Save() should ignore any non-enumerables on stringification and Reminders.Load() could be modified to re-create full-blooded Reminder() objects from the retrieved raw data. Reminder objects with say .activate(), .isDue() and .notification() methods would allow notify() to be simplified. That would be a lot of work for the sake of elegance but possibly worth while.

post-reminder.user.js

A bunch of nit-picks :

  • Move inline styles into the stylesheet.
  • Test .hasClass('...') rather than .attr('class') == '...'.
  • $.each(reminders, function (id, val) makes val available but reminders[id] is used instead, several times.
  • calendar = $(this).prev() is not used.
  • currentTime = new Date().getTime(); is not used.
  • new Date().getTime() is more efficiently written as Date.now().
  • $(this).data('postid') appears not to be set.
  • e.preventDefault() in click handlers won't hurt even where not strictly necessary.
  • In the onSelect handler new Reminder() parameter list could be composed directly rather than via a series of assignments. Suggest trawling through for other unnecessary assignments (gives GC less to do).
  • Even better, pass a hash (javascript plain object) to the Reminder() constructor instead of individual params.
  • In the $(".vote").each(...) loop, it should be possible to do the .datepicker() widgetization as you go, rather than rediscover the .datepicker elements after the loop has finished. For efficiency, you would need external, named functions for onSelect and beforeShow.

The need for timeouts is worrying. Definitely needs investigating. Possibly due to async loading of SO content?

\$\endgroup\$
7
\$\begingroup\$

reminders.js is good, but you don't need to return an array in for var reminders:

Use the prototype chain instead:

var Reminders = function(){};
Reminders.prototype.Add = function(reminder){};
Reminders.prototype.Clear = function(){};
Reminders.prototype.HasReminder = function(){};
Reminders.prototype.Load = function(){};
Reminders.prototype.Remove = function(reminderId){};
Reminders.prototype.Save = function(){};

That way you get rid of the extra level of indentation.


Your post-reminders.user.js file is a little different.

I found a lot of instances of massive strings to be appended.

Consider using document.createElement instead of strings.

What I also saw is that the jQuery you have could be replaced with vanilla JavaScript equivalents, meaning you could chop the library entirely out of your code.

\$\endgroup\$
  • \$\begingroup\$ Good ideas! I never considered using document.createElement that should be an easy switch. jQuery is already used by StackOverflow and StackExchange sites so it isn't explicitly declared in my script - I was just using it because it was already available but I completely agree that it could be changed to vanilla. \$\endgroup\$ – matt. Dec 21 '15 at 2:54
  • 1
    \$\begingroup\$ @ᴉʞuǝ I would advocate the opposite and say that since you're already using jQuery, just use the idiomatic jQuery element creation syntax $('<div>', { props... }) instead of document.createElement. Nothing against vanilla JS, but the DOM API is fairly verbose, and in the context of the userscript you're already getting jQuery for 'free', so why not use it? \$\endgroup\$ – Yi Jiang Dec 24 '15 at 4:49

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.