8000 feat(material/menu): allow updating menu position a la autocomplete by terencehonles · Pull Request #22046 · angular/components · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

terencehonles
Copy link
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
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
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
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
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
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

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
0