Enabling hide middleware in DropdownMenu by default
#2233
Conversation
r100-stack
changed the title
hide middleware in DropdownMenumiddleware.hide prop in DropdownMenu
r100-stack
requested review from
mayank99 and
smmr-dn
and removed request for
a team
r100-stack
changed the title
middleware.hide prop in DropdownMenuhide middleware in DropdownMenu by default
| * @see https://floating-ui.com/docs/middleware | ||
| */ | ||
| middleware?: { | ||
| hide?: boolean; |
There was a problem hiding this comment.
Building upon #2233 (comment), I think middleware.hide should be exposed in all places that use usePopover (directly or indirectly).
There was a problem hiding this comment.
Exposed middleware.hide in DropdownMenu, ComboBox. Exposed popoverOptions.middleware.hide in custom Select (https://github.com/iTwin/iTwinUI/pull/2233/files/68bcf9f57d921713dffb83cee347a0e0d1afe313..0d2b8426dac7565686127edb819d4e7e153181bc#diff-81cc6906121a5abe5bd05042dc29b66dcd2eb7dfdd771cffeb61c7387420652f).
There was a problem hiding this comment.
It should be under dropdownMenuProps in ComboBox and under popoverProps in Select.
Co-authored-by: Mayank <9084735+mayank99@users.noreply.github.com>
| } from '@itwin/itwinui-react'; | ||
| import { useSearchParams } from '@remix-run/react'; | ||
| import { SvgMore } from 'node_modules/@itwin/itwinui-react/esm/utils/index.js'; |
There was a problem hiding this comment.
Do not rely on these internal paths; instead import from @itwin/itwinui-icons-react if you need this icon.
| @@ -3,7 +3,7 @@ | |||
| * See LICENSE.md in the project root for license terms and full copyright notice. | |||
| *--------------------------------------------------------------------------------------------*/ | |||
| import * as React from 'react'; | |||
| import { render } from '@testing-library/react'; | |||
| import { render, waitFor } from '@testing-library/react'; | |||
There was a problem hiding this comment.
Is waitFor being used in this unit test? I think it might be a stray import.
Changes
Fixes #2235.
This PR makes
DropdownMenuexpose themiddlewareprop. Thehidemiddleware is enabled by default and can be opted out from. (#2233 (comment)).Only the
hidemiddleware is modifiable for now. This keeps the API surface small, manageable, and less buggy. As and when we need to expose more middleware, we can do so in the future.Unrelated fixes:
hidemiddleware inPopoverto actually applyvisibility: hidden. Up till now, that middleware used to unintentionally be no-op. (sandbox)Popover.foobarby default. Doing{foo: true, ...userProps}actually results in{foobar: undefined}ifuserProps.foobar = undefined. The fix I used was{foobar: userProps.foobar ?? true}.Testing
Added e2e test and story.
@mayank99 pointed out some issues that we faced with the
hidemodifier in old versions of itwinui-react that used to use popper instead of floating-ui. I made sure that the previous issues no longer happen after this PR or are no longer related to this PR:hidemodifier from popper iTwinUI-react#783: I'm not sure what was the issue here. But iiuc, I don't think it relates to this PR since the trigger of a dropdown menu is usually a button that doesn't getwidth: 0; height: 0;ordisplay: none;.hidemodifier inComboBox'susePopover()call still does not give the problem demoed in the issue.Docs
Added changesets and docs.