From c101f7ff75acdd624e1e599f1f33a67252e36558 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Green Date: Fri, 25 Dec 2020 16:15:01 +0100 Subject: [PATCH 1/7] fix(material/dialog): make align attribute into an input of dialog actions directive instead of css Fixes multiple issues, such as self-documentation of the align attribute, type checking, better bindings, and IDE support. Because align is not standard for HTMLDivElement, it doesn't make much sense to assume end users to know they can use the align attribute. Fixes #18479 --- src/dev-app/dialog/dialog-demo.ts | 18 +++++++-------- .../dialog/dialog-content-directives.ts | 22 ++++++++++++------- src/material/dialog/dialog.scss | 11 +++++----- src/material/dialog/dialog.spec.ts | 12 ++++++++-- 4 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/dev-app/dialog/dialog-demo.ts b/src/dev-app/dialog/dialog-demo.ts index 5223f841c918..7a28213edfad 100644 --- a/src/dev-app/dialog/dialog-demo.ts +++ b/src/dev-app/dialog/dialog-demo.ts @@ -22,7 +22,7 @@ export class DialogDemo { dialogRef: MatDialogRef | null; lastAfterClosedResult: string; lastBeforeCloseResult: string; - actionsAlignment: string; + actionsAlignment: 'end' | 'center' | undefined; config = { disableClose: false, panelClass: 'custom-overlay-pane-class', @@ -110,20 +110,20 @@ export class JazzDialog { private _dimesionToggle = false; constructor( - public dialogRef: MatDialogRef, - @Inject(MAT_DIALOG_DATA) public data: any) { } + public dialogRef: MatDialogRef, + @Inject(MAT_DIALOG_DATA) public data: any) { } togglePosition(): void { this._dimesionToggle = !this._dimesionToggle; if (this._dimesionToggle) { this.dialogRef - .updateSize('500px', '500px') - .updatePosition({ top: '25px', left: '25px' }); + .updateSize('500px', '500px') + .updatePosition({ top: '25px', left: '25px' }); } else { this.dialogRef - .updateSize() - .updatePosition(); + .updateSize() + .updatePosition(); } } @@ -160,7 +160,7 @@ export class JazzDialog {

- + '}) +@Component({template : '

Pizza

'}) class PizzaMsg { - constructor( - public dialogRef: MatDialogRef, public dialogInjector: Injector, - public directionality: Directionality) {} + constructor(public dialogRef: MatDialogRef, public dialogInjector: Injector, + public directionality: Directionality) {} } @Component({ - template: ` + template : `

This is the title

Lorem ipsum dolor sit amet. - + '}) +@Component({template : '

Pizza

'}) class PizzaMsg { - constructor(public dialogRef: MatDialogRef, - public dialogInjector: Injector, + constructor(public dialogRef: MatDialogRef, public dialogInjector: Injector, public directionality: Directionality) {} } @Component({ - template: ` + template : `

This is the title

Lorem ipsum dolor sit amet. @@ -1896,10 +1799,11 @@ class PizzaMsg { ` }) -class ContentElementDialog {} +class ContentElementDialog { +} @Component({ - template: ` + template : `

This is the title

Lorem ipsum dolor sit amet. @@ -1920,22 +1824,20 @@ class ComponentWithContentElementTemplateRef { @ViewChild(TemplateRef) templateRef: TemplateRef; } -@Component({ - template: '', - providers: [MatDialog] -}) +@Component({template : '', providers : [ MatDialog ]}) class ComponentThatProvidesMatDialog { constructor(public dialog: MatDialog) {} } /** Simple component for testing ComponentPortal. */ -@Component({template: ''}) +@Component({template : ''}) class DialogWithInjectedData { - constructor(@Inject(MAT_DIALOG_DATA) public data: any) { } + constructor(@Inject(MAT_DIALOG_DATA) public data: any) {} } -@Component({template: '

Pasta

'}) -class DialogWithoutFocusableElements {} +@Component({template : '

Pasta

'}) +class DialogWithoutFocusableElements { +} // Create a real (non-test) NgModule as a workaround for // https://github.com/angular/angular/issues/10760 @@ -1952,10 +1854,10 @@ const TEST_DIRECTIVES = [ ]; @NgModule({ - imports: [MatDialogModule, NoopAnimationsModule], - exports: TEST_DIRECTIVES, - declarations: TEST_DIRECTIVES, - entryComponents: [ + imports : [ MatDialogModule, NoopAnimationsModule ], + exports : TEST_DIRECTIVES, + declarations : TEST_DIRECTIVES, + entryComponents : [ ComponentWithChildViewContainer, ComponentWithTemplateRef, PizzaMsg, @@ -1964,4 +1866,5 @@ const TEST_DIRECTIVES = [ DialogWithoutFocusableElements, ], }) -class DialogTestModule { } +class DialogTestModule { +} diff --git a/tools/public_api_guard/material/dialog.d.ts b/tools/public_api_guard/material/dialog.d.ts index 52dd272a5bfe..6f952a3de2d0 100644 --- a/tools/public_api_guard/material/dialog.d.ts +++ b/tools/public_api_guard/material/dialog.d.ts @@ -74,7 +74,8 @@ export declare class MatDialog extends _MatDialogBase { } export declare class MatDialogActions { - static ɵdir: i0.ɵɵDirectiveDefWithMeta; + align?: 'center' | 'end'; + static ɵdir: i0.ɵɵDirectiveDefWithMeta; static ɵfac: i0.ɵɵFactoryDef; } From 8d367c1ef4b10ae865d5e08b101189781eed5263 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Date: Sun, 9 Jan 2022 23:05:54 +0100 Subject: [PATCH 3/7] Update comments in dialog.scss for more clarity --- src/material-experimental/mdc-dialog/dialog.scss | 3 ++- src/material/dialog/dialog.scss | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/material-experimental/mdc-dialog/dialog.scss b/src/material-experimental/mdc-dialog/dialog.scss index ba994299dfd2..0eec7117019e 100644 --- a/src/material-experimental/mdc-dialog/dialog.scss +++ b/src/material-experimental/mdc-dialog/dialog.scss @@ -29,7 +29,8 @@ $mat-dialog-button-horizontal-margin: 8px !default; // aligns actions at the end of the container. justify-content: start; - // .align-center and .align-end are set by directive input "align" + // .mat-mdc-dialog-actions-align-{center|end} are set by directive input "align" + // [align='center'] and [align='right'] are kept for backwards compability &.mat-mdc-dialog-actions-align-center, &[align='center'] { justify-content: center; } diff --git a/src/material/dialog/dialog.scss b/src/material/dialog/dialog.scss index 4c8beafb8547..6ba8ad382925 100644 --- a/src/material/dialog/dialog.scss +++ b/src/material/dialog/dialog.scss @@ -56,7 +56,7 @@ $button-margin: 8px !default; // Pull the actions down to avoid their padding stacking with the dialog's padding. margin-bottom: -$padding; - // .align-center and .align-end are set by directive input "align" + // .mat-dialog-actions-align-{center|end} are set by directive input "align" // [align='center'] and [align='right'] are kept for backwards compability &.mat-dialog-actions-align-center, &[align='center'] { justify-content: center; From d6deb456240553b299aa6e30b496274ccf1b8e8d Mon Sep 17 00:00:00 2001 From: Jean-Philippe Date: Sun, 9 Jan 2022 23:30:24 +0100 Subject: [PATCH 4/7] Use 'start' as default instead of undefined for align attribute in dialog actions --- .../mdc-dialog/dialog-content-directives.ts | 2 +- src/material/dialog/dialog-content-directives.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/material-experimental/mdc-dialog/dialog-content-directives.ts b/src/material-experimental/mdc-dialog/dialog-content-directives.ts index 6cd76b9e283c..38de7efaf900 100644 --- a/src/material-experimental/mdc-dialog/dialog-content-directives.ts +++ b/src/material-experimental/mdc-dialog/dialog-content-directives.ts @@ -148,7 +148,7 @@ export class MatDialogContent {} }, }) export class MatDialogActions { - @Input() align?: 'center' | 'end' = undefined; + @Input() align?: 'start' | 'center' | 'end' = 'start'; } /** diff --git a/src/material/dialog/dialog-content-directives.ts b/src/material/dialog/dialog-content-directives.ts index 29f5689d363c..0751984eb13d 100644 --- a/src/material/dialog/dialog-content-directives.ts +++ b/src/material/dialog/dialog-content-directives.ts @@ -152,7 +152,7 @@ export class MatDialogContent {} }, }) export class MatDialogActions { - @Input() align?: 'center' | 'end' = undefined; + @Input() align?: 'start' | 'center' | 'end' = 'start'; } /** From c87c4aae513d840f6d621ac172915cc25ff5885b Mon Sep 17 00:00:00 2001 From: Jean-Philippe Green Date: Mon, 10 Jan 2022 00:21:57 +0100 Subject: [PATCH 5/7] Revert weird changes to dialog.spec.ts --- .../mdc-dialog/dialog.spec.ts | 3 +- src/material/dialog/dialog.spec.ts | 46 ++++++------------- 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/src/material-experimental/mdc-dialog/dialog.spec.ts b/src/material-experimental/mdc-dialog/dialog.spec.ts index 0fa2c751b381..93b0df93f524 100644 --- a/src/material-experimental/mdc-dialog/dialog.spec.ts +++ b/src/material-experimental/mdc-dialog/dialog.spec.ts @@ -39,7 +39,6 @@ import {By} from '@angular/platform-browser'; import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations'; import {numbers} from '@material/dialog'; import {Subject} from 'rxjs'; - import { MatDialog, MatDialogModule, @@ -1672,7 +1671,7 @@ describe('MDC-based MatDialog', () => { .toBe(title.id); })); - it('should add align-* class according to given [align] input in [mat-dialog-actions]', () => { + it('should add mat-mdc-dialog-actions-align-* class according to given [align] input in [mat-dialog-actions]', () => { let actions = overlayContainerElement.querySelector('mat-dialog-actions')!; expect(actions) diff --git a/src/material/dialog/dialog.spec.ts b/src/material/dialog/dialog.spec.ts index ae89099400c7..176271f210bf 100644 --- a/src/material/dialog/dialog.spec.ts +++ b/src/material/dialog/dialog.spec.ts @@ -1,21 +1,16 @@ import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y'; -import {Directionality} from '@angular/cdk/bidi'; -import {A, ESCAPE} from '@angular/cdk/keycodes'; -import {Overlay, OverlayContainer, ScrollStrategy} from '@angular/cdk/overlay'; -import {ScrollDispatcher} from '@angular/cdk/scrolling'; import { - createKeyboardEvent, - dispatchEvent, - dispatchKeyboardEvent, - dispatchMouseEvent, - patchElementFocus, -} from '@angular/cdk/testing/private'; -import {Location} from '@angular/common'; -import {SpyLocation} from '@angular/common/testing'; + ComponentFixture, + fakeAsync, + flushMicrotasks, + inject, + TestBed, + tick, + flush, +} from '@angular/core/testing'; import { ChangeDetectionStrategy, Component, - ComponentFactoryResolver, Directive, Inject, Injector, @@ -26,15 +21,6 @@ import { NgZone, ViewEncapsulation, } from '@angular/core'; -import { - ComponentFixture, - fakeAsync, - flush, - flushMicrotasks, - inject, - TestBed, - tick, -} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations'; import {Location} from '@angular/common'; @@ -54,13 +40,12 @@ import { } from '../../cdk/testing/private'; import { MAT_DIALOG_DATA, - MAT_DIALOG_DEFAULT_OPTIONS, MatDialog, MatDialogModule, MatDialogRef, + MAT_DIALOG_DEFAULT_OPTIONS, MatDialogState, } from './index'; - import {Subject} from 'rxjs'; describe('MatDialog', () => { @@ -1396,8 +1381,7 @@ describe('MatDialog', () => { button.focus(); // Patch the element focus after the initial and real focus, because otherwise the - // `activeElement` won't be set, and the dialog won't be able to restore focus to an - // element. + // `activeElement` won't be set, and the dialog won't be able to restore focus to an element. patchElementFocus(button); dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef}); @@ -1430,8 +1414,7 @@ describe('MatDialog', () => { button.focus(); // Patch the element focus after the initial and real focus, because otherwise the - // `activeElement` won't be set, and the dialog won't be able to restore focus to an - // element. + // `activeElement` won't be set, and the dialog won't be able to restore focus to an element. patchElementFocus(button); dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef}); @@ -1640,10 +1623,9 @@ describe('MatDialog', () => { 'Expected the focus to change when dialog was opened.', ); + // Start the closing sequence and move focus out of dialog. dialogRef.close(); - flushMicrotasks(); - viewContainerFixture.detectChanges(); - tick(500); + otherButton.focus(); expect(document.activeElement!.id) .withContext('Expected focus to be on the alternate button.') @@ -1752,7 +1734,7 @@ describe('MatDialog', () => { .toBe(title.id); })); - it('should add align-* class according to given [align] input in [mat-dialog-actions]', () => { + it('should add mat-dialog-actions-align-* class according to given [align] input in [mat-dialog-actions]', () => { let actions = overlayContainerElement.querySelector('mat-dialog-actions')!; expect(actions) From 77fca709f57d6d9396981bef4589a932cb1cae72 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Green Date: Mon, 10 Jan 2022 01:20:43 +0100 Subject: [PATCH 6/7] Change actionsAlignment type in dialog demo --- src/dev-app/dialog/dialog-demo.ts | 4 ++-- src/dev-app/mdc-dialog/mdc-dialog-demo.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dev-app/dialog/dialog-demo.ts b/src/dev-app/dialog/dialog-demo.ts index 3f1075f2eab0..cc2ce89bcabd 100644 --- a/src/dev-app/dialog/dialog-demo.ts +++ b/src/dev-app/dialog/dialog-demo.ts @@ -21,7 +21,7 @@ export class DialogDemo { dialogRef: MatDialogRef | null; lastAfterClosedResult: string; lastBeforeCloseResult: string; - actionsAlignment: 'end' | 'center' | undefined; + actionsAlignment: 'start' | 'center' | 'end'; config = { disableClose: false, panelClass: 'custom-overlay-pane-class', @@ -177,7 +177,7 @@ export class JazzDialog { `, }) export class ContentElementDialog { - actionsAlignment: 'end' | 'center' | undefined; + actionsAlignment: 'start' | 'center' | 'end'; constructor(public dialog: MatDialog) {} diff --git a/src/dev-app/mdc-dialog/mdc-dialog-demo.ts b/src/dev-app/mdc-dialog/mdc-dialog-demo.ts index f294b14f4293..10e61c2e8ffd 100644 --- a/src/dev-app/mdc-dialog/mdc-dialog-demo.ts +++ b/src/dev-app/mdc-dialog/mdc-dialog-demo.ts @@ -29,7 +29,7 @@ export class DialogDemo { dialogRef: MatDialogRef | null; lastAfterClosedResult: string; lastBeforeCloseResult: string; - actionsAlignment: string; + actionsAlignment: 'start' | 'center' | 'end'; config = { disableClose: false, panelClass: 'custom-overlay-pane-class', @@ -194,7 +194,7 @@ export class JazzDialog { `, }) export class ContentElementDialog { - actionsAlignment: string; + actionsAlignment: 'start' | 'center' | 'end'; constructor(public dialog: MatDialog) {} From 815cf8f26b2e615e37d00dee0b3471973119a42c Mon Sep 17 00:00:00 2001 From: Jean-Philippe Green Date: Mon, 10 Jan 2022 01:23:12 +0100 Subject: [PATCH 7/7] Use directive binding in mdc-dialog demo for align --- src/dev-app/mdc-dialog/mdc-dialog-demo.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dev-app/mdc-dialog/mdc-dialog-demo.ts b/src/dev-app/mdc-dialog/mdc-dialog-demo.ts index 10e61c2e8ffd..92e750ea9738 100644 --- a/src/dev-app/mdc-dialog/mdc-dialog-demo.ts +++ b/src/dev-app/mdc-dialog/mdc-dialog-demo.ts @@ -173,7 +173,7 @@ export class JazzDialog {

- +