0

I'm trying to build a menu bar where the user can hover over the options and the corresponding sub-menu shows.

I have implemented this behaviour using OnMouseEnter and OnMouseLeave. Everything works ok if I enter an item and then exit it without entering another but the problem arises when I move from one item to another directly. It seems that OnMouseLeave is not fully executed before calling OnMouseEnter on the new Item and this leads to the state not being correctly updated (this is a guess though).

Here is the code:

import React from 'react';
import { useEffect } from 'react';
import { useState } from 'react';
import { BsChevronDown } from 'react-icons/bs';
import IMenuItemData from './Menu/IMenuItemData';
import NavigationMenu from './Menu/NavigationMenu';


function NavigationBar({menuData}:{menuData:IMenuItemData[]}) {
    
    const [hoverStates, setHovering] = useState<any>({});
    
    function _onMouseEnter(id:string|undefined)
    {
        console.log("OnMouseEnter id -> "+id);
        console.log(hoverStates);
        if(id)
        {
            let newState:any = {...hoverStates};
            newState[id] = true;
            setHovering(newState);
        }
    }

    function _onMouseLeave(id:string|undefined)
    {
        console.log("OnMouseLeave id -> "+id);
        console.log(hoverStates);
        if(id)
        {
            let newState:any = {...hoverStates};
            newState[id] = false;
            setHovering(newState);
        }
    }

    useEffect(()=>{
        let states: any = {};
        menuData.forEach( (el:IMenuItemData) =>{
            if(el.id) states[el.id] = false;
        });
        
        setHovering(states);

    },[]);

    return (
        <nav className="flex">
            {menuData.map((e,index) => {
                    return<div key={index}>
                         <a href="#" onMouseEnter={()=>_onMouseEnter(e.id)} onMouseLeave={()=>_onMouseLeave(e.id)} className="flex p-2 items-center bg-red-400">
                            {e.text}
                            <BsChevronDown className="ml-2"></BsChevronDown>
                        </a>
                        {e.children && e.id ? <NavigationMenu  data={e.children} visible={hoverStates[e.id]}></NavigationMenu> : null}
                        </div>
                })}
        </nav>
    )
}


export default NavigationBar;


export default interface IMenuItemData
{
    text: string,
    children? : IMenuItemData[] ,
    id?: string
}

This just iterates through IMenuItemData objects and add them to the menu and adds a key to the state to track the hovering for every menu item.

This is the output when just entering an element and exiting without entering a new one:

OnMouseEnter id -> menu-item-store NavigationBar.tsx:15

Object { "menu-item-store": false, "menu-item-about": false, "menu-item-community": true, "menu-item-support": false }
NavigationBar.tsx:16

OnMouseLeave id -> menu-item-store NavigationBar.tsx:27

Object { "menu-item-store": true, "menu-item-about": false, "menu-item-community": true, "menu-item-support": false }
NavigationBar.tsx:28

And this the output that is logged when I leave a menu option but enters another immediately:

OnMouseEnter id -> menu-item-store NavigationBar.tsx:15

Object { "menu-item-store": false, "menu-item-about": false, "menu-item-community": true, "menu-item-support": false }
NavigationBar.tsx:16

OnMouseLeave id -> menu-item-store NavigationBar.tsx:27

Object { "menu-item-store": true, "menu-item-about": false, "menu-item-community": true, "menu-item-support": false }
NavigationBar.tsx:28

OnMouseEnter id -> menu-item-about NavigationBar.tsx:15

Object { "menu-item-store": true, "menu-item-about": false, "menu-item-community": true, "menu-item-support": false }
NavigationBar.tsx:16 <--- This should show 'menu-item-store': false

OnMouseLeave id -> menu-item-about NavigationBar.tsx:27

Object { "menu-item-store": true, "menu-item-about": true, "menu-item-community": true, "menu-item-support": false }
NavigationBar.tsx:28
Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
Notbad
  • 5,936
  • 12
  • 54
  • 100
  • The events aren't async but the setState calls are. You should be using callbacks in your setState calls to access the most recent value. `setHovering(prev => ({...prev, [id]: false/true})` – pilchard Jun 29 '21 at 11:23
  • This type of problem is, in my experience, much better solved by using CSS hover. I've tried using synthetic events for something very similar and it was slow and unpredictable. – Jimmy-P Jun 29 '21 at 11:25
  • @pilchard: 1) setState can be only used when using classes. I'm using functional components. 2) In the case I would be using classes I would run into the same problem because the problem is the reading of the state in each MouseEnter/MouseLeave event. The only thing I could is somehow cache the calls and call it in a synchronous way with something custom. This is a bit overengeniered to me :). – Notbad Jun 29 '21 at 11:31
  • 1
    @Notbad He means `setHovering`, that's the equivalent of setState. And it takes the callback version too. – Keith Jun 29 '21 at 11:32
  • @Keith I am pretty new to react, this is true but I haven't seen anywhere that I could pass a callback to the setXXXX function returned by the useState hook. To achieve something similar with functional components you must use a combination of useState/useEffect. – Notbad Jun 29 '21 at 11:36
  • @Jimmy-P I started this trying to use hover pseudo classes but things got hairy quickly. I wanted to emulate something like the navbar at https://www.gog.com/ – Notbad Jun 29 '21 at 11:40
  • @Notbad passing a callback to useState is in the documentation, [useState: Functional updates](https://reactjs.org/docs/hooks-reference.html#functional-updates) – pilchard Jun 29 '21 at 13:17
  • @pilchard there's no place in that documentation showing how you can pass a callback to useState. Read this answer to a related question: https://stackoverflow.com/questions/56247433/how-to-use-setstate-callback-on-react-hooks. You must use the useState/useEffect mechanism – Notbad Jun 29 '21 at 13:44
  • @Notbad copied directly from a snippet in the linked documentation `` -- **`setCount(prevCount => prevCount - 1)`** – pilchard Jun 29 '21 at 13:57
  • @pilchard don't want to be that guy, but what I understand from this part of the doc is that this function is used to be able to compute a value based on the previous state but there's no callback being called when effectively the value is changed. I could be wrong though. – Notbad Jun 29 '21 at 14:21
  • That is accurate, but also exactly what you need - to compute the new array state based on the previous state. As your Question stands, you have created a closure around the initial value of your array which both setXX() calls access, thus the clashes. – pilchard Jun 29 '21 at 14:27

2 Answers2

1

I'm not able to reproduce the issue.

Below is a simple working snippet, you could maybe tweak to show the issue.

const {useState} = React;

const mount = document.querySelector('#mount');

function HoverBox() {
  const [color, setColor] = useState('red');

  return <div
    onMouseEnter={() => setColor('green')}
    onMouseLeave={() => setColor('red')}
    style={{
      display: "inline-block",
      margin: "2px",
      width: "40px",
      height: "40px",
      backgroundColor: color
    }}
  ></div>;
}


ReactDOM.render(<div>
  <HoverBox/><HoverBox/><HoverBox/>
  <HoverBox/><HoverBox/><HoverBox/>
  
</div>, mount);
<script crossorigin src="https://unpkg.com/react@17/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@17/umd/react-dom.development.js"></script>

<div id="mount"></div>
Notbad
  • 5,936
  • 12
  • 54
  • 100
Keith
  • 22,005
  • 2
  • 27
  • 44
  • Thanks a lot for creating this. I thought the problem was because of the margin but it is not. Let me compare what I got later and will be back to you. I think the problem would arise when you change visibility of another div based on the hover state of the main div. I mean, what I have is a list of that when are hovered make a div that was hidden display:block, and display:hide when left. This div is the submenu. – Notbad Jun 29 '21 at 12:00
  • While editing the code I got why your method works. You are saving the state of every menu option in the menu option itself. So, it doesn't clash with any update made by sibling hovers. In my case I set the state of an object that has the information of all menu options in the navbar. So, the problem comes when a menu option is exited (it builds a new object with the spread operator and sets the state) and a new menu option is entered (it builds a new object with the spread operator and sets the state). I assume that the OnMouseEnter handler doesn't read the updated value from the OmMouseExit. – Notbad Jun 29 '21 at 12:22
  • I would like to do it this way because I feel the navbar is the mediator that should control children visibilities and not the other way around, children controlling their own visiblity states. But this method leads to the problem I exposed. – Notbad Jun 29 '21 at 12:24
1

A simple solution to keep visibility control in the nav component is to use an active flag. Adapted Keith's snippet below.

(Also, don't use index as key, use a unique property of the mapped element instead.)

const { useState } = React;

const mount = document.querySelector('#mount');

function Menu() {
  const [menuItems, setMenuItems] = useState([{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }, { id: 5 }, { id: 6 }]);
  const [active, setActive] = useState(undefined);

  return (
    <div>
      {
        menuItems.map(item => (
          <div
            key={item.id}
            onMouseEnter={() => setActive(item.id)}
            onMouseLeave={() => setActive(undefined)}
            style={{
              display: "inline-block",
              margin: "2px",
              width: "40px",
              height: "40px",
              backgroundColor: active === item.id? "pink": "yellow",
            }}
          ></div>
        ))}
    </div>
  )
}


ReactDOM.render(<Menu />, mount);
<script crossorigin src="https://unpkg.com/react@17/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@17/umd/react-dom.development.js"></script>

<div id="mount"></div>

Managing the visibility/active state inside each menuItem as per your question is possible, but verbose and may lead to clashes of state. (note that I am using a callback in the setMenuItems() call to access the most current previous value.)

const { useState } = React;

function Menu() {
  const [menuItems, setMenuItems] = useState([{ id: 1, visible: false }, { id: 2, visible: false }, { id: 3, visible: false }, { id: 4, visible: false }, { id: 5, visible: false }, { id: 6, visible: false }]);

  const handleOnMouseEnter = (id) => {
    const itemIndex = menuItems.findIndex(({ id: id_ }) => id_ === id);
    setMenuItems(prevMenuItems => [
      ...prevMenuItems.slice(0, itemIndex),
      { ...prevMenuItems[itemIndex], visible: true },
      ...prevMenuItems.slice(itemIndex + 1)])
  }

  const handleOnMouseLeave = (id) => {
    const itemIndex = menuItems.findIndex(({ id: id_ }) => id_ === id);
    setMenuItems(prevMenuItems => [
      ...prevMenuItems.slice(0, itemIndex),
      { ...prevMenuItems[itemIndex], visible: false },
      ...prevMenuItems.slice(itemIndex + 1)])
  }

  return (
    <div>
      {
        menuItems.map(item => (
          <div
            key={item.id}
            onMouseEnter={() => handleOnMouseEnter(item.id)}
            onMouseLeave={() => handleOnMouseLeave(item.id)}
            style={{
              display: "inline-block",
              margin: "2px",
              width: "40px",
              height: "40px",
              backgroundColor: item.visible ? 'pink' : 'yellow',

            }}
          ></div>
        ))}
    </div>
  )
}

ReactDOM.render(<Menu />, document.querySelector('#mount'));
<script crossorigin src="https://unpkg.com/react@17/umd/react.development.js"></script>
<script crossorigin src="https://unpkg.com/react-dom@17/umd/react-dom.development.js"></script>

<div id="mount"></div>
pilchard
  • 12,414
  • 5
  • 11
  • 23
  • Thanks for pointing out the key issue. I sometimes forget that :). I understand your solution and it could work. I need to try it and see if it fits. At this time, I ended moving the state of each option into the menu option menu. This way different states for the menu options do not clash but I loose the navbar hability to act as the controller of the different menu options. I will come back to this later. I need to move on :). – Notbad Jun 29 '21 at 14:15