JavaScript Colour Converter

Question

I've made a little colour conversion app, and below is my JavaScript.

The app converts HEX values to RGB values and vice versa, as well as using a simple colour picker to get colours and convert them to both HEX and RGB.

Here is a picture so it is easier to understand what the code does. Basically you enter a HEX or RGB value into HTML input fields, and then the converted RGB/HEX value gets printed out.

You can also choose a colour from the 'Pick A Colour' section, and the colour selected also gets converted into HEX/RGB:

Does anyone have any suggestions to shorten this?

var svgPath = document.getElementById('path');
var colorInput = document.getElementById('color-input');
var hexOutput = document.getElementById('hex-output');
var rgbOutput = document.getElementById('rgb-output');
var rgb = document.getElementById("rgb");
var rgb2 = document.getElementById("rgb2");
var rgb3 = document.getElementById("rgb3");

// HEX to RGB
function hexToRGB(hex) {
    var bigint = parseInt(hex, 16);
    var r = (bigint >> 16) & 255;
    var g = (bigint >> 8) & 255;
    var b = bigint & 255;
    return r + ', ' + g + ', ' + b;
}

// RGB to HEX
function rgbToHEX(red, green, blue) {
    var rgb = blue | (green << 8) | (red << 16);
    return '#' + (0x1000000 + rgb).toString(16).slice(1);
}

// Gets HEX input 
document.getElementById("hex").addEventListener("input", function() {
    // Sets colours for SVG, picker, converts HEX to RGB and prints out HEX
    svgPath.style.fill = '#' + this.value;
    colorInput.value = '#' + this.value;
    hexOutput.innerHTML = 'HEX: #' + this.value.toUpperCase();
    rgbOutput.innerHTML = 'RGB: rgb(' + hexToRGB(this.value) + ')';
}, false);

// Selects elements under .rgb class
var rgbInput = document.querySelectorAll('.rgb');
for(var i = 0; i < rgbInput.length; ++i){
    rgbInput[i].addEventListener("input", rgbCalc);
}

function rgbCalc() {
    // Skips to next input box if 3 characters have been entered
    if (rgb.value.length >= this.maxLength) {
        rgb2.focus();
    }
    if (rgb2.value.length >= this.maxLength) {
        rgb3.focus();
    }

    // Stores the r, g, b values
    var r = rgb.value;
    var g = rgb2.value;
    var b = rgb3.value;

    // Sets colours for SVG, picker, converts RGB to HEX and prints out RGB
    svgPath.style.fill = rgbToHEX(r, g, b);
    colorInput.value = rgbToHEX(r, g, b);
    rgbOutput.innerHTML = 'RGB: rgb(' + r + ', ' + g + ', ' + b + ')';
    hexOutput.innerHTML = 'HEX: ' + rgbToHEX(r, g, b).toUpperCase();
}

// Gets colour picker input value
colorInput.addEventListener("input", function() {
    // Sets colours for SVG, converts Hex to RGB and prints out HEX
    svgPath.style.fill = colorInput.value;
    hexOutput.innerHTML = "HEX: " + colorInput.value.toUpperCase();
    rgbOutput.innerHTML = "RGB: rgb(" + hexToRGB(colorInput.value.replace('#', ''))  + ')';
}, false);

Answer 1

Nice code, only a few comments to offer here:

.1.

var rgb = document.getElementById("rgb");
var rgb2 = document.getElementById("rgb2");
var rgb3 = document.getElementById("rgb3");

This way of keeping the r, g, and b is not the best. Consider having to keep six of them, or more. You would not do this that way. You cannot run a loop with this and this gets obvious in

    if (rgb.value.length >= this.maxLength) {
        rgb2.focus();
    }
    if (rgb2.value.length >= this.maxLength) {
        rgb3.focus();
    }

So what I would do is:

var getEl = document.getElementById;
var rgbElements = {
                   r:getEl("rgb"),
                   g:getEl("rgb2"),
                   b:getEl("rgb3")
                  }

This way you could run a loop for these.

.2. This code

document.getElementById("hex").addEventListener("input", function() {
    // Sets colours for SVG, picker, converts HEX to RGB and prints out HEX
    svgPath.style.fill = '#' + this.value;
    colorInput.value = '#' + this.value;
    hexOutput.innerHTML = 'HEX: #' + this.value.toUpperCase();
    rgbOutput.innerHTML = 'RGB: rgb(' + hexToRGB(this.value) + ')';
}, false);

should be like this one

document.getElementById("hex").addEventListener("input", function() {
    // Sets colours for SVG, picker, converts HEX to RGB and prints out HEX
    var hexVal = this.value;
    svgPath.style.fill = '#' + hexVal;
    colorInput.value = '#' + hexVal;
    hexOutput.innerHTML = 'HEX: #' + hexVal.toUpperCase();
    rgbOutput.innerHTML = 'RGB: rgb(' + hexToRGB(hexVal) + ')';
}, false);

You should not 'ask' for values from 'this' or higher scope when you can have it closer.

Answer 2

Nice app. Here are some minor nits and optional tips:

1. Some developers will do var Element = document.getElementById; so that getting an element by ID requires less typing. This is completely up to you.

2. Comments should explain things that are not immediately obvious from the code.

   // Stores the r, g, b values
   var r = rgb.value;
   var g = rgb2.value;
   

   This is not really a useful comment, since it adds no value to the understanding of the reader.

Otherwise, this is clear and easy to read.