5
\$\begingroup\$

I made this little library that makes the title of a webpage scroll so you can see the entire string. I'm not sure if this is the best way to accomplish this so if anyone has any input that'd be great.

title-scroll.js

var start = 3000;
var speed = 250;

function getParams(startAttr, speedAttr) {
    var params = window.document.getElementsByTagName('script');

    for (var i = 0; i < params.length; i++) {
        if (params[i].src.indexOf('title-scroll.js') !== -1) {
            if (params[i].getAttribute(startAttr) !== null && params[i].getAttribute(startAttr) !== "") {
                start = params[i].getAttribute(startAttr);
            }

            if (params[i].getAttribute(speedAttr) !== null && params[i].getAttribute(speedAttr) !== "") {
                speed = params[i].getAttribute(speedAttr);
            }

            return;
        }
    }
}

window.onload = function(e) {
    getParams('data-start', 'data-speed');
    var title_ref = window.document.getElementsByTagName('title')[0];
    var title = title_ref.text;
    var i = 0;

    setTimeout(function() {
        setInterval(function () {
            title_ref.text = title.substr(i, title.length) + "  ---  " + title.substr(0, i);
            i++;

            if (i === title.length) {
                i = 0;
            }
        }, speed);
    }, start);
}

HTML to properly load the file

<script src="title-scroll.js" data-start="3000" data-speed="250"></script>

Edit:

data-start: Time between page load and when the title starts to scroll.

data-speed: Time between each character movement.

\$\endgroup\$
5
\$\begingroup\$

A history lesson

I feel obligated to note that similar functionality (in the page body) has already been implemented in native html using the <marquee> tag (mdn). This tag has been deprecated for forever though, because it is deemed a usability nightmare and visually distracting without a cause.

The same issue applies to changing the title of the page. It distracts the user, and may annoy the user. Users can already view the entire page title by hovering over the tab, but most people do not care about what the title is at all.

Avoid onload (and other on... definitions)

Don't use the on... family of attributes to create listeners for events. Your code does not operate in a void. There are other scripts, some of which you do not have control over. When you assign to an on... attribute, you override the previous assigned function. This, in turn, creates hard to debug problems.

Instead, use .addEventListener(..) (mdn). You can add and remove as many listeners as you want without having to worry about other code.

var clicker = document.getElementById('clicker');

clicker.onclick = function () {
  console.log('First handler');
}
clicker.onclick = function () {
  console.log('Second function');
}
The problem with onclick (and onload, and other on... attributes):

<a href="#" id="clicker">Click me!</a>

Custom parameters

You are correctly using attributes with the data- prefix for your custom attributes. If we want to use such attributes though, we usually put them on the element we are manipulating, not a random script tag. In this case, it's probably better to put it on the title tag. This will also simplify your code a lot, since you only need to grab the title tag and do not have to worry how you load in your code, or if any other code is using your custom attributes.

Optimalisation of housekeeping in interval callback

You are currently using the following code for housekeeping in your interval callback

i++;

if (i === title.length) {
    i = 0;
}

You could instead leverage the modulo operator to simplify the code.

i = (i + 1) % title.length;

Separator

You are calculating your title length without the separator. This means that the title will not complete a full round and instead will jump back to the beginning when -- is at the start.

Various other things

It is always a good idea to use IIFE (Immediate Invoked Function Expressions) to encapsulate your code. This prevents other scripts from interacting with your variables and functions. An IIFE looks something like the following, and is invoked with the variables it depends on (e.g. a javascript library such as jQuery):

(function () {
  // Your code
})();

You have on inconsistent space in your function declaration at setInterval(function () {. Your variable title_ref is not written in camelcase, while the rest of the variables and functions in your code are written that way.

Other than that your code looks clean. Your variable names are clear and make it easy to understand what they contain.

\$\endgroup\$
  • 2
    \$\begingroup\$ Wow thanks for that response, learned a lot. \$\endgroup\$ – Matthew Beaudin May 7 '18 at 19:47
2
\$\begingroup\$

A couple of things:

  • You can use document.currentScript to get the current script tag, though this doesn't work in IE. See https://stackoverflow.com/questions/403967/how-may-i-reference-the-script-tag-that-loaded-the-currently-executing-script

  • Don't use global variables, especially with common names like start and speed. I would have getParams return the parameters. Even the name getParams could also cause problems. You could use an IIFE or namespace your methods.

  • You can access document.title directly rather than accessing the title tag.

  • I would compare the current title to the last value you set it to before updating it. That way you can detect if something on the page changes the title and use the new title.

\$\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.