Skip to content

Commit da33c03

Browse files
trshaferjelbourn
authored andcommitted
fix(overlay): Render the templates before placing them in the overlay. (#2989)
This fixes positioning issues when rendering templates which contain embedded templates. The templates need to be rendered in order to properly determine width which can then determine placement.
1 parent eef57f6 commit da33c03

File tree

6 files changed

+104
-21
lines changed

6 files changed

+104
-21
lines changed

src/demo-app/overlay/overlay-demo.html

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
Pasta 3
1111
</button>
1212

13+
<button cdk-overlay-origin #tortelliniOrigin="cdkOverlayOrigin" (click)="openTortelliniPanel()">
14+
Pasta 4
15+
</button>
16+
1317

1418
<button cdk-overlay-origin #trigger="cdkOverlayOrigin" (click)="isMenuOpen = !isMenuOpen">
1519
Open menu
@@ -27,4 +31,8 @@
2731
<p class="demo-fusilli"> Fusilli </p>
2832
</ng-template>
2933

34+
<ng-template cdk-portal #tortelliniTemplate="cdkPortal">
35+
<ul class="demo-tortellini"><li *ngFor="let filling of tortelliniFillings">{{filling}}</li></ul>
36+
</ng-template>
37+
3038
<button (click)="openPanelWithBackdrop()">Backdrop panel</button>

src/demo-app/overlay/overlay-demo.scss

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,12 @@
1818
background-color: rebeccapurple;
1919
opacity: 0.5;
2020
}
21+
22+
.demo-tortellini {
23+
margin: 0;
24+
padding: 10px;
25+
border: 1px solid black;
26+
color: white;
27+
background-color: orangered;
28+
opacity: 0.5;
29+
}

src/demo-app/overlay/overlay-demo.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,12 @@ import {
2626
export class OverlayDemo {
2727
nextPosition: number = 0;
2828
isMenuOpen: boolean = false;
29+
tortelliniFillings = ['cheese and spinach', 'mushroom and broccoli'];
2930

3031
@ViewChildren(TemplatePortalDirective) templatePortals: QueryList<Portal<any>>;
3132
@ViewChild(OverlayOrigin) _overlayOrigin: OverlayOrigin;
33+
@ViewChild('tortelliniOrigin') tortelliniOrigin: OverlayOrigin;
34+
@ViewChild('tortelliniTemplate') tortelliniTemplate: TemplatePortalDirective;
3235

3336
constructor(public overlay: Overlay, public viewContainerRef: ViewContainerRef) { }
3437

@@ -75,6 +78,21 @@ export class OverlayDemo {
7578
overlayRef.attach(new ComponentPortal(SpagettiPanel, this.viewContainerRef));
7679
}
7780

81+
openTortelliniPanel() {
82+
let strategy = this.overlay.position()
83+
.connectedTo(
84+
this.tortelliniOrigin.elementRef,
85+
{originX: 'start', originY: 'bottom'},
86+
{overlayX: 'end', overlayY: 'top'} );
87+
88+
let config = new OverlayState();
89+
config.positionStrategy = strategy;
90+
91+
let overlayRef = this.overlay.create(config);
92+
93+
overlayRef.attach(this.tortelliniTemplate);
94+
}
95+
7896
openPanelWithBackdrop() {
7997
let config = new OverlayState();
8098

src/lib/core/portal/dom-portal-host.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export class DomPortalHost extends BasePortalHost {
6464
attachTemplatePortal(portal: TemplatePortal): Map<string, any> {
6565
let viewContainer = portal.viewContainerRef;
6666
let viewRef = viewContainer.createEmbeddedView(portal.templateRef);
67+
viewRef.detectChanges();
6768

6869
// The method `createEmbeddedView` will add the view as a child of the viewContainer.
6970
// But for the DomPortalHost the view can be added everywhere in the DOM (e.g Overlay Container)

src/lib/core/portal/portal.spec.ts

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
Injector,
1212
ApplicationRef,
1313
} from '@angular/core';
14+
import {CommonModule} from '@angular/common';
1415
import {TemplatePortalDirective, PortalHostDirective, PortalModule} from './portal-directives';
1516
import {Portal, ComponentPortal} from './portal';
1617
import {DomPortalHost} from './dom-portal-host';
@@ -123,6 +124,28 @@ describe('Portals', () => {
123124
expect(hostContainer.textContent).toContain('Mango');
124125
});
125126

127+
it('should load a <ng-template> portal with an inner template', () => {
128+
let testAppComponent = fixture.debugElement.componentInstance;
129+
130+
// Detect changes initially so that the component's ViewChildren are resolved.
131+
fixture.detectChanges();
132+
133+
// Set the selectedHost to be a TemplatePortal.
134+
testAppComponent.selectedPortal = testAppComponent.portalWithTemplate;
135+
fixture.detectChanges();
136+
137+
// Expect that the content of the attached portal is present.
138+
let hostContainer = fixture.nativeElement.querySelector('.portal-container');
139+
expect(hostContainer.textContent).toContain('Pineapple');
140+
141+
// When updating the binding value.
142+
testAppComponent.fruits = ['Mangosteen'];
143+
fixture.detectChanges();
144+
145+
// Expect the new value to be reflected in the rendered output.
146+
expect(hostContainer.textContent).toContain('Mangosteen');
147+
});
148+
126149
it('should change the attached portal', () => {
127150
let testAppComponent = fixture.debugElement.componentInstance;
128151

@@ -258,6 +281,15 @@ describe('Portals', () => {
258281
expect(someDomElement.textContent).toContain('Cake');
259282
});
260283

284+
it('should render a template portal with an inner template', () => {
285+
let fixture = TestBed.createComponent(PortalTestApp);
286+
fixture.detectChanges();
287+
288+
fixture.componentInstance.portalWithTemplate.attach(host);
289+
290+
expect(someDomElement.textContent).toContain('Durian');
291+
});
292+
261293
it('should attach and detach a template portal with a binding', () => {
262294
let fixture = TestBed.createComponent(PortalTestApp);
263295

@@ -384,14 +416,21 @@ class ArbitraryViewContainerRefComponent {
384416
<ng-template cdk-portal>Cake</ng-template>
385417
386418
<div *cdk-portal>Pie</div>
387-
388-
<ng-template cdk-portal> {{fruit}} </ng-template>`,
419+
<ng-template cdk-portal> {{fruit}} </ng-template>
420+
421+
<ng-template cdk-portal>
422+
<ul>
423+
<li *ngFor="let fruitName of fruits"> {{fruitName}} </li>
424+
</ul>
425+
</ng-template>
426+
`,
389427
})
390428
class PortalTestApp {
391429
@ViewChildren(TemplatePortalDirective) portals: QueryList<TemplatePortalDirective>;
392430
@ViewChild(PortalHostDirective) portalHost: PortalHostDirective;
393431
selectedPortal: Portal<any>;
394432
fruit: string = 'Banana';
433+
fruits = ['Apple', 'Pineapple', 'Durian'];
395434

396435
constructor(public injector: Injector) { }
397436

@@ -406,13 +445,17 @@ class PortalTestApp {
406445
get portalWithBinding() {
407446
return this.portals.toArray()[2];
408447
}
448+
449+
get portalWithTemplate() {
450+
return this.portals.toArray()[3];
451+
}
409452
}
410453

411454
// Create a real (non-test) NgModule as a workaround for
412455
// https://github.com/angular/angular/issues/10760
413456
const TEST_COMPONENTS = [PortalTestApp, ArbitraryViewContainerRefComponent, PizzaMsg];
414457
@NgModule({
415-
imports: [PortalModule],
458+
imports: [CommonModule, PortalModule],
416459
exports: TEST_COMPONENTS,
417460
declarations: TEST_COMPONENTS,
418461
entryComponents: TEST_COMPONENTS,

src/lib/select/select.spec.ts

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,34 +1040,38 @@ describe('MdSelect', () => {
10401040
select.style.marginRight = '20px';
10411041
});
10421042

1043-
it('should adjust for the checkbox in ltr', () => {
1043+
it('should adjust for the checkbox in ltr', async(() => {
10441044
trigger.click();
10451045
multiFixture.detectChanges();
10461046

1047-
const triggerLeft = trigger.getBoundingClientRect().left;
1048-
const firstOptionLeft =
1049-
document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().left;
1047+
multiFixture.whenStable().then(() => {
1048+
const triggerLeft = trigger.getBoundingClientRect().left;
1049+
const firstOptionLeft =
1050+
document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().left;
10501051

1051-
// 48px accounts for the checkbox size, margin and the panel's padding.
1052-
expect(firstOptionLeft.toFixed(2))
1053-
.toEqual((triggerLeft - 48).toFixed(2),
1054-
`Expected trigger label to align along x-axis, accounting for the checkbox.`);
1055-
});
1052+
// 48px accounts for the checkbox size, margin and the panel's padding.
1053+
expect(firstOptionLeft.toFixed(2))
1054+
.toEqual((triggerLeft - 48).toFixed(2),
1055+
`Expected trigger label to align along x-axis, accounting for the checkbox.`);
1056+
});
1057+
}));
10561058

1057-
it('should adjust for the checkbox in rtl', () => {
1059+
it('should adjust for the checkbox in rtl', async(() => {
10581060
dir.value = 'rtl';
10591061
trigger.click();
10601062
multiFixture.detectChanges();
10611063

1062-
const triggerRight = trigger.getBoundingClientRect().right;
1063-
const firstOptionRight =
1064-
document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right;
1064+
multiFixture.whenStable().then(() => {
1065+
const triggerRight = trigger.getBoundingClientRect().right;
1066+
const firstOptionRight =
1067+
document.querySelector('.cdk-overlay-pane md-option').getBoundingClientRect().right;
10651068

1066-
// 48px accounts for the checkbox size, margin and the panel's padding.
1067-
expect(firstOptionRight.toFixed(2))
1068-
.toEqual((triggerRight + 48).toFixed(2),
1069-
`Expected trigger label to align along x-axis, accounting for the checkbox.`);
1070-
});
1069+
// 48px accounts for the checkbox size, margin and the panel's padding.
1070+
expect(firstOptionRight.toFixed(2))
1071+
.toEqual((triggerRight + 48).toFixed(2),
1072+
`Expected trigger label to align along x-axis, accounting for the checkbox.`);
1073+
});
1074+
}));
10711075
});
10721076

10731077
});

0 commit comments

Comments
 (0)