fix: contextual menu container overflow

[petermakowski]

Done

This pull request fixes the menu position when scrolling in overflowing containers.

• fix contextual menu dropdown not being scrolled correctly in overflowing containers
• replace deprecated ClientRect with DOMRect
• fix dropdown props being passed incorrectly to the wrapper
• add options to useOnEscapePressed and useClickOutside
• rename useClickOutside to useOnClickOutside for consistency
• remove the portal ref hack, update react-useportal to 1.0.19 Support opening the portal programmatically. alex-cory/react-useportal#83 (comment)
• add options to useListener hook
• rename wrapperClassName to contextualMenuClassName for clarity

Screenshots

Before

After

QA

QA steps

• Go to the overflow example

• Ensure that once you open the dropdown and scroll, menu position is in sync with the toggle

• In the props control, set autoAdjust to false and position to right

• Open the dropdown menu

• Menu position should reflect the value of the prop (right)

• Close the dropdown

• Set autoAdjust to true

• Open the dropdown

• Menu position should be adjusted on open (and set to the left)

• Link react-components locally with your project and ensure that contextual menus work as expected

Fixes

Fixes: https://warthogs.atlassian.net/browse/MAASENG-2407
#687
#769

[ndv99]

This looks good to me 👍

[edlerd]

Looks good functionality wise. Two nitpicks about code style / simplifications. Feel free to address or keep as is.

Edit: I don't have a way to test this in other projects without a full release. But I won't expect this to break anything.

[edlerd]

I think there is no need to cast here.

Suggested change

	...(positionStyle as React.CSSProperties),
	...positionStyle,

[huwshimi]

Hi @petermakowski, this was using a portal so that it could escape its container to prevent issues such as overflow: hidden on a parent (same as we do for tooltips, portals etc.) With this change you can see that the menus get cut off in the storybook examples:

Any ideas for mitigating that issue?

[petermakowski]

Hi @petermakowski, this was using a portal so that it could escape its container to prevent issues such as overflow: hidden on a parent (same as we do for tooltips, portals etc.)
Any ideas for mitigating that issue?

@huwshimi Thanks for chipping in. Very good point, clearly I was missing some context here. I intended for this to be fixed by removing overflow: hidden from a container, but this might not work in all cases.

In the examples that you pasted specifically this can be worked around by using automatic positioning and displaying the menu at the top of a trigger when not enough space is available at the bottom (the approach that we're exploring in MAAS currently). Having said that, it's a big change in approach and something that not all teams may want to adopt. I'll give this some more thought. Marking this as do not merge for now.

[huwshimi]

Hi @petermakowski, this was using a portal so that it could escape its container to prevent issues such as overflow: hidden on a parent (same as we do for tooltips, portals etc.)
Any ideas for mitigating that issue?

@huwshimi Thanks for chipping in. Very good point, clearly I was missing some context here. I intended for this to be fixed by removing overflow: hidden from a container, but this might not work in all cases.

In the examples that you pasted specifically this can be worked around by using automatic positioning and displaying the menu at the top of a trigger when not enough space is available at the bottom (the approach that we're exploring in MAAS currently). Having said that, it's a big change in approach and something that not all teams may want to adopt. I'll give this some more thought. Marking this as do not merge for now.

Thanks Peter, we've definitely had cases in the past where we couldn't control the overflow which is why we ended up using portals, but maybe there's another approach we haven't considered.

[huwshimi]

Just leaving a "Request changes" here so that we remember this needs to address the overflow: hidden hidden issue as discussed.

[petermakowski]

@huwshimi This is ready for another review. I reverted changes related to relative positioning, keeping node position in sync with the container is now handled similarly to window resize.

[huwshimi]

Works great now, thanks for the improvement @petermakowski!