1

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>
  );
};
DukeSmellington
  • 83
  • 8
  • How is it possible to change to a color that is not used if you have 4 boxes and 4 colors? You either stay the same color or change to a color that already exists – Yftach May 18 '22 at 00:06
  • 1
    [This post](https://stackoverflow.com/a/2450976/14596856) might be useful. Start with `indices = [0, 1, 2, 3, ... n]`(more colors is better) and call `shuffle(indices)`, take first 4 items to color those 4 boxes, this gives 1st condition. Check for 2nd condition and swap elements if the colors of some did not change. – Vitalii May 18 '22 at 00:21

1 Answers1

2

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

Yftach
  • 712
  • 5
  • 15
  • "not respecting the immutability of objects" has been a recurring problem for me - thankyou for the input. This solution works really well. I was having a problem with the last square occasionally coming up with no colour but fixed it by adding an extra colour to the array. Thankyou – DukeSmellington May 18 '22 at 05:20
  • 1
    Thats a common issue, just something to take notice of. Also, in general i would do a different solution where i would not store the boxes colors in an object but in an array, then move the box component to a new component and run [...].map((color)=> ) would make it cleaner – Yftach May 18 '22 at 12:41