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:

Converter

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.