Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MenuLists in portals cause page to scroll with React 18 #956

Open
Emilios1995 opened this issue Aug 11, 2022 · 7 comments
Open

MenuLists in portals cause page to scroll with React 18 #956

Emilios1995 opened this issue Aug 11, 2022 · 7 comments

Comments

@Emilios1995
Copy link

🐛 Bug report

Current Behavior

When rendering menu buttons that open menu lists on slightly long pages, opening a menu after scrolling down a bit often causes the page to scroll farther down. This only happens when rendering the menu lists inside portals, and when using React 18.

This screen recording shows the behavior happening in the reproducible example.

Expected behavior

The menu should open without any change in the page's scroll position.

Reproducible example

Example in Code Sandbox

Your environment

Software Name(s) Version
Reach Package menu-button 0.17.0
React react 18.2.0
Browser Chrome 104.0.5112.79
Browser Firefox 103.0.1
Assistive tech - -
Node - -
npm/yarn - -
Operating System MacOS 12.4
@gfohl
Copy link

gfohl commented Aug 23, 2022

I have experienced this too

@jeanpan
Copy link
Contributor

jeanpan commented Aug 27, 2022

I believe I fixed this issue in this PR. You can also check my issue here. Hope we can get the patch released soon.

@mena234
Copy link

mena234 commented Aug 29, 2022

I have the same issue.

@spirosikmd
Copy link

spirosikmd commented Sep 3, 2022

I have this issue with or without using the portal prop in the MenuList.

One way to fix this issue before the team releases the patch is to use a function to render the Menu's children and only render the MenuList when isExpanded is true.

<Menu>
  {({ isExpanded }) => (
    <>
      <MenuButton>Actions</MenuButton>
      {isExpanded ? (
        <MenuList portal>
          <MenuItem onSelect={() => alert("Download")}>Download</MenuItem>
          <MenuItem onSelect={() => alert("Copy")}>Create a Copy</MenuItem>
          <MenuItem onSelect={() => alert("Delete")}>Delete</MenuItem>
          <MenuLink as="a" href="https://reacttraining.com/workshops/">
            Attend a Workshop
          </MenuLink>
        </MenuList>
      ) : null}
  </>
)}
</Menu>

@gfohl
Copy link

gfohl commented Oct 6, 2022

@spirosikmd your fix worked, the patch helped, but still caused some scroll issues the first time it was opened. Thank you very much

@spirosikmd
Copy link

The only potential issue I see is that in development, I see a warning from the reach-rect package when I open the menu: You need to place the ref. I am unsure whether this causes any issues. The menus behave as expected as far as I can see.

@JaceHensley
Copy link

I'm trying the suggestion above in a custom select component that uses Popover and DescendantProvider to wire up downshift. The problem I'm seeing is that until the menu is opened no descendants are registered, so I cannot grab the selected item's value to display. Has anyone worked around an issue like that? Or another workaround to the root issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants