fix(material/menu): adjust overlay size when amount of items changes

[crisbeto]

Currently we lock the menu into a position after it is opened so that it doesn't jump around when the user scrolls, but this means that if the amount of items changes, it might not be the optimal position anymore.

These changes add some code to re-calculate the position if the amount of items changes.

Fixes #21456.

[DaxChen]

Hey @crisbeto thanks for the quick PR!

This fix detects the number of child in menu, but what if the menu heigh change is due to other things? For example my use case has a checkbox tree inside mat-menu, and the menu height grows when some node in checkbox tree is expanded.

I noticed that .cdk-overlay-connected-position-bounding-box has the proper top/right/bottom/left set if the menu trigger is not inside a scrollable area, but will be set to top: 0; left: 0; width: 100%; height: 100% (here) when the menu trigger is in a scrollable area, causing the menu to grow until .mat-menu-panel hits max-height: calc(100vh - 48px) (defined here).

[crisbeto]

Unfortunately there's no efficient way for us to detect whether the menu height has changed due to something else. ResizeObserver isn't available everywhere yet and the alternative is to poll on every change detection which is slow.

[DaxChen]

Yeah I totally agree with that. In that case...

1. Is it possible to do feature detection and add ResizeObserver so supported browser can handle it correctly? (might be controversial to have a framework behave differently on different browsers tho?)
2. Is it possible to expose an API so we can manually call for example matMenu.reposition() when we know that height has changed, or we have hooked up ResizeObserver and only care about supported browsers? Exposing a new API might be harder and takes more process...
3. Therefore if none of those are possible for now, can we at least have some way to workaround this? I'm thinking with this PR, I can add/remove an invisible element anytime I know the hight changes to trigger the resizing. Will that work?

[crisbeto]

1. We've been avoiding adding ResizeObserver only when supported, because it'll cause things to break only on some browsers and we generally want all browsers to behave the same.
2. Exposing an API won't be a problem, although as you mentioned, it'll have to wait for a minor release. It's also worth noting that it would have to be on the menu trigger, because the menu itself doesn't know about the positioning.
3. As for a workaround, it's a little hacky but you can get a hold of the menu's OverlayRef like (menuTrigger as any)._overlayRef and add the same reposition function call as the one I added above.

[DaxChen]

Wow, thank you so much @crisbeto ! Getting the OverlayRef from menuTrigger is super helpful!

[jelbourn]

LGTM

[andrewseguin]

This seems to break tests for cases where the lastPosition is not yet set, perhaps where the menu content is lazy-loaded? If the logic checks first whether the position strategy already has an optimal position, all tests pass

Here's the proposed change to this PR (hopefully it doesn't negate what this is fixing):

Add this method in /src/cdk/overlay/position/flexible-connected-position-strategy.ts:

/**
 * Whether the position strategy has already determined an optimal position.
 */
hasLastPosition(): boolean {
  return !!this._lastPosition;
}

Then wrap your logic with the check in menu-trigger.ts:

if (positionStrategy.hasLastPosition()) {
  positionStrategy.withLockedPosition(false).reapplyLastPosition();
  positionStrategy.withLockedPosition(true);
}

Reference:

• Tests: test/OCL:423652344:BASE:423641362:1643050157883:14cca6e2
• Internal change: cl/423652344

[crisbeto]

Updated to null check the last position.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}