Skip to content

feat(material/menu): allow updating menu position a la autocomplete#22046

Merged
andrewseguin merged 1 commit intoangular:masterfrom
terencehonles:allow-updating-menu-position
Mar 23, 2021
Merged

feat(material/menu): allow updating menu position a la autocomplete#22046
andrewseguin merged 1 commit intoangular:masterfrom
terencehonles:allow-updating-menu-position

Conversation

@terencehonles
Copy link
Copy Markdown
Contributor

On document changes the menu may not be located in the correct location anymore. This exposes the OverlayRef's updatePosition on the trigger so a user can call it without relying on the private variable _overlayRef.

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 26, 2021
Copy link
Copy Markdown
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good, but the CI is failing because:

  1. The API golden needs to be updated by running yarn approve-api menu.
  2. The MDC test lint check is failing, because you have to add the same test to src/material-experimental/menu/menu.spec.ts.

*/
updatePosition(): void {
if (this._overlayRef) {
this._overlayRef.updatePosition();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can be reduce to this._overlayRef?.updatePosition().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 wasn't sure which was preferred here.

@terencehonles terencehonles force-pushed the allow-updating-menu-position branch from 336e038 to a03b5c5 Compare March 15, 2021 18:13
@terencehonles
Copy link
Copy Markdown
Contributor Author

Thanks, I was expecting I'd have time to update the PR earlier, but I'll probably tied up for a bit if that doesn't handle everything.

@terencehonles terencehonles force-pushed the allow-updating-menu-position branch from a03b5c5 to 4dff505 Compare March 22, 2021 16:45
@terencehonles terencehonles force-pushed the allow-updating-menu-position branch from 4dff505 to e95cac6 Compare March 22, 2021 17:39
Copy link
Copy Markdown
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: minor This PR is targeted for the next minor release labels Mar 22, 2021
@andrewseguin andrewseguin merged commit ad24865 into angular:master Mar 23, 2021
@terencehonles terencehonles deleted the allow-updating-menu-position branch March 23, 2021 23:02
@angular-automatic-lock-bot
Copy link
Copy Markdown

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants