patternjavascriptMinor
Updating an element's class based on the background color
Viewed 0 times
thebackgroundupdatingcolorelementbasedclass
Problem
I'm trying to build a generic thing that will update a fixed menu button's color to either 'black' or 'white' depending on which contrasts better with the background color. I've got it working, but I want to optimize the code.
I'm curious how I can improve organization. Am I optimally using ES6? Are there any opportunities to make the code more DRY?
Codepen
`/jshint esversion: 6 /
/*
** Helper Functions
*/
const helperFunctions = {
// Color contrast
getContrastYIQ: function getContrastYIQ(rgb) {
rgb = rgb.substring(4, rgb.length - 1)
.replace(/ /g, '')
.split(',');
const yiq = ((rgb[0] 299) + (rgb[1] 587) + (rgb[2] * 114)) / 1000;
return (yiq >= 128) ? 'black' : 'white';
},
// Get element position relative to viewport
offsetTop: function offsetTop(elem) {
const offset = {
top: 0,
left: 0
};
if (elem && elem.getBoundingClientRect) { // check if available
const rect = elem.getBoundingClientRect();
offset.top = rect.top;
offset.left = rect.left;
}
return offset.top;
},
// DOM Manipulation
addClass: function addClass(elem, classname) {
if (classname) {
if (elem.classList)
elem.classList.add(classname);
else
elem.className += ' ' + classname;
}
},
removeClass: function removeClass(elem, classname) {
if (classname) {
if (elem.classList)
elem.classList.remove(classname);
else
elem.className = elem.className.replace(new RegExp('(^|\\b)' + classname.split(' ').join('|') + '(\\b|$)', 'gi'), ' ');
}
}
};
/*
** Init
*/
const sections = document.querySelectorAll('section');
const sectionCollection = [];
const menu = document.querySelector('.menu-icon');
const menuOffset = helperFunctions.offsetTop(menu);
// Setting up section obejcts
for (let i = 0; i 0) ? window.getComputedStyle(sections[i - 1], null).getPropertyValue('background-color') : this.backgroundColor;
this.previousTextColor
I'm curious how I can improve organization. Am I optimally using ES6? Are there any opportunities to make the code more DRY?
Codepen
`/jshint esversion: 6 /
/*
** Helper Functions
*/
const helperFunctions = {
// Color contrast
getContrastYIQ: function getContrastYIQ(rgb) {
rgb = rgb.substring(4, rgb.length - 1)
.replace(/ /g, '')
.split(',');
const yiq = ((rgb[0] 299) + (rgb[1] 587) + (rgb[2] * 114)) / 1000;
return (yiq >= 128) ? 'black' : 'white';
},
// Get element position relative to viewport
offsetTop: function offsetTop(elem) {
const offset = {
top: 0,
left: 0
};
if (elem && elem.getBoundingClientRect) { // check if available
const rect = elem.getBoundingClientRect();
offset.top = rect.top;
offset.left = rect.left;
}
return offset.top;
},
// DOM Manipulation
addClass: function addClass(elem, classname) {
if (classname) {
if (elem.classList)
elem.classList.add(classname);
else
elem.className += ' ' + classname;
}
},
removeClass: function removeClass(elem, classname) {
if (classname) {
if (elem.classList)
elem.classList.remove(classname);
else
elem.className = elem.className.replace(new RegExp('(^|\\b)' + classname.split(' ').join('|') + '(\\b|$)', 'gi'), ' ');
}
}
};
/*
** Init
*/
const sections = document.querySelectorAll('section');
const sectionCollection = [];
const menu = document.querySelector('.menu-icon');
const menuOffset = helperFunctions.offsetTop(menu);
// Setting up section obejcts
for (let i = 0; i 0) ? window.getComputedStyle(sections[i - 1], null).getPropertyValue('background-color') : this.backgroundColor;
this.previousTextColor
Solution
Two things stand out to me right away.
First, your
I'd also consider extracting the color-string parsing, since - if you're doing things with color - that might be useful later on. That'd reduce your function to one that only calculates the Y component, and done.
Secondly, rather than rolling your own
One thing your code doesn't take into account is transparency. If, for instance, the background for the text is more or less transparent, you'd basically need to calculate the composited color of the background on top of its background, etc. etc.. I wouldn't recommend trying to do all that, but rather just make it clear that it's a limitation of the code.
First, your
getContrastYIQ function doesn't so much return a "contrast" metric; it returns a color directly. I'd prefer the function to just calculate the Y (luma) component of the YIQ color, and let another piece of code decide what to do with that information.I'd also consider extracting the color-string parsing, since - if you're doing things with color - that might be useful later on. That'd reduce your function to one that only calculates the Y component, and done.
Secondly, rather than rolling your own
addClass removeClass, I'd consider using the existing classList API. Sure, support is spotty in Internet Explorer, but adding/removing a single class works.One thing your code doesn't take into account is transparency. If, for instance, the background for the text is more or less transparent, you'd basically need to calculate the composited color of the background on top of its background, etc. etc.. I wouldn't recommend trying to do all that, but rather just make it clear that it's a limitation of the code.
Context
StackExchange Code Review Q#157072, answer score: 2
Revisions (0)
No revisions yet.