Skip to content

feat(overlay): add ability to set custom class on panes #1438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

feat(overlay): add ability to set custom class on panes #1438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
109c61b
add ability to set custom class on overlay pane, implement in dialog …
Oct 5, 2016
5eafef5
fix missing semicolon
Oct 5, 2016
bdddf59
remove unused import
fxck Oct 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/demo-app/dialog/dialog-demo.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
.demo-dialog {
color: rebeccapurple;
}

// apply custom z-index
/deep/ .md-overlay-pane {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're avoiding using /deep/ at all in material2.

&.jazz-dialog {
z-index: 9999;
}
}
3 changes: 2 additions & 1 deletion src/demo-app/dialog/dialog-demo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {MdDialog, MdDialogConfig, MdDialogRef} from '@angular/material';
moduleId: module.id,
selector: 'dialog-demo',
templateUrl: 'dialog-demo.html',
styleUrls: ['dialog-demo.css'],
styleUrls: ['dialog-demo.css']
})
export class DialogDemo {
dialogRef: MdDialogRef<JazzDialog>;
Expand All @@ -18,6 +18,7 @@ export class DialogDemo {
open() {
let config = new MdDialogConfig();
config.viewContainerRef = this.viewContainerRef;
config.paneClassName = 'jazz-dialog';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't need to be part of the demo


this.dialogRef = this.dialog.open(JazzDialog, config);

Expand Down
16 changes: 16 additions & 0 deletions src/demo-app/menu/menu-demo.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,20 @@
</button>
</md-menu>
</div>
<div class="menu-section">
<p>Custom classes on menu' overlay pane</p>

<md-toolbar>
<button md-icon-button [md-menu-trigger-for]="menuCustomClassName">
<md-icon>more_vert</md-icon>
</button>
</md-toolbar>

<md-menu #menuCustomClassName="mdMenu" [paneClassName]="['foo', 'bar']">

<button md-menu-item *ngFor="let item of items" (click)="select(item.text)" [disabled]="item.disabled">
{{ item.text }}
</button>
</md-menu>
</div>
</div>
3 changes: 3 additions & 0 deletions src/lib/core/overlay/overlay-state.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ export class OverlayState {
/** Whether the overlay has a backdrop. */
hasBackdrop: boolean = false;

/** Optional custom class to be added to the pane. */
paneClassName: string | Array<string>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make this overlayClass to align with backdropClass (I don't believe we expose the term "pane" in the public API).


// TODO(jelbourn): configuration still to add
// - overlay size
// - focus trap
Expand Down
1 change: 0 additions & 1 deletion src/lib/core/overlay/overlay.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
left: 0;
height: 100%;
width: 100%;
z-index: $md-z-index-overlay-container;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it HAS to be removed, otherwise any inner z-index will be ignored.

See http://jsbin.com/mesujat/1/edit?html,css,output yellow is over green, despite it having thousand times higher z-index.


// A single overlay pane.
Expand Down
17 changes: 14 additions & 3 deletions src/lib/core/overlay/overlay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class Overlay {
* @returns A reference to the created overlay.
*/
create(state: OverlayState = defaultState): OverlayRef {
return this._createOverlayRef(this._createPaneElement(), state);
return this._createOverlayRef(this._createPaneElement(state), state);
}

/**
Expand All @@ -52,10 +52,21 @@ export class Overlay {
* Creates the DOM element for an overlay and appends it to the overlay container.
* @returns Promise resolving to the created element.
*/
private _createPaneElement(): HTMLElement {
private _createPaneElement(state: OverlayState): HTMLElement {
var pane = document.createElement('div');
pane.id = `md-overlay-${nextUniqueId++}`;
const id = nextUniqueId++;
pane.id = `md-overlay-${id}`;
pane.classList.add('md-overlay-pane');
pane.classList.add(`md-has-index-${id}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this class for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you can more granularly control stacking order of multiple panes inside a single overlay.. I guess you could do the same with :nth-child.. but you know, with BEM and everything having a class could be handy.

I won't mind removing it though, you tell me.


if (state.paneClassName) {
// check if custom class is an array of classes
if (state.paneClassName.constructor === Array) {
pane.classList.add(...<Array<string>>state.paneClassName);
} else {
pane.classList.add(<string>state.paneClassName);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a bit cleaner:

let overlayClass = state.overlayClass;
if (overlayClass) {
  if (typeof overlayClass == 'string') {
    overlayClass = [overlayClass];
  }
  pane.classList.add(...overlayClass);
}


this._overlayContainer.getContainerElement().appendChild(pane);

Expand Down
3 changes: 3 additions & 0 deletions src/lib/dialog/dialog-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ export class MdDialogConfig {
/** The ARIA role of the dialog element. */
role: DialogRole = 'dialog';

/** Optional custom class to be added to dialog's overlay pane. */
paneClassName: string | Array<string>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

string | string[]


// TODO(jelbourn): add configuration for size, clickOutsideToClose, lifecycle hooks,
// ARIA labelling.
}
4 changes: 4 additions & 0 deletions src/lib/dialog/dialog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ export class MdDialog {
.centerHorizontally()
.centerVertically();

if (dialogConfig.paneClassName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your use-cases for exposing this through to dialog and menu? I can see the case for custom components built on top of overlay, but I'm not sure if it's the right API surface for the leaf components.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole #1432 is a use-case for menu, see http://plnkr.co/edit/AqjYdsScCYPlK9w44aHQ?p=preview (try opening the menu and then clicking on something).. if I removed z-index from md-overlay-container, it would work, but only as long as my fixed element had z-index lower than 1000.. with this option I could simple set the menu pane to always have higher z-index.

Copy link
Contributor Author

@fxck fxck Oct 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing for dialog really, I could have a fixed element that will have z-index higher than 1000 and it would be overlaying the backdrop.

state.paneClassName = dialogConfig.paneClassName;
}

return state;
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/lib/menu/menu-directive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ export class MdMenu {
}, {});
}

/**
* Takes either a string or array of strins and applies them to the
* parent overlay pane, giving consumer control over z-indexes
*/
@Input('paneClassName') paneClassName: string | Array<string>;

@Output() close = new EventEmitter;

/**
Expand Down
8 changes: 7 additions & 1 deletion src/lib/menu/menu-trigger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,14 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
* @returns OverlayState
*/
private _getOverlayConfig(): OverlayState {
const overlayState = new OverlayState();
let overlayState: OverlayState = new OverlayState();
overlayState.positionStrategy = this._getPosition();

// check if custom paneClassName is set on menu and pass it to overlay config
if (this.menu.paneClassName) {
overlayState.paneClassName = this.menu.paneClassName;
}

return overlayState;
}

Expand Down