Array.prototype.forEach does not do what you think it does.
- It invokes the inner function for every element in the array.
- Crucially (in your case), it does not support returning values, nor finding elements in an array.
Also, in general: don't use Array.prototype.forEach.
- It isn't intended to be used as a pure function (unlike
Array.protoype.map or Array.prototype.filter) which makes it harder to reason about (e.g. what happens if you modify the array while inside the forEach callback?).
- Also, it's impossible to control iteration inside the body, and careless use of it often inadvertently captures state in its closure which is a frequent source of bugs.
Array.prototype.forEach exists only because earlier versions of JavaScript didn't have a simple way to iterate through general collection-types.
- Instead, use the modern
for( of ) statement instead.
Also, your logic in your .forEach inner func can be moved to the selector:
c.classList.contains('color__active') is the same as using .color__active in the selector.- The check for a non-falsy (i.e. non-empty)
c.dataset.color can be added to the CSS selector by using the "has non-empty attribute" selector: [data-color]:not([data-color=""i])
- Surprisingly CSS doesn't have an "attribute-value not-equal-to X" selector, but you can use
:not([attribute-name="value"]) as a logically-equivalent, if more verbose, alternative.
- And you should add the
button element type to the selector too:
- So the final selector is:
button.product__color.color__active[data-color]:not([data-color=""i])
Then just use document.querySelector to grab the first element (rather than all elements) that match the selector instead of using querySelectorAll.
- Though it would be a good idea to check if there are multiple elements matching the selector and then
throw new Error(), just to be safe.
So change your code to just this:
const firstColorButtonOrNull = document.querySelector( 'button.product__color.color__active[data-color]:not([data-color=""i])' ) ?? null;
const color = firstColorButtonOrNull?.dataset['color'] ?? null;
- The
?? null at the end of each line is to coalesce undefined to null which is often easier to process in JS.
- The
?. is the safe chaining operator (also known as the Elvis operator or the safe navigation operator in other languages).
- So if
firstColorButtonOrNull is null or undefined, then the expression evaluates to undefined instead of crashing.
- The second
?? null at the end is so that color will be null instead of undefined.
...which can then be simplified down to a single-liner:
const firstColorOrNull = document.querySelector( 'button.product__color.color__active[data-color]:not([data-color=""i])' )?.dataset['color'] ?? null;
Consider using defensive programming practices:
This alternative implementation below (getFirstActiveColorOrNull) ensures there's only ever 0 or 1 elements with .product__color otherwise it throws an error. This approach is an example of defensive programming, and should be used if you care about correctness and self-documenting code (as it demonstrates intent: as with your current code it's unclear to anyone else if product__color is only meant to be used at-most once or not).
function getFirstActiveColorOrNull() {
const selector = 'button.product__color.color__active[data-color]:not([data-color=""i])';
function getFirstOrNull( elements, projection ) {
if( colorButtons.length == 0 ) {
return null;
}
else if( colorButtons.length == 1 ) {
return projection( colorButtons[0] );
}
else { // Implicit: colorButtons.length > 1
throw new Error( "Encountered multiple elements matching selector \"" + selector + "\". This should never happen." );
}
}
const colorButtons = document.querySelectorAll( selector );
return getFirstOrNull( colorButtons, el => el.dataset.color );
}
//
function makeMeActive( button ) {
const otherActiveElements = document.querySelectorAll( '.color__active' );
for( const el of otherActiveElements ) el.classList.remove( 'color__active' );
button.classList.add( 'color__active' );
}
.color__active {
outline: 5px solid blue;
}
<div class="flex items-center space-x-4">
<button
class="product__color color__active flex h-5 w-5 items-center justify-center rounded-full text-white"
style="background-color: #000; color: white;"
data-color="#000"
onclick="makeMeActive(this)"
>
Black
</button>
<button
class="product__color flex h-5 w-5 items-center justify-center rounded-full text-white"
style="background-color: #ff0000"
data-color="#ff0000"
onclick="makeMeActive(this)"
>Red</button>
</div>
<hr />
<p>Click a button to make it "active": it will be given a blue border.</p>
<button onclick="console.log( getFirstActiveColorOrNull() )">Click me to print active color to console</button>
