iTwin / iTwinUI-react

A react component library for iTwinUI.
https://github.com/iTwin/iTwinUI
Other
83 stars 23 forks source link

Dropdown menu appearing under subsequent elements #500

Closed S-Bra closed 2 years ago

S-Bra commented 2 years ago

Dropdown hosted in a ButtonGroup that is opening across the table is unusable. As soon as the mouse pointer is within the table header, the header is drawn above the menu items. Mouse clicks do not reach the menu item anymore.

Expected behavior

Dropdown menu item stay visible on top of the table and can be selected.

Reproduction

https://codesandbox.io/s/itwinui-react-more-dropdown-unusable-s1gof?file=/src/App.tsx

Steps to reproduce
  1. Open the dropdown menu by clicking the "More" button

Additional information

Change @itwin/itwinui-react version to 1.27.0 to see previous behavior

bentleyvk commented 2 years ago

Also, Stephan found that <ButtonGroup style={{zIndex: 1}}> is fixing the issue.

mayank99 commented 2 years ago

Also, Stephan found that <ButtonGroup style={{zIndex: 1}}> is fixing the issue.

That will have side effects. For a cleaner workaround, I recommend adding appendTo={() => document.body} to the DropdownMenu. This is documented in our FAQ.

The root cause of this issue is the stacking context created by ButtonGroup combined with the popover being placed inside the ButtonGroup. These two ideas work against each other, leading to this frustrating situation. By placing the popover as a child of body, we can move it out of the ButtonGroup's stacking context.

S-Bra commented 2 years ago

I confirm that adding appendTo={() => document.body } to the DropdownMenu fixed the issue

veekeys commented 2 years ago

@mayank99 Would this be a part of your button group fixes? If not, seems there is a workaround for this issue, right?

mayank99 commented 2 years ago

Would this be a part of your button group fixes? If not, seems there is a workaround for this issue, right?

Since we haven't decided what to do with button group, it's hard to tell. But yes, there is a workaround. We were also talking about including it by default in #514

veekeys commented 2 years ago

Ok, @S-Bra are you ok with current suggestion? Can we close this issue?

mayank99 commented 2 years ago

@S-Bra This is now fixed in 1.32.0 so you can remove the workaround