Changed style onClick applies to all the ListItems instead of single ListItems

Viewed 331

I am developing a menu item the code of is provided below. I am trying to change the style of the menu item when it is clicked. I have applied a function clickHandler to toggle between selected and not selected states. Based on the state of the menuItem, I want to change its style. In current scenario, if I clicked on any ListItem, the changed style is applied to all the ListItems instead of one. How can I achieve it?

import React, {Fragment, useCallback, useState} from 'react';
import {makeStyles, withStyles} from '@material-ui/core/styles';
import AppBar from '@material-ui/core/AppBar';
import {Link} from "react-router-dom";
import ListItemText from "@material-ui/core/ListItemText";
import ListItem from "@material-ui/core/ListItem";
import Typography from "@material-ui/core/Typography";
import Toolbar from "@material-ui/core/Toolbar";
import Box from "@material-ui/core/Box";
import List from "@material-ui/core/List";
import clsx from "clsx";

const useStyles = makeStyles((theme) => ({
  root: {
    width: '100%',
    backgroundColor: theme.palette.background.paper,
  },
  appBar: {
    shadowColor: '#206a5d',
    margin: 0
  },
  active: {
    color: '#206a5d',
    borderBottom: '2px solid blue'
  },
}));


export function Submenu({items, header}) {
  const classes = useStyles();
  const [active, setActive] = useState(false)

  const clickHandler = () => {
    setActive((prev) => !prev);
  };



  return (
    <Fragment>
    <div className={classes.root}>
      <AppBar position="sticky"
              color="inherit"
              className={classes.appBar}
      >
        <Typography style={{marginLeft: '1.5rem'}}>
          <h1 style={{color:'#1f4068'}}>{header}</h1>
        </Typography>
        <div style={{display: 'flex',}}
        >
          {items.map(item =>
          <ListItem button
                    key={item.title}
                    onClick={clickHandler}
                    component={Link}
                    className={clsx(null, {[classes.active]: !active})}
                    to={item.to || '/404'}
                    style={{padding:'0.75rem', width: 'auto'}}
          >
            {item.title}
          </ListItem>
        )}
        </div>
      </AppBar>
    </div>
      </Fragment>
  );
}
4 Answers

If you want to set the style of a single ListItem, you need to provide an identifier as a parameter to clickHandler so that you can identify which one is clicked and according to that you can change style.

That is happening, because your onClick function is triggering a re-render due to the change in state, so all of your ListItems get re-rendered with the new "active" state.

Instead of holding one single value in the state, you should create an array that holds the active state for all of your ListItems, like

[
  {itemID: "item1", active: false}, 
  {itemID: "item2", active: false}, 
  {itemID: "item3", active: true}
]

That is the default initial state of the list items, so if you want all of them to be inactive, set all active to false.

And when you are rendering with the .map, you just check wether the current item that is being rendered has its active set to true or false and change the class for only that item.

That would also require you to pass additional parameters to your onClick function, you do that by creating an anonymous function and not calling it:

onClick={() => { ClickHandler(idOfCurrentListItem, param2, param3...) }}

So it would look something like this (mind you im doing this from the top of my head, dont have an editor infront of me :P):

allItems.map(item => {
  return <ListItem onClick={() => { ClickHandler(item.itemID) }}></ListItem>
});

And in the ClickHandler:

const ClickHandler = (id) => {
  setItemsState(prevItems => {

    // looping through all items from the initial state, and when we reach the item that matches the 
    // id passed to the click handler, we change only its state to true
    // otherwise we return the same item without change
    return prevItems.map(item => {
      if (item.id === id) return { ...item, item.active: true }
      
      return item;
    });
  });
}

Another approach would be to move the onClick inside the ListItem component itself, so that each ListItem handles its own state, and then you can do it the simpler way, like you've been trying to do it. That owuld also probably be the better option as that would not trigger a re-render of the whole menu, but just of that particular ListItem.

the issue here is that your state from the useState for active is a global state for all the list item.

So even if you click any list item, it is changing the state active which is given to all the list items.

So solve this, you can make the state for setting active styles and clickHandler in the ListItem component.

That would solve your issue

TLDR

At that situation, You can use string or object for indicate what is selected

Answer

Simply you can use activeKey and setActiveKey. And you should indicate with item.title

Code

import React, {Fragment, useCallback, useState} from 'react';
import {makeStyles, withStyles} from '@material-ui/core/styles';
import AppBar from '@material-ui/core/AppBar';
import {Link} from "react-router-dom";
import ListItemText from "@material-ui/core/ListItemText";
import ListItem from "@material-ui/core/ListItem";
import Typography from "@material-ui/core/Typography";
import Toolbar from "@material-ui/core/Toolbar";
import Box from "@material-ui/core/Box";
import List from "@material-ui/core/List";
import clsx from "clsx";

const useStyles = makeStyles((theme) => ({
  root: {
    width: '100%',
    backgroundColor: theme.palette.background.paper,
  },
  appBar: {
    shadowColor: '#206a5d',
    margin: 0
  },
  active: {
    color: '#206a5d',
    borderBottom: '2px solid blue'
  },
}));


export function Submenu({items, header}) {
  const classes = useStyles();
  const [activeItem, setActiveItem] = useState("")

  const clickHandler = useCallback((itemKey) => () => {
    setActiveItem((itemKey));
  },[]);



  return (
    <Fragment>
    <div className={classes.root}>
      <AppBar position="sticky"
              color="inherit"
              className={classes.appBar}
      >
        <Typography style={{marginLeft: '1.5rem'}}>
          <h1 style={{color:'#1f4068'}}>{header}</h1>
        </Typography>
        <div style={{display: 'flex',}}
        >
          {items.map(item =>
          <ListItem button
                    key={item.title}
                    onClick={clickHandler(item.title)}
                    component={Link}
                    className={clsx(null, {[classes.active]: item.title === activeKey})}
                    to={item.to || '/404'}
                    style={{padding:'0.75rem', width: 'auto'}}
          >
            {item.title}
          </ListItem>
        )}
        </div>
      </AppBar>
    </div>
      </Fragment>
  );
}

ETC

But more efficiently It is good to use object like other people say. But If you are newbie in react I think it is good to use either string type and object !

And you can sometime realize that what is more efficient to use

Related