From e0443dec6810c1656c7a6fd08b7aa183b80bbd20 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 17 Dec 2021 08:19:17 +0100 Subject: [PATCH] refactor(material/menu): use separate signature for deprecated constructor parameters Moves the constructor deprecations out into a separate signature in order to make it easier to remove later on and to make the autocompletion in IDEs more accurate. This is something we've wanted to do for a while, but VIewEngine didn't support multiple constructor signatures. Also this is meant as a test to check if anything comes up during the presubmit. If it passes, we can roll this approach out to all constructors. --- src/material/menu/menu-content.ts | 19 ++++++++++++---- src/material/menu/menu-item.ts | 29 +++++++++++++------------ src/material/menu/menu-trigger.ts | 19 +++++++++++++--- tools/public_api_guard/material/menu.md | 11 +++++----- 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/src/material/menu/menu-content.ts b/src/material/menu/menu-content.ts index 1510ec0e9c55..d3f2f9753002 100644 --- a/src/material/menu/menu-content.ts +++ b/src/material/menu/menu-content.ts @@ -37,6 +37,20 @@ export abstract class _MatMenuContentBase implements OnDestroy { /** Emits when the menu content has been attached. */ readonly _attached = new Subject(); + /** + * @deprecated `changeDetectorRef` is now a required parameter. + * @breaking-change 9.0.0 + */ + constructor( + template: TemplateRef, + componentFactoryResolver: ComponentFactoryResolver, + appRef: ApplicationRef, + injector: Injector, + viewContainerRef: ViewContainerRef, + document: any, + changeDetectorRef?: ChangeDetectorRef, + ); + constructor( private _template: TemplateRef, private _componentFactoryResolver: ComponentFactoryResolver, @@ -80,10 +94,7 @@ export abstract class _MatMenuContentBase implements OnDestroy { // not be updated by Angular. By explicitly marking for check here, we tell Angular that // it needs to check for new menu items and update the `@ContentChild` in `MatMenu`. // @breaking-change 9.0.0 Make change detector ref required - if (this._changeDetectorRef) { - this._changeDetectorRef.markForCheck(); - } - + this._changeDetectorRef?.markForCheck(); this._portal.attach(this._outlet, context); this._attached.next(); } diff --git a/src/material/menu/menu-item.ts b/src/material/menu/menu-item.ts index 63bb1ce25852..1756ffd5e38a 100644 --- a/src/material/menu/menu-item.ts +++ b/src/material/menu/menu-item.ts @@ -75,27 +75,28 @@ export class MatMenuItem /** Whether the menu item acts as a trigger for a sub-menu. */ _triggersSubmenu: boolean = false; + /** + * @deprecated `document` parameter to be removed, `changeDetectorRef` and + * `focusMonitor` to become required. + * @breaking-change 12.0.0 + */ + constructor( + elementRef: ElementRef, + document?: any, + focusMonitor?: FocusMonitor, + parentMenu?: MatMenuPanel, + changeDetectorRef?: ChangeDetectorRef, + ); + constructor( private _elementRef: ElementRef, - /** - * @deprecated `_document` parameter is no longer being used and will be removed. - * @breaking-change 12.0.0 - */ @Inject(DOCUMENT) _document?: any, private _focusMonitor?: FocusMonitor, @Inject(MAT_MENU_PANEL) @Optional() public _parentMenu?: MatMenuPanel, - /** - * @deprecated `_changeDetectorRef` to become a required parameter. - * @breaking-change 14.0.0 - */ private _changeDetectorRef?: ChangeDetectorRef, ) { - // @breaking-change 8.0.0 make `_focusMonitor` and `document` required params. super(); - - if (_parentMenu && _parentMenu.addItem) { - _parentMenu.addItem(this); - } + _parentMenu?.addItem?.(this); } /** Focuses the menu item. */ @@ -171,7 +172,7 @@ export class MatMenuItem // We need to mark this for check for the case where the content is coming from a // `matMenuContent` whose change detection tree is at the declaration position, // not the insertion position. See #23175. - // @breaking-change 14.0.0 Remove null check for `_changeDetectorRef`. + // @breaking-change 12.0.0 Remove null check for `_changeDetectorRef`. this._highlighted = isHighlighted; this._changeDetectorRef?.markForCheck(); } diff --git a/src/material/menu/menu-trigger.ts b/src/material/menu/menu-trigger.ts index 0456a2e79ac5..d51c42715890 100644 --- a/src/material/menu/menu-trigger.ts +++ b/src/material/menu/menu-trigger.ts @@ -185,6 +185,21 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy // tslint:disable-next-line:no-output-on-prefix @Output() readonly onMenuClose: EventEmitter = this.menuClosed; + /** + * @deprecated `focusMonitor` will become a required parameter. + * @breaking-change 8.0.0 + */ + constructor( + overlay: Overlay, + element: ElementRef, + viewContainerRef: ViewContainerRef, + scrollStrategy: any, + parentMenu: MatMenuPanel, + menuItemInstance: MatMenuItem, + dir: Directionality, + focusMonitor?: FocusMonitor | null, + ); + constructor( private _overlay: Overlay, private _element: ElementRef, @@ -195,9 +210,7 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy // tslint:disable-next-line: lightweight-tokens @Optional() @Self() private _menuItemInstance: MatMenuItem, @Optional() private _dir: Directionality, - // TODO(crisbeto): make the _focusMonitor required when doing breaking changes. - // @breaking-change 8.0.0 - private _focusMonitor?: FocusMonitor, + private _focusMonitor: FocusMonitor | null, ) { this._scrollStrategy = scrollStrategy; this._parentMaterialMenu = parentMenu instanceof _MatMenuBase ? parentMenu : undefined; diff --git a/tools/public_api_guard/material/menu.md b/tools/public_api_guard/material/menu.md index 1eccbea708cc..da6acf65fbff 100644 --- a/tools/public_api_guard/material/menu.md +++ b/tools/public_api_guard/material/menu.md @@ -167,7 +167,8 @@ export class MatMenuContent extends _MatMenuContentBase { // @public (undocumented) export abstract class _MatMenuContentBase implements OnDestroy { - constructor(_template: TemplateRef, _componentFactoryResolver: ComponentFactoryResolver, _appRef: ApplicationRef, _injector: Injector, _viewContainerRef: ViewContainerRef, _document: any, _changeDetectorRef?: ChangeDetectorRef | undefined); + // @deprecated + constructor(template: TemplateRef, componentFactoryResolver: ComponentFactoryResolver, appRef: ApplicationRef, injector: Injector, viewContainerRef: ViewContainerRef, document: any, changeDetectorRef?: ChangeDetectorRef); attach(context?: any): void; readonly _attached: Subject; detach(): void; @@ -191,9 +192,8 @@ export interface MatMenuDefaultOptions { // @public export class MatMenuItem extends _MatMenuItemBase implements FocusableOption, CanDisable, CanDisableRipple, AfterViewInit, OnDestroy { - constructor(_elementRef: ElementRef, - _document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel | undefined, - _changeDetectorRef?: ChangeDetectorRef | undefined); + // @deprecated + constructor(elementRef: ElementRef, document?: any, focusMonitor?: FocusMonitor, parentMenu?: MatMenuPanel, changeDetectorRef?: ChangeDetectorRef); _checkDisabled(event: Event): void; focus(origin?: FocusOrigin, options?: FocusOptions): void; readonly _focused: Subject; @@ -279,7 +279,8 @@ export class MatMenuTrigger extends _MatMenuTriggerBase { // @public (undocumented) export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy { - constructor(_overlay: Overlay, _element: ElementRef, _viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, _menuItemInstance: MatMenuItem, _dir: Directionality, _focusMonitor?: FocusMonitor | undefined); + // @deprecated + constructor(overlay: Overlay, element: ElementRef, viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, menuItemInstance: MatMenuItem, dir: Directionality, focusMonitor?: FocusMonitor | null); closeMenu(): void; // @deprecated (undocumented) get _deprecatedMatMenuTriggerFor(): MatMenuPanel;