@reach/menu-button: Consider exposing outer menu div in the API

[chaance]

Per #148, there is currently no way to pass props to the outer menu div. While not initially intended as a display component, this could be potentially useful in some scenarios. We may want to consider an additive API change to make this possible.

[brendancarney]

Bringing my comment form #148 over to here where it's probably more useful:

---

The use case is adding padding to the wrapper and setting a background on it so that I can have a scroll bar that looks like this:

I don't think this would be possible without the wrapper, but I'm not 100% sure.

Maybe someday there could be a lower level implementation to hook into instead, so that odd use cases like this could build their own tree? Some useMenu() hook or something.

I did end up with a pretty hacky workaround, which is to modify the styles of that node whenever the menu is opened.

[brendancarney]

Maybe the lower level solution ultimately involves controlling how the portal and positioning is rendered? I could see this logic being useful for other components, like a Popover.

Rough idea: maybe if a user specifies a PositionedPortal in the tree, then the child of that is used for the menu positioning, giving freedom to structure the DOM however. The normal API would be the same, and automatically wrap the MenuList in a portal.

<Menu>
  <MenuButton>Open</MenuButton>
  <PositionedPortal>
    <div> // this div gets the positioning props
      <MenuList/>
    </div>
  </PositionedPortal>
</Menu>

[CodingDive]

Love the discussion so far. I would also appreciate a lower level API using hooks/props for:

• hiding/showing the menu button (see user land implementation here)
• opt out of auto focus e.g when using @reach/menu-button as a dropdown menu for autocompletion like GitHub offers when typing '@username' or ':emoji'.
• additional styling

[ripley-at-cascade]

Also in favor of this possible change - I'm running into an issue where the menu is being implemented within a nav bar that has a z-index set that overrides the positioning of the [data-reach-menu]. Since I'm using styled components, I haven't found a way to override or add an additional z-index to the [data-reach-menu] so that it can sit above the whole nav bar.

[chaance]

Playing with this as a lower level API solution. This would allow you to pass props into the container and add additional arbitrary nested components if you need to for whatever reason without exposing the Portal or positioning logic.

<Menu>
  <MenuButton>Open</MenuButton>
  <MenuListContainer style={{ borderColor: 'red' }}>
    <div className="arbitrary-div"> 
      <MenuListInner>
        {/* ...items */}
      </MenuListInner>
    </div>
  </MenuListContainer>
</Menu>

[ripley-at-cascade]

Would that MenuListContainer still be contained within the div that gets created in the (i.e. the div with the [data-reach-menu] attribute) or would this essentially be that div and now be able to have attributes passed to it?

[chaance]

@lindsaythegroves This would be that div. Naming of data-attributes would probably change (eventually) for the sake of consistency. In general we like to stay as close to the DOM as possible, so providing a low-level solution here fits with that approach.

[ripley-at-cascade]

@chancestrickland That sounds great!

@brendancarney

I did end up with a pretty hacky workaround, which is to modify the styles of that node whenever the menu is opened.

How did you target that particular node? I've been trying to do that as well but it's been returning null since it doesn't see that node as existing on click of the menu to open the dropdown (so maybe checking on click is my issue). Are you checking if the menu is open and then accessing that node?

[chaance]

Talked to Ryan about this and we settled on the same API posted above with minor naming tweaks. This will go out in the next release.

<Menu>
  <MenuButton>Open</MenuButton>
  <MenuPopover style={{ borderColor: 'red' }}>
    <div className="arbitrary-div"> 
      <MenuItems>
        {/* ...items */}
      </MenuItems>
    </div>
  </MenuPopover>
</Menu>

[brendancarney]

@lindsaythegroves here's the function I use:

function hackyFixForReachMenuStyles() {
  setTimeout(() => {
    const node = document.querySelector("[data-reach-menu]");
    if (node) {
      node.style.background = "rgb(64, 78, 92)";
      node.style.padding = 0;
    }
  }, 0);
}

I call this in the onClick for <MenuButton>. I'm guessing I put the setTimeout there to solve the node being null issue you were having, but I don't remember 🙂.

---

@chancestrickland the updated API looks great, thank you so much!

[ripley-at-cascade]

@brendancarney Ah, that timeout might be the thing I was missing - thanks for sharing!

And yes, the updates look great @chancestrickland - so I may not need to worry about checking for nodes anymore :) Thanks so much for doing this!

[chaance]

This is now apart of the MenuButton API as of 0.5.0 #316 (comment). @lindsaythegroves @brendancarney Take a look and let me know if you have any issues with implementation here!

[ripley-at-cascade]

@chancestrickland Thanks! In trying to check it out, I ran into a version issue since I had installed @reach/menu-button via npm and because that version is still at 0.4.0 I instead tried to use the github repo as a dependency. I tried via the command line with npm install --save reach/reach-ui but it complained that there wasn't a version in the package.json file so I'm not sure if there's an alternative way to do it?

[chaance]

@lindsaythegroves The latest published version on npm is at 0.5.0. Did you update the version required in your own package.json? https://www.npmjs.com/package/@reach/menu-button?activeTab=dependencies

[CodingDive]

@lindsaythegroves did you use the correct package name?

Instead of

npm install --save reach/reach-ui

try

npm install @reach/menu-button

[ripley-at-cascade]

@lindsaythegroves The latest published version on npm is at 0.5.0. Did you update the version required in your own package.json? https://www.npmjs.com/package/@reach/menu-button?activeTab=dependencies

I totally was just looking at the 0.4.0 version page, my bad:) It's now updated and seems to be working great so far.

[CodingDive]

I haven't seen this in the docs yet. I can add a PR but I'm having some trouble using the MenuPopover right now.

import styled from 'styled-components';
import {
  MenuPopover as _MenuPopover,
} from '@reach/menu-button';

export const MenuPopover = styled(_MenuPopover)`
  width: 100%;
  z-index: 1;
  background-color: red;
`;

Trying to style the popover like above, renders two reach portals and popovers. The first one shows the styles but does not have a height (and is not rendered).

while the other one is rendered to the DOM (has a height) with the default @reach-ui styles.

Is this a bug or am I missing something? Using @reach/menu-button v0.5.4

[InfraK]

I'm having the same issue as CodingDive

[chaance]

@CodingDive or @InfraK Do you mind opening a new issue with your specific problem? Happy to take a look, just don't want to lose track as I'm working through some other priorities this week!