10
\$\begingroup\$

This script makes it possible for badges to onebox in Chat.

enter image description here


The install requires Tamper/GreaseMonkey. Click here to install, and it's on GitHub and StackApps!

// ==UserScript==
// @name Badge Oneboxer
// @description Converts tag-like badge comments to badges
// @version 0.24
// @match *://chat.stackexchange.com/rooms/*
// @match *://chat.stackoverflow.com/rooms/*
// @match *://chat.meta.stackexchange.com/rooms/*
// @author The-Quill
// @downloadURL  https://raw.githubusercontent.com/The-Quill/badge-oneboxer/master/badge-oneboxer.user.js
// @updateURL https://raw.githubusercontent.com/The-Quill/badge-oneboxer/master/badge-oneboxer.user.js
// @grant GM_getResourceText
// @resource    badges  https://rawgit.com/The-Quill/badge-oneboxer/master/badges.json
// @run-at document-end
// ==/UserScript==

    'use strict';
    var Badges;
    $.get("https://rawgit.com/The-Quill/badge-oneboxer/master/badges.json", function(data){
        Badges = data;
    }).done(function(){
        var m = new MutationObserver(function(){
            ReplaceAll();
        });
        m.observe(document.getElementById("chat"), {childList: true});
        ReplaceAll();
    });
    function ReplaceAll(){
        var messages = document.getElementsByClassName('message');
        for (var i = 0, length = messages.length; i < length - 1; i++){
            ReplaceText(messages[i])
        }
    }
    var regex = /(\[badge:([a-zA-Z#.\-]+)\])/i;
    var ColourTransforms = {
        bronze: "CC9966",
        silver: "C5C5C5",
        gold: "FFCC00"
    };
    function ReplaceText(node){
        if (!node) return false;
        var badgeProperties = SelectBadgeProperties(node.innerText);
        if (!badgeProperties) { return false };
        if (badgeProperties.total == "") return false;
        node.innerHTML = node.innerHTML.replace(
            badgeProperties.total,
            "<span class=\"ob-post-tag\" style=\"background-color: #FFF; color: #000; border-color: #000; border-style: solid;\">" +
            "<svg version=\"1.1\" height=\"18\" width=\"15\" xmlns=\"http://www.w3.org/2000/svg\"><circle fill=\"#" +
            (Badges.hasOwnProperty(badgeProperties.name) ? ColourTransforms[Badges[badgeProperties.name]] : "000") +
            "\" r=\"3\" cy=\"13.5\" cx=\"3\" /></svg>" +
            badgeProperties.name + "</span>"
        );
        if (HasBadgeText(node.innerText)){
            console.log("again");
            ReplaceText(node);
        }
    };
    function HasBadgeText(text){
        return regex.test(text);
    }
    function SelectBadgeProperties(text){
        if (!HasBadgeText(text)) return false;
        var matchesArray = text.match(regex);
        return {
            total: matchesArray[0],
            name:  matchesArray[2]
        };
    }
\$\endgroup\$
2
\$\begingroup\$

N.b. the current GitHub source already has some of these fixed.

Firstly, all lines are indented one level more than I would expect (the first comment is also indented in the GitHub source); that seems to break the user script annotation, as at the moment I don't see either the description, the name, or the @match restrictions applied after installation.

Otherwise formatting is fine, though the naming convention (upper camel case for top level definitions) is unusual. My editor also marks a few lines with missing semicolon; that could be fixed just for consistency.

Even though it might not happen usually I'd ensure that all referenced functions are defined before calling them, in this case ReplaceAll is run from the callback, when it's only defined a few lines later.

The function ReplaceAll also returns false in a couple of cases, but firstly, the return value isn't used anywhere and secondly, the function doesn't return a value in one case. I'd suggest to either always return a value, or not at all.

The left over console.log should likely be deleted as it doesn't give the reader much information.

In SelectBadgeProperties the regex match is done twice; it would make more sense to only do the match and distinguish between the null value and an actual match.

A minor gripe with ColourTransforms is that the starting # is left out of the colour definition - this means that some external editors, plugins, etc. won't recognize it as a colour. I'd leave it in, in the interest of compatibility and readability, even if it takes three additional characters in the source.

And lastly, inserting many copies of the SVG image doesn't sound performant, but I can't back that up with data obviously; I also doubt that people will insert a massive number of badges and therefore this point is kinda moot.

Otherwise looks great to complement the chat functionality.

\$\endgroup\$
  • \$\begingroup\$ The hash in Colour Transforms is already on the line above \$\endgroup\$ – Quill Feb 14 '16 at 11:39
  • 1
    \$\begingroup\$ I'm not saying it's missing entirely, I'm saying it should be put together with the actual colours so that tools can detect them as such. \$\endgroup\$ – ferada Feb 14 '16 at 11:40
2
\$\begingroup\$

I apologize, but my review isn't going to be very vigorous.


Don't check all elements

Every time a new chat message is added, you are searching through all the chat messages on the page for messages to one-box. This isn't very efficient; you've already one-boxed the passed messages, so you only need to handle the ones recently said.

You can do this using the MutationObserver. I can't get the docs up right now, but every time your listener function is called, some parameters are passed in that tell which elements have been, in your case, added.

Once you get those, pass them to the ReplaceAll function and have it only work on those elements. You could rewrite that function like this:

function ReplaceAll(messages){
    var messages = messages || document.getElementsByClassName('message');
    for (var i = 0, length = messages.length; i < length - 1; i++){
        ReplaceText(messages[i])
    }
}

This will allow for a list of elements to be checked to be passed in and, if they aren't, it will check all the elements.

As for when messages are edited, you may have to configure your MutationObserver a bit more (again, I don't have the docs up, so I don't know what you can do)


GM_getResourceText

In your userscript metadata block, you are using this:

// @resource    badges  https://rawgit.com/The-Quill/badge-oneboxer/master/badges.json

However, you don't use this resource later. Instead, you use jQuery to load the element asynchronously. Now, I'm not exactly sure how this function works, but you can much more easily get this resource just with GM_getResourceText:

var badges = GM_getResourceText("badges");

Too much regex

I don't mean it as the header shows, but your SelectBadgeProperties function doing a lot with regex:

function SelectBadgeProperties(text){
    if (!HasBadgeText(text)) return false;   <---
    var matchesArray = text.match(regex);    <---
    return {
        total: matchesArray[0],
        name:  matchesArray[2]
    };
}

Right there, one after the other, you are matching a regex to a string. It would be simpler and faster to instead only use the match call:

function SelectBadgeProperties(text){
    if ( !(var matchesArray = text.match(regex)) ) return false;
    return {
        total: matchesArray[0],
        name:  matchesArray[2]
    };
}

Misc

function ReplaceAll(){
    var messages = document.getElementsByClassName('message');
    for (var i = 0, length = messages.length; i < length - 1; i++){
        ReplaceText(messages[i])
    }
}

It might be better/faster if you instead check to see if the message actually has the badge text before you try to replace it. But, that's up to you.


var m = new MutationObserver(function(){
    ReplaceAll();
});

can be simplified to

var m = new MutationObserver(ReplaceAll);

function ReplaceAll(){
    var messages = document.getElementsByClassName('message');
    for (var i = 0, length = messages.length; i < length - 1; i++){
        ReplaceText(messages[i])
    }
}

To me, it would make more sense to use map here because you are performing the same operation to each of the elements:

function ReplaceAll(){
    Array.from(document.getElementsByClassName('message'), function(message) {
        ReplaceText(message)
    });
}

\$\endgroup\$
  • \$\begingroup\$ In your last example, that wouldn't work because it's a HTMLNodeList not an array, and so it doesn't have the map prototype. You can fix this by putting that inside Array.prototype.slice.apply or Array.from \$\endgroup\$ – Quill May 11 '16 at 21:24
  • \$\begingroup\$ @Quill Gah, I feared that would be the case. Fixing now. \$\endgroup\$ – SirPython May 11 '16 at 21:27

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.