szhsin / react-menu

React component for building accessible menu, dropdown, submenu, context menu, and more.
https://szhsin.github.io/react-menu/
MIT License
1.15k stars 58 forks source link

Wrapping multiple menu components in custom components #494

Closed domipieg closed 2 years ago

domipieg commented 2 years ago

React/React-dom version: 16.8.5 React-Menu version: 2.3.4

Hi, Is possible to wrap several menu items (and sub menus) in the way presented below? My context menu component become bloated and I would like to separate views and logic to smaller components. In the documentation I see only a way how to wrap single component: MenuItem/SubMenu/FocusableItem. I need to place several components in wrapper component. Is it posible?

const MyComponent = () => { return (

Cut
  <MenuItem>New File</MenuItem>
  <MenuItem disabled>Save</MenuItem>

); };

const MyComponent2 = () => { return (

Option 1
  <MenuItem>Option 2</MenuItem>
  <MenuItem >Option 3</MenuItem>

); };

export default function App() { return ( <Menu menuButton={Open menu}> {someBoolean ? : } ); }

szhsin commented 2 years ago

Hi @domipieg , it's fine to render multiple items in wrapper component from React-Menu 3.0.0. In earlier versions, there can be only one MenuItem/SubMenu/FocusableItem rendered in a wrapper component.

However, you might not really need a wrapper component for this case. A function which returns multiple items in an array or React fragment would work from 2.0.1

The following code works from React-Menu 3.0.0

const MenuItems = () => {
  return (
    <>
      <MenuItem>Cut</MenuItem>
      <MenuItem>New File</MenuItem>
      <MenuItem>Save</MenuItem>
    </>
  );
};

<Menu>
  <MenuItems />
</Menu>;

The following code works from React-Menu 2.0.1

const getMenuItems = () => {
  return (
    <>
      <MenuItem>Cut</MenuItem>
      <MenuItem>New File</MenuItem>
      <MenuItem>Save</MenuItem>
    </>
  );
};

const getMenuOptions = (options) =>
  options.map((option) => <MenuItem key={option}>{option}</MenuItem>);

<Menu>
  {getMenuItems()}
  {getMenuOptions(["Option 1", "Option 2", "Option 3"])}
</Menu>;
domipieg commented 2 years ago

Thank you for your quick response. The solution works! I updated my react and react-menu versions, and I am able to wrap multiple menu items :)

However.. the solution with catching current direction (passing to menuClassName function to get direction) does not work properly. Problems are:

  1. after having direction state I am not able to persist it (Warning: setState(...): Cannot update during an existing state transition), because function is executed during render, apparently.
  2. I am getting then weird behaviour of switching direction infinitly (my function passed to menuClassName executes infinitely).

I would much appreciate any possibile solution in 3.0.0 version which would help me to get and save direction in which menu is opened.

If it is not good to continue this direction problem here, please let me know, I will then create separate issue (but it is connected to version change).

szhsin commented 2 years ago

menuClassName function runs in the React render phase so that it should be pure. It cannot update React state and should return the same result every time it's called given the same parameter. This is similar to render props.

It won't run infinitely unless React state is updated in the function. There is an internal memorisation to ensure the function is executed only when any of its parameter states has changed. When an inline function (arrow function) is provided to the menuClassName prop, the memorisation will be invalidated which causes the function to re-run whenever the component (App) rendering menu re-renders. This behaviour is harmless and won't cause problem in most situations. However, if you don't want the function to re-run, you could define the function outside react component scope or create a function using the useCallback hook.

It's not possible to save menu direction (in a React state) currently, but I'm happy to get further understanding of your use case thus I can assess if a new feature is worth or not, for example, you might want to show different menu items when menu directions have changed?

domipieg commented 2 years ago

Thank you for your response! I handled this in the possibly simplest way I suppose. I just pass anchorRef instead of anchorPoint. I found it after some time in the documentation. I wanted menu to not cover anchor element when it is opening sometimes in bottom sometimes in top direction. So anchorRef fulfills my need :)