- Does my code use good JS/ES6 programming practices and proper coding style? (I tried to follow the airbnb style guide for this)
For the most part I would say Yes. It caches DOM references in variables, though those could be stored in constants:
let hexInput = document.getElementById("hex");
let rgbInput = document.getElementById("rgb");
Because those are never re-assigned, it is wise to default to using the const keyword until a good reason for re-assingment arises.
const hexInput = document.getElementById("hex");
const rgbInput = document.getElementById("rgb");
- Is my code well structured and well written?
For the most part I would say Yes. However I do notice that the convert functions create a local variable (e.g. let rgbColorValues = [];) and then if the input value isn't valid, an empty string is returned. One could argue that the valid input check should come before declaring the local variable, lest memory be wasted (though in a small SPA like this it will likely be negligible).
- Should I give the IIFE function in my JS a name, so that it can be tested using a framework like Mocha (Maybe if I wanted to extract these functions into a small plugin), or should this code be structured differently in order to make it more maintainable and testable? (I believe I currently cannot access these functions inside this anonymous function)
I don't think giving it a name would be useful, though if you wanted to expose the methods, then perhaps the revealing module pattern would be good to follow for that.
##Other feedback
// I've heard of something called arrow functions, how could
// I use those here?
The biggest change would be removing the keyword function, and adding an arrow (i.e. =>) between the (empty) list of arguments and the function block (since there are multiple statements). Then because an arrow function "does not have its own this"1, this now refers to the enclosing scope, so change this.value to hexInput.value (in the first callback function, and then rgbInput.value in the second event callback function).
hexInput.addEventListener("change", () => {
if (isValidColor(hexInput.value)) {
rgbInput.value = convertHexToRGB(hexInput.value);
document.body.style.backgroundColor = hexInput.value;
}
});
Then another area that could be simplified using arrow functions is the end of the function isValidColor - specifically this block:
for (const color of rgbColorValues) {
if (color < 0 || color > 255) {
return false;
}
}
return true;
Using a functional approach, an array method like find() could be used to look for any value that is outside that 0-255 range. If any such value is returned, then return false, otherwise if undefined is returned, return true:
return rgbColorValues.find(function(color) {
return color < 0 || color > 255;
}) === undefined;
And that can be simplified using an arrow function - to one line:
return rgbColorValues.find(color => color < 0 || color > 255) === undefined;