Skip to content

Commit

Permalink
refactor(cdk/dialog): expand and clean up API
Browse files Browse the repository at this point in the history
Adjusts the public API of the CDK dialog based on a recent feedback session by:
* Expanding `DialogRef.restoreFocus` to allow CSS selectors and DOM nodes.
* Changing `Dialog.openDialogs`, `DialogRef.componentInstance` and `DialogRef.containerInstance` to be readonly.
* Allowing for numbers to be passed in to `DialogRef.updateSize`.
* Updating the doc string of `DialogRef.updateSize`.
  • Loading branch information
crisbeto committed Apr 28, 2022
1 parent 587c9e3 commit 70825a3
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 26 deletions.
10 changes: 7 additions & 3 deletions src/cdk/dialog/dialog-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,14 @@ export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet =
autoFocus?: AutoFocusTarget | string | boolean = 'first-tabbable';

/**
* Whether the dialog should restore focus to the
* previously-focused element upon closing.
* Whether the dialog should restore focus to the previously-focused element upon closing.
* Has the following behavior based on the type that is passed in:
* - `boolean` - when true, will return focus to the element that was focused before the dialog
* was opened, otherwise won't restore focus at all.
* - `string` - focus will be restored to the first element that matches the CSS selector.
* - `HTMLElement` - focus will be restored to the specific element.
*/
restoreFocus?: boolean = true;
restoreFocus?: boolean | string | HTMLElement = true;

/**
* Scroll strategy to be used for the dialog. This determines how
Expand Down
19 changes: 14 additions & 5 deletions src/cdk/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,22 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>

/** Restores focus to the element that was focused before the dialog opened. */
private _restoreFocus() {
const previousElement = this._elementFocusedBeforeDialogWasOpened;
const focusConfig = this._config.restoreFocus;
let focusTargetElement: HTMLElement | null = null;

if (typeof focusConfig === 'string') {
focusTargetElement = this._document.querySelector(focusConfig);
} else if (typeof focusConfig === 'boolean') {
focusTargetElement = focusConfig ? this._elementFocusedBeforeDialogWasOpened : null;
} else if (focusConfig) {
focusTargetElement = focusConfig;
}

// We need the extra check, because IE can set the `activeElement` to null in some cases.
if (
this._config.restoreFocus &&
previousElement &&
typeof previousElement.focus === 'function'
focusTargetElement &&
typeof focusTargetElement.focus === 'function'
) {
const activeElement = _getFocusedElementPierceShadowDom();
const element = this._elementRef.nativeElement;
Expand All @@ -263,10 +272,10 @@ export class CdkDialogContainer<C extends DialogConfig = DialogConfig>
element.contains(activeElement)
) {
if (this._focusMonitor) {
this._focusMonitor.focusVia(previousElement, this._closeInteractionType);
this._focusMonitor.focusVia(focusTargetElement, this._closeInteractionType);
this._closeInteractionType = null;
} else {
previousElement.focus();
focusTargetElement.focus();
}
}
}
Expand Down
13 changes: 7 additions & 6 deletions src/cdk/dialog/dialog-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ export class DialogRef<R = unknown, C = unknown> {
* Instance of component opened into the dialog. Will be
* null when the dialog is opened using a `TemplateRef`.
*/
componentInstance: C | null;
readonly componentInstance: C | null;

/** Instance of the container that is rendering out the dialog content. */
containerInstance: BasePortalOutlet & {_closeInteractionType?: FocusOrigin};
readonly containerInstance: BasePortalOutlet & {_closeInteractionType?: FocusOrigin};

/** Whether the user is allowed to close the dialog. */
disableClose: boolean | undefined;
Expand Down Expand Up @@ -86,11 +86,13 @@ export class DialogRef<R = unknown, C = unknown> {
this.overlayRef.dispose();
closedSubject.next(result);
closedSubject.complete();
this.componentInstance = this.containerInstance = null!;
(this as {componentInstance: C}).componentInstance = (
this as {containerInstance: BasePortalOutlet}
).containerInstance = null!;
}
}

/** Updates the dialog's position. */
/** Updates the position of the dialog based on the current position strategy. */
updatePosition(): this {
this.overlayRef.updatePosition();
return this;
Expand All @@ -101,9 +103,8 @@ export class DialogRef<R = unknown, C = unknown> {
* @param width New width of the dialog.
* @param height New height of the dialog.
*/
updateSize(width: string = '', height: string = ''): this {
updateSize(width: string | number = '', height: string | number = ''): this {
this.overlayRef.updateSize({width, height});
this.overlayRef.updatePosition();
return this;
}

Expand Down
79 changes: 79 additions & 0 deletions src/cdk/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,85 @@ describe('Dialog', () => {
.withContext('Expected dialog container to be focused.')
.toBe('cdk-dialog-container');
}));

it('should allow for focus restoration to be disabled', fakeAsync(() => {
// Create a element that has focus before the dialog is opened.
const button = document.createElement('button');
button.id = 'dialog-trigger';
document.body.appendChild(button);
button.focus();

const dialogRef = dialog.open(PizzaMsg, {
viewContainerRef: testViewContainerRef,
restoreFocus: false,
});

flushMicrotasks();
viewContainerFixture.detectChanges();
flushMicrotasks();

expect(document.activeElement!.id).not.toBe('dialog-trigger');

dialogRef.close();
flushMicrotasks();
viewContainerFixture.detectChanges();
flush();

expect(document.activeElement!.id).not.toBe('dialog-trigger');
button.remove();
}));

it('should allow for focus to be restored to an element matching a selector', fakeAsync(() => {
// Create a element that has focus before the dialog is opened.
const button = document.createElement('button');
button.id = 'dialog-trigger';
document.body.appendChild(button);

const dialogRef = dialog.open(PizzaMsg, {
viewContainerRef: testViewContainerRef,
restoreFocus: `#${button.id}`,
});

flushMicrotasks();
viewContainerFixture.detectChanges();
flushMicrotasks();

expect(document.activeElement!.id).not.toBe('dialog-trigger');

dialogRef.close();
flushMicrotasks();
viewContainerFixture.detectChanges();
flush();

expect(document.activeElement!.id).toBe('dialog-trigger');
button.remove();
}));

it('should allow for focus to be restored to a specific DOM node', fakeAsync(() => {
// Create a element that has focus before the dialog is opened.
const button = document.createElement('button');
button.id = 'dialog-trigger';
document.body.appendChild(button);

const dialogRef = dialog.open(PizzaMsg, {
viewContainerRef: testViewContainerRef,
restoreFocus: button,
});

flushMicrotasks();
viewContainerFixture.detectChanges();
flushMicrotasks();

expect(document.activeElement!.id).not.toBe('dialog-trigger');

dialogRef.close();
flushMicrotasks();
viewContainerFixture.detectChanges();
flush();

expect(document.activeElement!.id).toBe('dialog-trigger');
button.remove();
}));
});

describe('aria-label', () => {
Expand Down
12 changes: 6 additions & 6 deletions src/cdk/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class Dialog implements OnDestroy {
private _scrollStrategy: () => ScrollStrategy;

/** Keeps track of the currently-open dialogs. */
get openDialogs(): DialogRef<any, any>[] {
get openDialogs(): readonly DialogRef<any, any>[] {
return this._parentDialog ? this._parentDialog.openDialogs : this._openDialogsAtThisLevel;
}

Expand Down Expand Up @@ -129,15 +129,15 @@ export class Dialog implements OnDestroy {
const dialogRef = new DialogRef(overlayRef, config);
const dialogContainer = this._attachContainer(overlayRef, dialogRef, config);

dialogRef.containerInstance = dialogContainer;
(dialogRef as {containerInstance: BasePortalOutlet}).containerInstance = dialogContainer;
this._attachDialogContent(componentOrTemplateRef, dialogRef, dialogContainer, config);

// If this is the first dialog that we're opening, hide all the non-overlay content.
if (!this.openDialogs.length) {
this._hideNonDialogContentFromAssistiveTechnology();
}

this.openDialogs.push(dialogRef);
(this.openDialogs as DialogRef<R, C>[]).push(dialogRef);
dialogRef.closed.subscribe(() => this._removeOpenDialog(dialogRef));
this.afterOpened.next(dialogRef);

Expand Down Expand Up @@ -278,7 +278,7 @@ export class Dialog implements OnDestroy {
config.componentFactoryResolver,
),
);
dialogRef.componentInstance = contentRef.instance;
(dialogRef as {componentInstance: C}).componentInstance = contentRef.instance;
}
}

Expand Down Expand Up @@ -331,7 +331,7 @@ export class Dialog implements OnDestroy {
const index = this.openDialogs.indexOf(dialogRef);

if (index > -1) {
this.openDialogs.splice(index, 1);
(this.openDialogs as DialogRef<R, C>[]).splice(index, 1);

// If all the dialogs were closed, remove/restore the `aria-hidden`
// to a the siblings and emit to the `afterAllClosed` stream.
Expand Down Expand Up @@ -375,7 +375,7 @@ export class Dialog implements OnDestroy {
}

/** Closes all of the dialogs in an array. */
private _closeDialogs(dialogs: DialogRef<unknown>[]) {
private _closeDialogs(dialogs: readonly DialogRef<unknown>[]) {
let i = dialogs.length;

while (i--) {
Expand Down
1 change: 0 additions & 1 deletion src/dev-app/cdk-dialog/dialog-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export class DialogDemo {
maxHeight: defaultDialogConfig.maxHeight,
data: {
message: 'Jazzy jazz jazz',
hmm: false,
},
};
numTemplateOpens = 0;
Expand Down
10 changes: 5 additions & 5 deletions tools/public_api_guard/cdk/dialog.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class Dialog implements OnDestroy {
open<R = unknown, D = unknown, C = unknown>(template: TemplateRef<C>, config?: DialogConfig<D, DialogRef<R, C>>): DialogRef<R, C>;
// (undocumented)
open<R = unknown, D = unknown, C = unknown>(componentOrTemplateRef: ComponentType<C> | TemplateRef<C>, config?: DialogConfig<D, DialogRef<R, C>>): DialogRef<R, C>;
get openDialogs(): DialogRef<any, any>[];
get openDialogs(): readonly DialogRef<any, any>[];
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<Dialog, [null, null, { optional: true; }, { optional: true; skipSelf: true; }, null, null]>;
// (undocumented)
Expand Down Expand Up @@ -145,7 +145,7 @@ export class DialogConfig<D = unknown, R = unknown, C extends BasePortalOutlet =
panelClass?: string | string[];
positionStrategy?: PositionStrategy;
providers?: StaticProvider[] | ((dialogRef: R, config: DialogConfig<D, R, C>, container: C) => StaticProvider[]);
restoreFocus?: boolean;
restoreFocus?: boolean | string | HTMLElement;
role?: DialogRole;
scrollStrategy?: ScrollStrategy;
templateContext?: Record<string, any> | (() => Record<string, any>);
Expand All @@ -170,10 +170,10 @@ export class DialogRef<R = unknown, C = unknown> {
readonly backdropClick: Observable<MouseEvent>;
close(result?: R, options?: DialogCloseOptions): void;
readonly closed: Observable<R | undefined>;
componentInstance: C | null;
readonly componentInstance: C | null;
// (undocumented)
readonly config: DialogConfig<any, DialogRef<R, C>, BasePortalOutlet>;
containerInstance: BasePortalOutlet & {
readonly containerInstance: BasePortalOutlet & {
_closeInteractionType?: FocusOrigin;
};
disableClose: boolean | undefined;
Expand All @@ -184,7 +184,7 @@ export class DialogRef<R = unknown, C = unknown> {
readonly overlayRef: OverlayRef;
removePanelClass(classes: string | string[]): this;
updatePosition(): this;
updateSize(width?: string, height?: string): this;
updateSize(width?: string | number, height?: string | number): this;
}

// @public
Expand Down

0 comments on commit 70825a3

Please sign in to comment.