I've created a grid component in React. I have an array of strings named 'availableColors' in which I am storing the css class names I want to use.
In the 'RandomColorGrid' component I'm setting the initial colors of each grid item in 'useState', assigning an index from 'availableColors' to each item.
Each item in the grid calls 'changeColors()' onClick. Inside that method I reassign the calue of each 'box' in 'colors' with a new randomly chosen index from 'availableColors'.
This works well enough but feels a little clunky. There are two things I am trying to improve but am getting stuck.
First; I would like to use each color only once when the 'changeColors()' function is called. Currently it's possible for the same color to be used on more than one grid item and I would like them to be four unique colours each time.
Second; I would like for no item to be the same color twice in a row. So for any given item I would line to exclude that item's current color from the possible random choices.
I've been trying to achieve this by taking the color index of the current color and forming a new array of colors to randomly select from for each item and then another array to try and track the colors that have already been used in order to avoid duplicates but in doing so have gotten into a real mess. This leads me to believe that my design is probably bad from the start.
How can I improve on this?
import React, { useState } from "react";
const availableColors = ["red", "green", "blue", "yellow"];
const changeColors = (colors, setColors) => {
colors.box1 = availableColors[randomNumber(colors.box1)];
colors.box2 = availableColors[randomNumber(colors.box2)];
colors.box3 = availableColors[randomNumber(colors.box3)];
colors.box4 = availableColors[randomNumber(colors.box4)];
setColors({ ...colors });
};
const randomNumber = (currentColour) => {
let indices = [0, 1, 2, 3];
indices.splice(availableColors.indexOf(currentColour), 1);
return indices[Math.floor(Math.random() * indices.length)];
};
export const RandomColorGrid = () => {
let [colors, setColors] = useState({
box1: availableColors[0],
box2: availableColors[1],
box3: availableColors[2],
box4: availableColors[3],
});
return (
<div className="grid">
<div
className={`${colors.box1}`}
onClick={() => changeColors(colors, setColors)}
/>
<div
className={`${colors.box2}`}
onClick={() => changeColors(colors, setColors)}
/>
<div
className={`${colors.box3}`}
onClick={() => changeColors(colors, setColors)}
/>
<div
className={`${colors.box4}`}
onClick={() => changeColors(colors, setColors)}
/>
</div>
);
};
Your problems come from not respecting the immutability of objects. You change an object and rely on the object not changing in the next line (in changeColors)
The solution would be to copy new arrays of the available colors, and using .filter to make sure we dont repeat the same colors twice by replacing the new currentlyAvailableColors array to only include colors that are ok to use
const changeColors = (colors, setCurrentColours) => {
const nextColors = {};
let currentlyAvailableColors = [...availableColors];
nextColors.box1 = getRandomOption(colors.box1, currentlyAvailableColors)
currentlyAvailableColors = currentlyAvailableColors.filter(col => col !== nextColors.box1);
nextColors.box2 = getRandomOption(colors.box2, currentlyAvailableColors)
currentlyAvailableColors = currentlyAvailableColors.filter(col => col !== nextColors.box2);
nextColors.box3 = getRandomOption(colors.box3, currentlyAvailableColors)
currentlyAvailableColors = currentlyAvailableColors.filter(col => col !== nextColors.box3);
nextColors.box4 = getRandomOption(colors.box4, currentlyAvailableColors)
currentlyAvailableColors = currentlyAvailableColors.filter(col => col !== nextColors.box4);
setCurrentColours({ ...nextColors });
};
Heres a working codepen https://codepen.io/yftachman/pen/XWZMqVZ