3
\$\begingroup\$

I've created a userscript for Tampermonkey to style unvisited links with blue color, visited links with violet color, and red for hovered/active/focused links.

It was created primarily for my researches on Wikipedia.

Below are shown 3 implementations. The first one is very simple and shown just for demonstration purposes.

My actual question is about 2nd and 3rd implementations. I don't sure which one of them to choose from. Which is better?

1st and 2nd implementations

// ==UserScript==
// @name         Test
// @match        *://*/*
// ==/UserScript==

/* Preferences
=============================================== */

var links = 2;
// Set to "1" to style only simple links (example: a:link)
// Set to "2" to style also hidden links (example: a:link *)

/* Functions
=============================================== */

function AddGlobalStyle(css) {
    var head, style;
    head = document.getElementsByTagName('head')[0];
    if (!head) return;
    style = document.createElement('style');
    style.type = 'text/css';
    style.innerHTML = css;
    head.appendChild(style);
}

function Links(blue, violet, red) {
    AddGlobalStyle(
        blue + '{' +
            'color: rgb(0, 138, 206) !important;' +
            'text-decoration: none !important;' +
        '}'
    );

    AddGlobalStyle(
        violet + '{' +
            'color: rgb(180, 14, 180) !important;' +
        '}'
    );

    AddGlobalStyle(
        red + '{' +
            'color: rgb(204, 0, 0) !important;' +
        '}'
    );
}

/* Main part
=============================================== */

/* 1st way. Simple and works fine. Shown here just to show an idea
----------------- */

/*
if (links == 1)
    Links('a:link', 'a:visited', 'a:hover, a:focus, a:active');
if (links == 2) {
    Links('a:link', 'a:visited', 'a:hover, a:focus, a:active');
    Links('a:link *', 'a:visited *', 'a:hover *, a:focus *, a:active *');
}
*/

/* 2nd way
----------------- */

const plusX = s => s + ' *';

var args = ['a:link', 'a:visited', 'a:hover, a:focus, a:active'];
var modifiedArgs = args.map(
    s => s
        .split(',')
        .map(plusX)
        .join(',')
    );

if (links == 1)
    Links(...args);
if (links == 2) {
    Links(...args);
    Links(...modifiedArgs);
}

The resulting rules in the head section will be as follows (6 style elements):

<style>
    a:link {
        color: rgb(0, 138, 206) !important;
        text-decoration: none !important;
    }
</style>


<style>
    a:visited {
        color: rgb(180, 14, 180) !important;
        text-decoration: none !important;
    }
</style>

<style>
    a:hover, a:focus, a:active {
        color: rgb(204, 0, 0) !important;
        text-decoration: none !important;
    }
</style>

<style>
    a:link * {
        color: rgb(0, 138, 206) !important;
        text-decoration: none !important;
    }
</style>

<style>
    a:visited * {
        color: rgb(180, 14, 180) !important;
        text-decoration: none !important;
    }
</style>

<style>
    a:hover *, a:focus *, a:active * {
        color: rgb(204, 0, 0) !important;
        text-decoration: none !important;
    }
</style>

3rd implementation

// ==UserScript==
// @name         Test
// @match        *://*/*
// ==/UserScript==

/* Preferences
=============================================== */

var links = 2;
// Set to "1" to style only simple links (example: a:link)
// Set to "2" to style also hidden links (example: a:link *)

/* Functions
=============================================== */

// This function is the same
function AddGlobalStyle(css) {
    var head, style;
    head = document.getElementsByTagName('head')[0];
    if (!head) return;
    style = document.createElement('style');
    style.type = 'text/css';
    style.innerHTML = css;
    head.appendChild(style);
}

// But this one have changed
function Links(blue, violet, red) {
    if (links == 2) {
        blue = blue + ',' + blue + ' *';
        violet = violet + ',' + violet + ' *';
        redArray = red.split(',')
        const plusX = s => s + ' *';
        redNew = redArray.map(
            s => s
                .split(',')
                .map(plusX)
                .join(',')
            );
        red = red + ',' + redNew;
    }

    AddGlobalStyle(
        blue + '{' +
            'color: rgb(0, 138, 206) !important;' +
            'text-decoration: none !important;' +
        '}'
    );

    AddGlobalStyle(
        violet + '{' +
            'color: rgb(180, 14, 180) !important;' +
        '}'
    );

    AddGlobalStyle(
        red + '{' +
            'color: rgb(204, 0, 0) !important;' +
        '}'
    );
}

/* Main part
=============================================== */

Links('a:link', 'a:visited', 'a:hover, a:focus, a:active');

The resulting rules in the head section will be as follows (3 style elements):

<style>
    a:link, a:link * {
        color: rgb(0, 138, 206) !important;
        text-decoration: none;
    }
</style>

<style>
    a:visited, a:visited * {
        color: rgb(180, 14, 180) !important;
        text-decoration: none;
    }
</style>

<style>
    a:hover, a:focus, a:active, a:hover *, a:focus *, a:active * {
        color: rgb(204, 0, 0) !important;
        text-decoration: none;
    }
</style>
\$\endgroup\$

2 Answers 2

5
+100
\$\begingroup\$

Your question

My actual question is about 2nd and 3rd implementations. I don't sure which choose from them. Which is better?

That might lead to some opinionated answers, especially because better is subjective. The 2nd implementation calls .split(), .map() and .join() on all three array elements while that is only really only mandatory for the third element. While there would be excess processing on the first two elements, the code is uniform and simpler to read. The 3rd implementation isn't necessarily less-readable but it does take a little longer for one to discern the difference in handling between the two types of strings. The 2nd implementation leads to 6 distinct rulesets while the 3rd leads to 3 - I would rather have as few as necessary and would thus pick the 3rd implementation (presuming the flaw mentioned in the first paragraph below is fixed). While CSS styleguides differ on how to format selectors, it should be agreed that the D.R.Y. principle applies and the output of the 3rd implementation adheres to it.

Review feedback

Flaw in 3rd implementation

redArray = red.split(',')
const plusX = s => s + ' *';
redNew = redArray.map(
    s => s
        .split(',')
        .map(plusX)
        .join(',')
);
red = red + ',' + redNew;

That code calls .split() and extra time (well, actually once extra for each string after separating the original string on commas), so that call can be removed inside the mapping. redNew is an array (and implicitly converted to a string joined by commas via Array.toString()). Perhaps you intended to call .join() on the array returned from .map():

redArray = red.split(',');
const plusX = s => s + ' *';
const redNew = redArray.map(plusX)
    .join(',');

And as you suggested in your jsfiddle via comments you could just omit the call to .join(). In a large web application one might want to explicitly use .join() but it is probably fine to omit it in a small script like this that you are likely the only one who will maintain it.

Notice how I added const. In your code, redArray and redNew are global variables because "Undeclared variables are always global."1, which could have unintentional consequences - for example, if the wikipedia JavaScript had a variable with the same name, then it would be overwritten.

One style tag for each ruleset?

Why not just add one style tag for the whole set of rulesets, instead of one style tag for each? To do this, just combine the three strings and call AddGlobalStyle() once.

Template Literal

Going along with the advice in the previous section, you could use a template literal to add the styles. With expression interpolation, there is no need to terminate the string to add the arguments. That way there is no need to worry about appending strings together.

function Links(blue, violet, red) {
    AddGlobalStyle(`
        ${blue} { 
            color: rgb(0, 138, 206) !important;
            text-decoration: none !important;
        }
        ${violet} {
            color: rgb(180, 14, 180) !important;
        }
        ${red} { 
            color: rgb(204, 0, 0) !important;
        }
    `);
}

Simplifying 2nd implementation

The 2nd implementation code could be simplified:

if (links == 1)
    Links(...args);
if (links == 2) {
    Links(...args);
    Links(...modifiedArgs);
}

Since Links() is always called with ...args, that can be moved out of the if blocks and then only have a check to see if Links() should be called with the modified arguments (i.e. links == 2).

Links(...args);     
if (links == 2) {
    Links(...modifiedArgs);
}

And if you really want to make it shorter, use short-circuiting logical operators (though some may argue it is less readable or isn't as verbose):

Links(...args);     
links == 2 && Links(...modifiedArgs);

1https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/var#Description

\$\endgroup\$
4
  • 1
    \$\begingroup\$ First of all, thank you very much. Very interesting to read. I have some notes (N1, N2) about it. \$\endgroup\$
    – john c. j.
    Commented Jan 30, 2018 at 18:31
  • 1
    \$\begingroup\$ Okay - I realize there is a flaw in the third implementation... See the newly added first paragraph under the Review Feedback section \$\endgroup\$ Commented Jan 30, 2018 at 19:13
  • \$\begingroup\$ In that screenshot I see 2 style tags (i.e. <style>...</style>), each with three declaration blocks so still 6 total declaration blocks (I admit I had to refresh my memory on the terminology today :) ) Actually, I guess we should call them rulesets \$\endgroup\$ Commented Jan 30, 2018 at 19:48
  • \$\begingroup\$ Big thanks for your edits, not the answer is much more clear. Could you elaborate is it a good idea to omit .join()? Probably this omission have some drawbacks? \$\endgroup\$
    – john c. j.
    Commented Jan 31, 2018 at 17:59
3
\$\begingroup\$

Better how?

In this case for me better is

  • simple
  • portable

I have not spent too much time with User Scripts but I write front end code all the time, so I will do my best.

Have you considered a User Stylesheet? Maybe you already have a bunch of tamper monkey code and want to continue using that.

Why spend so much time beating around the bush in Javascript? Maybe you want a more programmatic interface. But it seems like you aren't interested in varying from your requirements too much.

I know you wanted an answer on which is better, but I kind of had a hard time following it. I think it comes from where you are trying to optimize your code reuse. I think that is your biggest problem.

I would define your AddGlobalStyle function, and then call it. Not much else.

AddGlobalStyle('a:link, a:link * {'+
    'color: rgb(0, 138, 206) !important;'+
    'text-decoration: none;'+
'}'
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for your post. ) Hardcoding both a:linkand a:link * in the function will remove the ability to switch between them (as shown in my Preferences section). Because, for some reasons (it will be a bit offtopic to describe it here), most of the time I prefer to use just a:link and keep a:link * in "off" state. \$\endgroup\$
    – john c. j.
    Commented Jan 30, 2018 at 16:57
  • \$\begingroup\$ Also, yes, I agree, the code is a bit overcomplicated, but that's because I tried to learn something new while writing it. The 1st implementation was very simple... Too simple to like it ) \$\endgroup\$
    – john c. j.
    Commented Jan 30, 2018 at 17:04
  • \$\begingroup\$ Ok, so you do want to be able to customize beyond your default. That does invalidate some of my discussion. \$\endgroup\$
    – TMB
    Commented Jan 30, 2018 at 17:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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