Skip to content

Commit

Permalink
refactor(dialog): better handling of scrollable content
Browse files Browse the repository at this point in the history
Currently, the dialog's scrollable content (md-dialog-content) is limited to 65% of the viewport height, however on smaller screens the dialog still ends up being too high. This proposal reworks the `md-dialog-content` to make it take up all of the remaining height, instead of being hardcoded to 65%. The max height is provided by the wrapper instead.

Fixes #2481.
  • Loading branch information
crisbeto committed Jan 5, 2017
1 parent f4f69c4 commit 8eacecf
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 7 deletions.
14 changes: 13 additions & 1 deletion src/lib/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import {
ViewEncapsulation,
NgZone,
OnDestroy,
Renderer,
} from '@angular/core';
import {BasePortalHost, ComponentPortal, PortalHostDirective, TemplatePortal} from '../core';
import {MdDialogConfig} from './dialog-config';
import {MdDialogRef} from './dialog-ref';
import {MD_DIALOG_CONTENT_SELECTOR} from './dialog-content-directives';
import {MdDialogContentAlreadyAttachedError} from './dialog-errors';
import {FocusTrap} from '../core/a11y/focus-trap';
import 'rxjs/add/operator/first';
Expand Down Expand Up @@ -46,7 +48,7 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy {
/** Reference to the open dialog. */
dialogRef: MdDialogRef<any>;

constructor(private _ngZone: NgZone) {
constructor(private _ngZone: NgZone, private _renderer: Renderer) {
super();
}

Expand All @@ -60,6 +62,16 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy {
}

let attachResult = this._portalHost.attachComponentPortal(portal);
let componentElement = attachResult.location.nativeElement;

// Add a class that we can use for styling the root element.
this._renderer.setElementClass(componentElement, 'md-dialog-root', true);

// Add flexbox styling if the user is using the `md-dialog-content`.
if ('querySelector' in componentElement) {
this._renderer.setElementClass(componentElement, 'md-dialog-root-flex',
!!componentElement.querySelector(MD_DIALOG_CONTENT_SELECTOR));
}

// If were to attempt to focus immediately, then the content of the dialog would not yet be
// ready in instances where change detection has to run first. To deal with this, we simply
Expand Down
5 changes: 4 additions & 1 deletion src/lib/dialog/dialog-content-directives.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import {Directive, Input} from '@angular/core';
import {MdDialogRef} from './dialog-ref';

export const MD_DIALOG_CONTENT_SELECTOR = '[md-dialog-content], md-dialog-content' +
', [mat-dialog-content], mat-dialog-content';


/**
* Button that will close the current dialog.
Expand Down Expand Up @@ -32,7 +35,7 @@ export class MdDialogTitle { }
* Scrollable content container of a dialog.
*/
@Directive({
selector: '[md-dialog-content], md-dialog-content, [mat-dialog-content], mat-dialog-content'
selector: MD_DIALOG_CONTENT_SELECTOR
})
export class MdDialogContent { }

Expand Down
14 changes: 11 additions & 3 deletions src/lib/dialog/dialog.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
$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-max-height: 80vh !default;

md-dialog-container {
@include md-elevation(24);
Expand All @@ -15,7 +15,6 @@ md-dialog-container {
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%;
Expand All @@ -26,11 +25,20 @@ md-dialog-container {
}
}

.md-dialog-root {
max-height: calc(#{$md-dialog-max-height} - #{$md-dialog-padding * 2});
max-width: $md-dialog-max-width;

&.md-dialog-root-flex {
display: flex;
flex-flow: column;
}
}

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;
max-height: $md-dialog-max-height;
overflow: auto;
}

Expand Down
25 changes: 23 additions & 2 deletions src/lib/dialog/dialog.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,23 @@ describe('MdDialog', () => {
expect(overlayContainerElement.querySelectorAll('md-dialog-container').length).toBe(0);
});

it('should add a class for styling to the root element', () => {
dialog.open(PizzaMsg);

let pizzaMsgContainer = overlayContainerElement.querySelector('pizza-msg');

expect(pizzaMsgContainer.classList).toContain('md-dialog-root');
expect(pizzaMsgContainer.classList).not.toContain('md-dialog-root-flex');
});

it('should add an extra flexbox class to components that use md-dialog-content', () => {
dialog.open(ContentElementDialog);

let dialogContainer = overlayContainerElement.querySelector('content-element-dialog');

expect(dialogContainer.classList).toContain('md-dialog-root-flex');
});

describe('disableClose option', () => {
it('should prevent closing via clicks on the backdrop', () => {
dialog.open(PizzaMsg, {
Expand Down Expand Up @@ -365,7 +382,10 @@ class ComponentWithChildViewContainer {
}

/** Simple component for testing ComponentPortal. */
@Component({template: '<p>Pizza</p> <input> <button>Close</button>'})
@Component({
template: '<p>Pizza</p> <input> <button>Close</button>',
selector: 'pizza-msg'
})
class PizzaMsg {
constructor(public dialogRef: MdDialogRef<PizzaMsg>) { }
}
Expand All @@ -378,7 +398,8 @@ class PizzaMsg {
<button md-dialog-close [aria-label]="closeButtonAriaLabel">Close</button>
<div md-dialog-close>Should not close</div>
</md-dialog-actions>
`
`,
selector: 'content-element-dialog'
})
class ContentElementDialog {
closeButtonAriaLabel: string;
Expand Down

0 comments on commit 8eacecf

Please sign in to comment.