Skip to content

Commit

Permalink
refactor(material/menu): use separate signature for deprecated constr…
Browse files Browse the repository at this point in the history
…uctor parameters (#24116)

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.

(cherry picked from commit b81259e)
  • Loading branch information
crisbeto authored and wagnermaciel committed Jan 10, 2022
1 parent 69753d7 commit 84e6b02
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 26 deletions.
19 changes: 15 additions & 4 deletions src/material/menu/menu-content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ export abstract class _MatMenuContentBase implements OnDestroy {
/** Emits when the menu content has been attached. */
readonly _attached = new Subject<void>();

/**
* @deprecated `changeDetectorRef` is now a required parameter.
* @breaking-change 9.0.0
*/
constructor(
template: TemplateRef<any>,
componentFactoryResolver: ComponentFactoryResolver,
appRef: ApplicationRef,
injector: Injector,
viewContainerRef: ViewContainerRef,
document: any,
changeDetectorRef?: ChangeDetectorRef,
);

constructor(
private _template: TemplateRef<any>,
private _componentFactoryResolver: ComponentFactoryResolver,
Expand Down Expand Up @@ -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();
}
Expand Down
29 changes: 15 additions & 14 deletions src/material/menu/menu-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLElement>,
document?: any,
focusMonitor?: FocusMonitor,
parentMenu?: MatMenuPanel<MatMenuItem>,
changeDetectorRef?: ChangeDetectorRef,
);

constructor(
private _elementRef: ElementRef<HTMLElement>,
/**
* @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<MatMenuItem>,
/**
* @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. */
Expand Down Expand Up @@ -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();
}
Expand Down
19 changes: 16 additions & 3 deletions src/material/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,21 @@ export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy
// tslint:disable-next-line:no-output-on-prefix
@Output() readonly onMenuClose: EventEmitter<void> = this.menuClosed;

/**
* @deprecated `focusMonitor` will become a required parameter.
* @breaking-change 8.0.0
*/
constructor(
overlay: Overlay,
element: ElementRef<HTMLElement>,
viewContainerRef: ViewContainerRef,
scrollStrategy: any,
parentMenu: MatMenuPanel,
menuItemInstance: MatMenuItem,
dir: Directionality,
focusMonitor?: FocusMonitor | null,
);

constructor(
private _overlay: Overlay,
private _element: ElementRef<HTMLElement>,
Expand All @@ -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;
Expand Down
11 changes: 6 additions & 5 deletions tools/public_api_guard/material/menu.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ export class MatMenuContent extends _MatMenuContentBase {

// @public (undocumented)
export abstract class _MatMenuContentBase implements OnDestroy {
constructor(_template: TemplateRef<any>, _componentFactoryResolver: ComponentFactoryResolver, _appRef: ApplicationRef, _injector: Injector, _viewContainerRef: ViewContainerRef, _document: any, _changeDetectorRef?: ChangeDetectorRef | undefined);
// @deprecated
constructor(template: TemplateRef<any>, componentFactoryResolver: ComponentFactoryResolver, appRef: ApplicationRef, injector: Injector, viewContainerRef: ViewContainerRef, document: any, changeDetectorRef?: ChangeDetectorRef);
attach(context?: any): void;
readonly _attached: Subject<void>;
detach(): void;
Expand All @@ -191,9 +192,8 @@ export interface MatMenuDefaultOptions {

// @public
export class MatMenuItem extends _MatMenuItemBase implements FocusableOption, CanDisable, CanDisableRipple, AfterViewInit, OnDestroy {
constructor(_elementRef: ElementRef<HTMLElement>,
_document?: any, _focusMonitor?: FocusMonitor | undefined, _parentMenu?: MatMenuPanel<MatMenuItem> | undefined,
_changeDetectorRef?: ChangeDetectorRef | undefined);
// @deprecated
constructor(elementRef: ElementRef<HTMLElement>, document?: any, focusMonitor?: FocusMonitor, parentMenu?: MatMenuPanel<MatMenuItem>, changeDetectorRef?: ChangeDetectorRef);
_checkDisabled(event: Event): void;
focus(origin?: FocusOrigin, options?: FocusOptions): void;
readonly _focused: Subject<MatMenuItem>;
Expand Down Expand Up @@ -279,7 +279,8 @@ export class MatMenuTrigger extends _MatMenuTriggerBase {

// @public (undocumented)
export abstract class _MatMenuTriggerBase implements AfterContentInit, OnDestroy {
constructor(_overlay: Overlay, _element: ElementRef<HTMLElement>, _viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, _menuItemInstance: MatMenuItem, _dir: Directionality, _focusMonitor?: FocusMonitor | undefined);
// @deprecated
constructor(overlay: Overlay, element: ElementRef<HTMLElement>, viewContainerRef: ViewContainerRef, scrollStrategy: any, parentMenu: MatMenuPanel, menuItemInstance: MatMenuItem, dir: Directionality, focusMonitor?: FocusMonitor | null);
closeMenu(): void;
// @deprecated (undocumented)
get _deprecatedMatMenuTriggerFor(): MatMenuPanel;
Expand Down

0 comments on commit 84e6b02

Please sign in to comment.