From 33cb7418dddee6eb9ce5280806ebb2b79c7192ea Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 5 Dec 2016 23:05:37 +0100 Subject: [PATCH 1/9] feat(dialog): add dialog content elements Adds the following dialog-specific directives: * `md-dialog-close` - Closes the current dialog. * `md-dialog-title` - Title of a dialog. * `md-dialog-content` - Scrollable content for a dialog. * `md-dialog-actions` - Container for the bottom buttons in a dialog. Fixes #1624. Fixes #2042. --- src/demo-app/demo-app-module.ts | 4 +- src/demo-app/dialog/dialog-demo.html | 3 +- src/demo-app/dialog/dialog-demo.ts | 47 ++++++++++++++++- src/lib/dialog/README.md | 29 ++++++++--- src/lib/dialog/dialog-container.scss | 24 --------- src/lib/dialog/dialog-container.ts | 2 +- src/lib/dialog/dialog-directives.ts | 49 +++++++++++++++++ src/lib/dialog/dialog.scss | 52 +++++++++++++++++++ src/lib/dialog/dialog.spec.ts | 78 ++++++++++++++++++++++++++-- src/lib/dialog/dialog.ts | 52 ++++++------------- src/lib/dialog/index.ts | 50 ++++++++++++++++++ 11 files changed, 316 insertions(+), 74 deletions(-) delete mode 100644 src/lib/dialog/dialog-container.scss create mode 100644 src/lib/dialog/dialog-directives.ts create mode 100644 src/lib/dialog/dialog.scss diff --git a/src/demo-app/demo-app-module.ts b/src/demo-app/demo-app-module.ts index 0a85b5da9475..e9d9cde47819 100644 --- a/src/demo-app/demo-app-module.ts +++ b/src/demo-app/demo-app-module.ts @@ -7,7 +7,7 @@ import {RouterModule} from '@angular/router'; import {MaterialModule} from '@angular/material'; import {DEMO_APP_ROUTES} from './demo-app/routes'; import {ProgressBarDemo} from './progress-bar/progress-bar-demo'; -import {JazzDialog, DialogDemo} from './dialog/dialog-demo'; +import {JazzDialog, ContentElementDialog, DialogDemo} from './dialog/dialog-demo'; import {RippleDemo} from './ripple/ripple-demo'; import {IconDemo} from './icon/icon-demo'; import {GesturesDemo} from './gestures/gestures-demo'; @@ -65,6 +65,7 @@ import {InputContainerDemo} from './input/input-container-demo'; InputDemo, InputContainerDemo, JazzDialog, + ContentElementDialog, ListDemo, LiveAnnouncerDemo, MdCheckboxDemoNestedChecklist, @@ -96,6 +97,7 @@ import {InputContainerDemo} from './input/input-container-demo'; entryComponents: [ DemoApp, JazzDialog, + ContentElementDialog, RotiniPanel, ScienceJoke, SpagettiPanel, diff --git a/src/demo-app/dialog/dialog-demo.html b/src/demo-app/dialog/dialog-demo.html index fe71fed7f98f..491e8031393f 100644 --- a/src/demo-app/dialog/dialog-demo.html +++ b/src/demo-app/dialog/dialog-demo.html @@ -1,6 +1,7 @@

Dialog demo

- + + diff --git a/src/demo-app/dialog/dialog-demo.ts b/src/demo-app/dialog/dialog-demo.ts index c2ffc0dad2ee..7323d8f67173 100644 --- a/src/demo-app/dialog/dialog-demo.ts +++ b/src/demo-app/dialog/dialog-demo.ts @@ -24,7 +24,7 @@ export class DialogDemo { constructor(public dialog: MdDialog) { } - open() { + openJazz() { this.dialogRef = this.dialog.open(JazzDialog, this.config); this.dialogRef.afterClosed().subscribe(result => { @@ -32,6 +32,10 @@ export class DialogDemo { this.dialogRef = null; }); } + + openContentElement() { + this.dialog.open(ContentElementDialog, this.config); + } } @@ -48,3 +52,44 @@ export class JazzDialog { constructor(public dialogRef: MdDialogRef) { } } + + +@Component({ + selector: 'demo-content-element-dialog', + styles: [ + `img { + max-width: 100%; + }` + ], + template: ` +

Neptune

+ + + + +

+ Neptune is the eighth and farthest known planet from the Sun in the Solar System. In the + Solar System, it is the fourth-largest planet by diameter, the third-most-massive planet, + and the densest giant planet. Neptune is 17 times the mass of Earth and is slightly more + massive than its near-twin Uranus, which is 15 times the mass of Earth and slightly larger + than Neptune. Neptune orbits the Sun once every 164.8 years at an average distance of 30.1 + astronomical units (4.50×109 km). It is named after the Roman god of the sea and has the + astronomical symbol ♆, a stylised version of the god Neptune's trident. +

+ + + + + + Read more on Wikipedia + + ` +}) +export class ContentElementDialog { } diff --git a/src/lib/dialog/README.md b/src/lib/dialog/README.md index d68f89f54aff..194b8fe0b969 100644 --- a/src/lib/dialog/README.md +++ b/src/lib/dialog/README.md @@ -4,15 +4,16 @@ MdDialog is a service, which opens dialogs components in the view. ### Methods -| Name | Description | -| --- | --- | +| Name | Description | +| ---- | ----------- | | `open(component: ComponentType, config: MdDialogConfig): MdDialogRef` | Creates and opens a dialog matching material spec. | | `closeAll(): void` | Closes all of the dialogs that are currently open. | +| `closeTop(): void` | Closes the topmost of the open dialogs. | ### Config -| Key | Description | -| --- | --- | +| Key | Description | +| --- | ------------ | | `role: DialogRole = 'dialog'` | The ARIA role of the dialog element. Possible values are `dialog` and `alertdialog`. Optional. | | `disableClose: boolean = false` | Whether to prevent the user from closing a dialog by clicking on the backdrop or pressing escape. Optional. | | `width: string = ''` | Width of the dialog. Takes any valid CSS value. Optional. | @@ -26,11 +27,19 @@ A reference to the dialog created by the MdDialog `open` method. ### Methods -| Name | Description | -| --- | --- | +| Name | Description | +| ---- | ----------- | | `close(dialogResult?: any)` | Closes the dialog, pushing a value to the afterClosed observable. | | `afterClosed(): Observable` | Returns an observable which will emit the dialog result, passed to the `close` method above. | +### Directives +| Name | Description | +| --- | ------------ | +| `md-dialog-title` | Marks the title of the dialog. +| `md-dialog-content` | Scrollable content of the dialog. +| `md-dialog-close` | When added to a `button`, makes the element act as a close button for the dialog. +| `md-dialog-actions` | Wrapper for the set of actions at the bottom of a dialog. Typically contains buttons. + ## Example The service can be injected in a component. @@ -62,8 +71,12 @@ export class PizzaComponent { @Component({ selector: 'pizza-dialog', template: ` - - +

Would you like to order pizza?

+ + + + + ` }) export class PizzaDialog { diff --git a/src/lib/dialog/dialog-container.scss b/src/lib/dialog/dialog-container.scss deleted file mode 100644 index c7675efbbbcc..000000000000 --- a/src/lib/dialog/dialog-container.scss +++ /dev/null @@ -1,24 +0,0 @@ -@import '../core/style/elevation'; -@import '../core/a11y/a11y'; - - -$md-dialog-padding: 24px !default; -$md-dialog-border-radius: 2px !default; - -md-dialog-container { - @include md-elevation(24); - - display: block; - padding: $md-dialog-padding; - border-radius: $md-dialog-border-radius; - box-sizing: border-box; - overflow: auto; - - // The dialog container should completely fill its parent overlay element. - width: 100%; - height: 100%; - - @include md-high-contrast { - outline: solid 1px; - } -} diff --git a/src/lib/dialog/dialog-container.ts b/src/lib/dialog/dialog-container.ts index 39a2db89cc50..c9726d9499c3 100644 --- a/src/lib/dialog/dialog-container.ts +++ b/src/lib/dialog/dialog-container.ts @@ -22,7 +22,7 @@ import 'rxjs/add/operator/first'; moduleId: module.id, selector: 'md-dialog-container, mat-dialog-container', templateUrl: 'dialog-container.html', - styleUrls: ['dialog-container.css'], + styleUrls: ['dialog.css'], host: { 'class': 'md-dialog-container', '[attr.role]': 'dialogConfig?.role', diff --git a/src/lib/dialog/dialog-directives.ts b/src/lib/dialog/dialog-directives.ts new file mode 100644 index 000000000000..3c201a65d6aa --- /dev/null +++ b/src/lib/dialog/dialog-directives.ts @@ -0,0 +1,49 @@ +import {Directive} from '@angular/core'; +import {MdDialog} from './dialog'; + + +/** + * Button that will close the current dialog. + */ +@Directive({ + selector: 'button[md-dialog-close]', + host: { + '(click)': 'dialog.closeTop()' + } +}) +export class MdDialogClose { + constructor(public dialog: MdDialog) { } +} + +/** + * Title of a dialog element. Stays fixed to the top of the dialog when scrolling. + */ +@Directive({ + selector: '[md-dialog-title]', + host: { + role: 'heading' + } +}) +export class MdDialogTitle { } + + +/** + * Scrollable content container of a dialog. + */ +@Directive({ + selector: '[md-dialog-content], md-dialog-content', + host: { + role: 'main' + } +}) +export class MdDialogContent { } + + +/** + * Container for the bottom action buttons in a dialog. + * Stays fixed to the bottom when scrolling. + */ +@Directive({ + selector: '[md-dialog-actions], md-dialog-actions' +}) +export class MdDialogActions { } diff --git a/src/lib/dialog/dialog.scss b/src/lib/dialog/dialog.scss new file mode 100644 index 000000000000..c7e7a5fce6af --- /dev/null +++ b/src/lib/dialog/dialog.scss @@ -0,0 +1,52 @@ +@import '../core/style/elevation'; +@import '../core/a11y/a11y'; + + +$md-dialog-padding: 24px !default; +$md-dialog-border-radius: 2px !default; +$md-dialog-max-width: 80vw !default; +$md-dialog-max-height: 65vh !default; + +md-dialog-container { + @include md-elevation(24); + + display: block; + padding: $md-dialog-padding; + border-radius: $md-dialog-border-radius; + box-sizing: border-box; + overflow: auto; + max-width: $md-dialog-max-width; + + // The dialog container should completely fill its parent overlay element. + width: 100%; + height: 100%; + + @include md-high-contrast { + outline: solid 1px; + } +} + +md-dialog-content, [md-dialog-content] { + display: block; + margin: 0 $md-dialog-padding * -1; + padding: 0 $md-dialog-padding; + max-height: $md-dialog-max-height; + overflow: auto; +} + +[md-dialog-title] { + font-size: rem(2); + font-weight: bold; + letter-spacing: 0.005em; + margin: 0 0 rem(2); + display: block; +} + +md-dialog-actions, [md-dialog-actions] { + padding: $md-dialog-padding / 2 0; + display: block; + + &:last-child { + margin-bottom: -$md-dialog-padding; + } +} diff --git a/src/lib/dialog/dialog.spec.ts b/src/lib/dialog/dialog.spec.ts index 34855db3f59a..be6d5a9b48a6 100644 --- a/src/lib/dialog/dialog.spec.ts +++ b/src/lib/dialog/dialog.spec.ts @@ -9,7 +9,8 @@ import { } from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {NgModule, Component, Directive, ViewChild, ViewContainerRef} from '@angular/core'; -import {MdDialog, MdDialogModule} from './dialog'; +import {MdDialogModule} from './index'; +import {MdDialog} from './dialog'; import {OverlayContainer} from '../core'; import {MdDialogRef} from './dialog-ref'; import {MdDialogContainer} from './dialog-container'; @@ -226,6 +227,21 @@ describe('MdDialog', () => { expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0); }); + it('should close the top dialog', () => { + dialog.open(PizzaMsg); + dialog.open(PizzaMsg); + + expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(2); + + dialog.closeTop(); + + expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(1); + + dialog.closeTop(); + + expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0); + }); + describe('disableClose option', () => { it('should prevent closing via clicks on the backdrop', () => { dialog.open(PizzaMsg, { @@ -308,6 +324,44 @@ describe('MdDialog', () => { .toBe('dialog-trigger', 'Expected that the trigger was refocused after dialog close'); })); }); + + describe('dialog content elements', () => { + let fixture: ComponentFixture; + let dialogElement: HTMLElement; + + beforeEach(() => { + fixture = TestBed.createComponent(ContentElementDialog); + dialogElement = fixture.debugElement.nativeElement; + dialog.open(ContentElementDialog); + fixture.detectChanges(); + }); + + it('close the dialog when clicking on the close button', () => { + expect(overlayContainerElement.querySelectorAll('.md-dialog-container').length).toBe(1); + + (dialogElement.querySelector('button[md-dialog-close]') as HTMLElement).click(); + + expect(overlayContainerElement.querySelectorAll('.md-dialog-container').length).toBe(0); + }); + + it('close not close the dialog if [md-dialog-close] is applied on a non-button node', () => { + expect(overlayContainerElement.querySelectorAll('.md-dialog-container').length).toBe(1); + + (dialogElement.querySelector('div[md-dialog-close]') as HTMLElement).click(); + + expect(overlayContainerElement.querySelectorAll('.md-dialog-container').length).toBe(1); + }); + + it('should add a role to the dialog title', () => { + let header = dialogElement.querySelector('[md-dialog-title]'); + expect(header.getAttribute('role')).toBe('heading'); + }); + + it('should add a role to the dialog content', () => { + let content = dialogElement.querySelector('md-dialog-content'); + expect(content.getAttribute('role')).toBe('main'); + }); + }); }); @@ -334,13 +388,31 @@ class PizzaMsg { constructor(public dialogRef: MdDialogRef) { } } +@Component({ + template: ` +

This is the title

+ Lorem ipsum dolor sit amet. + + +
Should not close
+ + ` +}) +class ContentElementDialog {} + // Create a real (non-test) NgModule as a workaround for // https://github.com/angular/angular/issues/10760 -const TEST_DIRECTIVES = [ComponentWithChildViewContainer, PizzaMsg, DirectiveWithViewContainer]; +const TEST_DIRECTIVES = [ + ComponentWithChildViewContainer, + PizzaMsg, + DirectiveWithViewContainer, + ContentElementDialog +]; + @NgModule({ imports: [MdDialogModule], exports: TEST_DIRECTIVES, declarations: TEST_DIRECTIVES, - entryComponents: [ComponentWithChildViewContainer, PizzaMsg], + entryComponents: [ComponentWithChildViewContainer, PizzaMsg, ContentElementDialog], }) class DialogTestModule { } diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index fe15864617af..26c0f7150de1 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -1,29 +1,15 @@ -import {NgModule, ModuleWithProviders, Injector, ComponentRef, Injectable} from '@angular/core'; -import { - Overlay, - OverlayModule, - PortalModule, - OverlayRef, - OverlayState, - ComponentPortal, - OVERLAY_PROVIDERS, - ComponentType, - A11yModule, - InteractivityChecker, - MdPlatform, - DefaultStyleCompatibilityModeModule, -} from '../core'; +import {Injector, ComponentRef, Injectable} from '@angular/core'; + +import {Overlay, OverlayRef, ComponentType, OverlayState, ComponentPortal} from '../core'; +import {extendObject} from '../core/util/object-extend'; + +import {DialogInjector} from './dialog-injector'; import {MdDialogConfig} from './dialog-config'; import {MdDialogRef} from './dialog-ref'; -import {DialogInjector} from './dialog-injector'; import {MdDialogContainer} from './dialog-container'; -import {extendObject} from '../core/util/object-extend'; -export {MdDialogConfig} from './dialog-config'; -export {MdDialogRef} from './dialog-ref'; // TODO(jelbourn): add support for opening with a TemplateRef -// TODO(jelbourn): dialog content directives (e.g., md-dialog-header) // TODO(jelbourn): animations @@ -71,6 +57,17 @@ export class MdDialog { } } + /** + * Closes the top dialog + */ + closeTop(): void { + let dialogs = this._openDialogs.length; + + if (dialogs) { + this._openDialogs[dialogs - 1].close(); + } + } + /** * Creates the overlay into which the dialog will be loaded. * @param dialogConfig The dialog configuration. @@ -184,18 +181,3 @@ function _applyConfigDefaults(dialogConfig: MdDialogConfig): MdDialogConfig { return extendObject(new MdDialogConfig(), dialogConfig); } - -@NgModule({ - imports: [OverlayModule, PortalModule, A11yModule, DefaultStyleCompatibilityModeModule], - exports: [MdDialogContainer, DefaultStyleCompatibilityModeModule], - declarations: [MdDialogContainer], - entryComponents: [MdDialogContainer], -}) -export class MdDialogModule { - static forRoot(): ModuleWithProviders { - return { - ngModule: MdDialogModule, - providers: [MdDialog, OVERLAY_PROVIDERS, InteractivityChecker, MdPlatform], - }; - } -} diff --git a/src/lib/dialog/index.ts b/src/lib/dialog/index.ts index bac800313647..4f805e945f7f 100644 --- a/src/lib/dialog/index.ts +++ b/src/lib/dialog/index.ts @@ -1,4 +1,54 @@ +import {NgModule, ModuleWithProviders} from '@angular/core'; +import { + OverlayModule, + PortalModule, + OVERLAY_PROVIDERS, + A11yModule, + InteractivityChecker, + MdPlatform, + DefaultStyleCompatibilityModeModule, +} from '../core'; + +import {MdDialog} from './dialog'; +import {MdDialogContainer} from './dialog-container'; +import {MdDialogClose, MdDialogContent, MdDialogTitle, MdDialogActions} from './dialog-directives'; + + +@NgModule({ + imports: [ + OverlayModule, + PortalModule, + A11yModule, + DefaultStyleCompatibilityModeModule + ], + exports: [ + MdDialogContainer, + MdDialogClose, + MdDialogTitle, + MdDialogContent, + MdDialogActions, + DefaultStyleCompatibilityModeModule + ], + declarations: [ + MdDialogContainer, + MdDialogClose, + MdDialogTitle, + MdDialogActions, + MdDialogContent + ], + entryComponents: [MdDialogContainer], +}) +export class MdDialogModule { + static forRoot(): ModuleWithProviders { + return { + ngModule: MdDialogModule, + providers: [MdDialog, OVERLAY_PROVIDERS, InteractivityChecker, MdPlatform], + }; + } +} + export * from './dialog'; export * from './dialog-container'; +export * from './dialog-directives'; export * from './dialog-config'; export * from './dialog-ref'; From becff1dc849ddff3f3fb3da998909eb9b5c13e59 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 8 Dec 2016 18:16:22 +0100 Subject: [PATCH 2/9] Rename the dialog directives file. --- ...dialog-directives.ts => dialog-content-directives.ts} | 0 src/lib/dialog/index.ts | 9 +++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) rename src/lib/dialog/{dialog-directives.ts => dialog-content-directives.ts} (100%) diff --git a/src/lib/dialog/dialog-directives.ts b/src/lib/dialog/dialog-content-directives.ts similarity index 100% rename from src/lib/dialog/dialog-directives.ts rename to src/lib/dialog/dialog-content-directives.ts diff --git a/src/lib/dialog/index.ts b/src/lib/dialog/index.ts index 4f805e945f7f..fba41fd5e751 100644 --- a/src/lib/dialog/index.ts +++ b/src/lib/dialog/index.ts @@ -11,7 +11,12 @@ import { import {MdDialog} from './dialog'; import {MdDialogContainer} from './dialog-container'; -import {MdDialogClose, MdDialogContent, MdDialogTitle, MdDialogActions} from './dialog-directives'; +import { + MdDialogClose, + MdDialogContent, + MdDialogTitle, + MdDialogActions +} from './dialog-content-directives'; @NgModule({ @@ -49,6 +54,6 @@ export class MdDialogModule { export * from './dialog'; export * from './dialog-container'; -export * from './dialog-directives'; +export * from './dialog-content-directives'; export * from './dialog-config'; export * from './dialog-ref'; From cf767fe85bb69396c79a66bcec965b847770a2d3 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 8 Dec 2016 18:26:39 +0100 Subject: [PATCH 3/9] Add the selectors for Material 1 compatibility. --- src/lib/dialog/dialog-content-directives.ts | 8 ++++---- src/lib/dialog/dialog.scss | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/lib/dialog/dialog-content-directives.ts b/src/lib/dialog/dialog-content-directives.ts index 3c201a65d6aa..93e5e28efe97 100644 --- a/src/lib/dialog/dialog-content-directives.ts +++ b/src/lib/dialog/dialog-content-directives.ts @@ -6,7 +6,7 @@ import {MdDialog} from './dialog'; * Button that will close the current dialog. */ @Directive({ - selector: 'button[md-dialog-close]', + selector: 'button[md-dialog-close], button[mat-dialog-close]', host: { '(click)': 'dialog.closeTop()' } @@ -19,7 +19,7 @@ export class MdDialogClose { * Title of a dialog element. Stays fixed to the top of the dialog when scrolling. */ @Directive({ - selector: '[md-dialog-title]', + selector: '[md-dialog-title], [mat-dialog-title]', host: { role: 'heading' } @@ -31,7 +31,7 @@ export class MdDialogTitle { } * Scrollable content container of a dialog. */ @Directive({ - selector: '[md-dialog-content], md-dialog-content', + selector: '[md-dialog-content], md-dialog-content, [mat-dialog-content], mat-dialog-content', host: { role: 'main' } @@ -44,6 +44,6 @@ export class MdDialogContent { } * Stays fixed to the bottom when scrolling. */ @Directive({ - selector: '[md-dialog-actions], md-dialog-actions' + selector: '[md-dialog-actions], md-dialog-actions, [mat-dialog-actions], mat-dialog-actions' }) export class MdDialogActions { } diff --git a/src/lib/dialog/dialog.scss b/src/lib/dialog/dialog.scss index c7e7a5fce6af..4597fdd0eaa0 100644 --- a/src/lib/dialog/dialog.scss +++ b/src/lib/dialog/dialog.scss @@ -26,7 +26,7 @@ md-dialog-container { } } -md-dialog-content, [md-dialog-content] { +md-dialog-content, [md-dialog-content], mat-dialog-content, [mat-dialog-content] { display: block; margin: 0 $md-dialog-padding * -1; padding: 0 $md-dialog-padding; @@ -34,7 +34,7 @@ md-dialog-content, [md-dialog-content] { overflow: auto; } -[md-dialog-title] { +[md-dialog-title], [mat-dialog-title] { font-size: rem(2); font-weight: bold; letter-spacing: 0.005em; @@ -42,7 +42,7 @@ md-dialog-content, [md-dialog-content] { display: block; } -md-dialog-actions, [md-dialog-actions] { +md-dialog-actions, [md-dialog-actions], mat-dialog-actions, [mat-dialog-actions] { padding: $md-dialog-padding / 2 0; display: block; From e7712cbd6191ce133d38e89e9ddbb87ff1de0228 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 8 Dec 2016 18:32:04 +0100 Subject: [PATCH 4/9] Remove the closeTop method and use the dialogRef instead. --- src/lib/dialog/dialog-content-directives.ts | 6 +++--- src/lib/dialog/dialog.spec.ts | 15 --------------- src/lib/dialog/dialog.ts | 11 ----------- 3 files changed, 3 insertions(+), 29 deletions(-) diff --git a/src/lib/dialog/dialog-content-directives.ts b/src/lib/dialog/dialog-content-directives.ts index 93e5e28efe97..7887f92d589b 100644 --- a/src/lib/dialog/dialog-content-directives.ts +++ b/src/lib/dialog/dialog-content-directives.ts @@ -1,5 +1,5 @@ import {Directive} from '@angular/core'; -import {MdDialog} from './dialog'; +import {MdDialogRef} from './dialog-ref'; /** @@ -8,11 +8,11 @@ import {MdDialog} from './dialog'; @Directive({ selector: 'button[md-dialog-close], button[mat-dialog-close]', host: { - '(click)': 'dialog.closeTop()' + '(click)': 'dialogRef.close()' } }) export class MdDialogClose { - constructor(public dialog: MdDialog) { } + constructor(public dialogRef: MdDialogRef) { } } /** diff --git a/src/lib/dialog/dialog.spec.ts b/src/lib/dialog/dialog.spec.ts index be6d5a9b48a6..c623dd0b3eb2 100644 --- a/src/lib/dialog/dialog.spec.ts +++ b/src/lib/dialog/dialog.spec.ts @@ -227,21 +227,6 @@ describe('MdDialog', () => { expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0); }); - it('should close the top dialog', () => { - dialog.open(PizzaMsg); - dialog.open(PizzaMsg); - - expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(2); - - dialog.closeTop(); - - expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(1); - - dialog.closeTop(); - - expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0); - }); - describe('disableClose option', () => { it('should prevent closing via clicks on the backdrop', () => { dialog.open(PizzaMsg, { diff --git a/src/lib/dialog/dialog.ts b/src/lib/dialog/dialog.ts index 26c0f7150de1..8c10cfd7d421 100644 --- a/src/lib/dialog/dialog.ts +++ b/src/lib/dialog/dialog.ts @@ -57,17 +57,6 @@ export class MdDialog { } } - /** - * Closes the top dialog - */ - closeTop(): void { - let dialogs = this._openDialogs.length; - - if (dialogs) { - this._openDialogs[dialogs - 1].close(); - } - } - /** * Creates the overlay into which the dialog will be loaded. * @param dialogConfig The dialog configuration. From 4f078ec42a33f62fad3e8d3b8ea372f08b98c5bd Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 8 Dec 2016 19:33:07 +0100 Subject: [PATCH 5/9] Add an aria-label to the close button and simplify the testing setup. --- src/lib/dialog/dialog-content-directives.ts | 8 +++-- src/lib/dialog/dialog.spec.ts | 37 +++++++++++++-------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/lib/dialog/dialog-content-directives.ts b/src/lib/dialog/dialog-content-directives.ts index 7887f92d589b..ae2958335b43 100644 --- a/src/lib/dialog/dialog-content-directives.ts +++ b/src/lib/dialog/dialog-content-directives.ts @@ -1,4 +1,4 @@ -import {Directive} from '@angular/core'; +import {Directive, Input} from '@angular/core'; import {MdDialogRef} from './dialog-ref'; @@ -8,10 +8,14 @@ import {MdDialogRef} from './dialog-ref'; @Directive({ selector: 'button[md-dialog-close], button[mat-dialog-close]', host: { - '(click)': 'dialogRef.close()' + '(click)': 'dialogRef.close()', + '[attr.aria-label]': 'ariaLabel' } }) export class MdDialogClose { + /** Screenreader label for the button. */ + @Input('aria-label') ariaLabel: string = 'Close dialog'; + constructor(public dialogRef: MdDialogRef) { } } diff --git a/src/lib/dialog/dialog.spec.ts b/src/lib/dialog/dialog.spec.ts index c623dd0b3eb2..d836c62e2b5f 100644 --- a/src/lib/dialog/dialog.spec.ts +++ b/src/lib/dialog/dialog.spec.ts @@ -311,41 +311,48 @@ describe('MdDialog', () => { }); describe('dialog content elements', () => { - let fixture: ComponentFixture; - let dialogElement: HTMLElement; + let dialogRef: MdDialogRef; beforeEach(() => { - fixture = TestBed.createComponent(ContentElementDialog); - dialogElement = fixture.debugElement.nativeElement; - dialog.open(ContentElementDialog); - fixture.detectChanges(); + dialogRef = dialog.open(ContentElementDialog); + viewContainerFixture.detectChanges(); }); - it('close the dialog when clicking on the close button', () => { + it('should close the dialog when clicking on the close button', () => { expect(overlayContainerElement.querySelectorAll('.md-dialog-container').length).toBe(1); - (dialogElement.querySelector('button[md-dialog-close]') as HTMLElement).click(); + (overlayContainerElement.querySelector('button[md-dialog-close]') as HTMLElement).click(); expect(overlayContainerElement.querySelectorAll('.md-dialog-container').length).toBe(0); }); - it('close not close the dialog if [md-dialog-close] is applied on a non-button node', () => { + it('should not close the dialog if [md-dialog-close] is applied on a non-button node', () => { expect(overlayContainerElement.querySelectorAll('.md-dialog-container').length).toBe(1); - (dialogElement.querySelector('div[md-dialog-close]') as HTMLElement).click(); + (overlayContainerElement.querySelector('div[md-dialog-close]') as HTMLElement).click(); expect(overlayContainerElement.querySelectorAll('.md-dialog-container').length).toBe(1); }); + it('should allow for a user-specified aria-label on the close button', () => { + let button = overlayContainerElement.querySelector('button[md-dialog-close]'); + + dialogRef.componentInstance.closeButtonAriaLabel = 'Best close button ever'; + viewContainerFixture.detectChanges(); + + expect(button.getAttribute('aria-label')).toBe('Best close button ever'); + }); + it('should add a role to the dialog title', () => { - let header = dialogElement.querySelector('[md-dialog-title]'); + let header = overlayContainerElement.querySelector('[md-dialog-title]'); expect(header.getAttribute('role')).toBe('heading'); }); it('should add a role to the dialog content', () => { - let content = dialogElement.querySelector('md-dialog-content'); + let content = overlayContainerElement.querySelector('md-dialog-content'); expect(content.getAttribute('role')).toBe('main'); }); + }); }); @@ -378,12 +385,14 @@ class PizzaMsg {

This is the title

Lorem ipsum dolor sit amet. - +
Should not close
 ` }) -class ContentElementDialog {} +class ContentElementDialog { + closeButtonAriaLabel: string; +} // Create a real (non-test) NgModule as a workaround for // https://github.com/angular/angular/issues/10760 From 6e89905cb08b1c5460615f21459dbd01993afff6 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 8 Dec 2016 19:34:01 +0100 Subject: [PATCH 6/9] Remove redundant element roles. --- src/lib/dialog/dialog-content-directives.ts | 10 ++-------- src/lib/dialog/dialog.spec.ts | 10 ---------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/src/lib/dialog/dialog-content-directives.ts b/src/lib/dialog/dialog-content-directives.ts index ae2958335b43..a7c8e3875ef1 100644 --- a/src/lib/dialog/dialog-content-directives.ts +++ b/src/lib/dialog/dialog-content-directives.ts @@ -23,10 +23,7 @@ export class MdDialogClose { * Title of a dialog element. Stays fixed to the top of the dialog when scrolling. */ @Directive({ - selector: '[md-dialog-title], [mat-dialog-title]', - host: { - role: 'heading' - } + selector: '[md-dialog-title], [mat-dialog-title]' }) export class MdDialogTitle { } @@ -35,10 +32,7 @@ export class MdDialogTitle { } * Scrollable content container of a dialog. */ @Directive({ - selector: '[md-dialog-content], md-dialog-content, [mat-dialog-content], mat-dialog-content', - host: { - role: 'main' - } + selector: '[md-dialog-content], md-dialog-content, [mat-dialog-content], mat-dialog-content' }) export class MdDialogContent { } diff --git a/src/lib/dialog/dialog.spec.ts b/src/lib/dialog/dialog.spec.ts index d836c62e2b5f..e88377b7593d 100644 --- a/src/lib/dialog/dialog.spec.ts +++ b/src/lib/dialog/dialog.spec.ts @@ -343,16 +343,6 @@ describe('MdDialog', () => { expect(button.getAttribute('aria-label')).toBe('Best close button ever'); }); - it('should add a role to the dialog title', () => { - let header = overlayContainerElement.querySelector('[md-dialog-title]'); - expect(header.getAttribute('role')).toBe('heading'); - }); - - it('should add a role to the dialog content', () => { - let content = overlayContainerElement.querySelector('md-dialog-content'); - expect(content.getAttribute('role')).toBe('main'); - }); - }); }); From 1c88fbc4176c548c9d409d89f56ae0d3229aef2a Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 8 Dec 2016 19:35:52 +0100 Subject: [PATCH 7/9] Use the computed value on the dialog title font size. --- src/lib/dialog/dialog.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dialog/dialog.scss b/src/lib/dialog/dialog.scss index 4597fdd0eaa0..29fd95c0b57f 100644 --- a/src/lib/dialog/dialog.scss +++ b/src/lib/dialog/dialog.scss @@ -35,10 +35,10 @@ md-dialog-content, [md-dialog-content], mat-dialog-content, [mat-dialog-content] } [md-dialog-title], [mat-dialog-title] { - font-size: rem(2); + font-size: 20px; font-weight: bold; letter-spacing: 0.005em; - margin: 0 0 rem(2); + margin: 0 0 20px; display: block; } From 8043139d53258ec6cc8fb6f480c5a34a12406148 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 8 Dec 2016 19:36:22 +0100 Subject: [PATCH 8/9] Remove the letter spacing override on the dialog title. --- src/lib/dialog/dialog.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/dialog/dialog.scss b/src/lib/dialog/dialog.scss index 29fd95c0b57f..05753b3a5559 100644 --- a/src/lib/dialog/dialog.scss +++ b/src/lib/dialog/dialog.scss @@ -37,7 +37,6 @@ md-dialog-content, [md-dialog-content], mat-dialog-content, [mat-dialog-content] [md-dialog-title], [mat-dialog-title] { font-size: 20px; font-weight: bold; - letter-spacing: 0.005em; margin: 0 0 20px; display: block; } From eeff3e117e7b75450348100a4fa9c7890fb42c04 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 8 Dec 2016 19:40:22 +0100 Subject: [PATCH 9/9] Add a comment regarding the negative bottom margin on the dialog actions. --- src/lib/dialog/dialog.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib/dialog/dialog.scss b/src/lib/dialog/dialog.scss index 05753b3a5559..06492d08629c 100644 --- a/src/lib/dialog/dialog.scss +++ b/src/lib/dialog/dialog.scss @@ -46,6 +46,9 @@ md-dialog-actions, [md-dialog-actions], mat-dialog-actions, [mat-dialog-actions] display: block; &:last-child { + // If the actions are the last element in a dialog, we need to pull them down + // over the dialog padding, in order to avoid the action's padding stacking + // with the dialog's. margin-bottom: -$md-dialog-padding; } }