Skip to content

Commit afe1fa4

Browse files
crisbetotinayuangao
authored andcommitted
fix(global-position-strategy): ignoring width and height from OverlayConfig (#9774)
* Switches to using the `width` and `height` from the `OverlayConfig`, rather than the properties on the `GlobalPositionStrategy`. * Deprecates the `GlobalPositionStrategy.width` and `GlobalPositionStrategy.height` methods in favor of passing the dimensions through the config. * Reworks the `GlobalPositionStrategy` testing setup to make it less prone to breaking if any of the `OverlayRef` internals change. Fixes #9715.
1 parent bc07968 commit afe1fa4

File tree

2 files changed

+211
-91
lines changed

2 files changed

+211
-91
lines changed
Lines changed: 173 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,51 @@
1+
import {NgModule, NgZone, Component} from '@angular/core';
12
import {TestBed, inject} from '@angular/core/testing';
2-
import {OverlayModule, Overlay, OverlayRef, GlobalPositionStrategy} from '../index';
3+
import {MockNgZone} from '@angular/cdk/testing';
4+
import {PortalModule, ComponentPortal} from '@angular/cdk/portal';
5+
import {OverlayModule, Overlay, OverlayConfig, OverlayRef} from '../index';
36

47

58
describe('GlobalPositonStrategy', () => {
6-
let element: HTMLElement;
7-
let strategy: GlobalPositionStrategy;
8-
let hasOverlayAttached: boolean;
9+
let overlayRef: OverlayRef;
10+
let overlay: Overlay;
11+
let zone: MockNgZone;
912

1013
beforeEach(() => {
11-
TestBed.configureTestingModule({imports: [OverlayModule]});
14+
TestBed.configureTestingModule({
15+
imports: [GlobalOverlayTestModule],
16+
providers: [{provide: NgZone, useFactory: () => zone = new MockNgZone()}]
17+
});
1218

13-
inject([Overlay], (overlay: Overlay) => {
14-
strategy = overlay.position().global();
19+
inject([Overlay], (o: Overlay) => {
20+
overlay = o;
1521
})();
16-
17-
element = document.createElement('div');
18-
document.body.appendChild(element);
19-
hasOverlayAttached = true;
20-
strategy.attach({
21-
overlayElement: element,
22-
hasAttached: () => hasOverlayAttached
23-
} as OverlayRef);
2422
});
2523

2624
afterEach(() => {
27-
if (element.parentNode) {
28-
element.parentNode.removeChild(element);
25+
if (overlayRef) {
26+
overlayRef.dispose();
27+
overlayRef = null!;
2928
}
30-
31-
strategy.dispose();
3229
});
3330

31+
function attachOverlay(config: OverlayConfig): OverlayRef {
32+
const portal = new ComponentPortal(BlankPortal);
33+
overlayRef = overlay.create(config);
34+
overlayRef.attach(portal);
35+
zone.simulateZoneExit();
36+
return overlayRef;
37+
}
38+
3439
it('should position the element to the (top, left) with an offset', () => {
35-
strategy.top('10px').left('40px').apply();
40+
attachOverlay({
41+
positionStrategy: overlay.position()
42+
.global()
43+
.top('10px')
44+
.left('40px')
45+
});
3646

37-
let elementStyle = element.style;
38-
let parentStyle = (element.parentNode as HTMLElement).style;
47+
const elementStyle = overlayRef.overlayElement.style;
48+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
3949

4050
expect(elementStyle.marginTop).toBe('10px');
4151
expect(elementStyle.marginLeft).toBe('40px');
@@ -47,10 +57,15 @@ describe('GlobalPositonStrategy', () => {
4757
});
4858

4959
it('should position the element to the (bottom, right) with an offset', () => {
50-
strategy.bottom('70px').right('15em').apply();
60+
attachOverlay({
61+
positionStrategy: overlay.position()
62+
.global()
63+
.bottom('70px')
64+
.right('15em')
65+
});
5166

52-
let elementStyle = element.style;
53-
let parentStyle = (element.parentNode as HTMLElement).style;
67+
const elementStyle = overlayRef.overlayElement.style;
68+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
5469

5570
expect(elementStyle.marginTop).toBe('');
5671
expect(elementStyle.marginLeft).toBe('');
@@ -62,11 +77,17 @@ describe('GlobalPositonStrategy', () => {
6277
});
6378

6479
it('should overwrite previously applied positioning', () => {
65-
strategy.centerHorizontally().centerVertically().apply();
66-
strategy.top('10px').left('40%').apply();
80+
const positionStrategy = overlay.position()
81+
.global()
82+
.centerHorizontally()
83+
.centerVertically();
84+
85+
attachOverlay({positionStrategy});
86+
positionStrategy.top('10px').left('40%');
87+
overlayRef.updatePosition();
6788

68-
let elementStyle = element.style;
69-
let parentStyle = (element.parentNode as HTMLElement).style;
89+
const elementStyle = overlayRef.overlayElement.style;
90+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
7091

7192
expect(elementStyle.marginTop).toBe('10px');
7293
expect(elementStyle.marginLeft).toBe('40%');
@@ -76,31 +97,42 @@ describe('GlobalPositonStrategy', () => {
7697
expect(parentStyle.justifyContent).toBe('flex-start');
7798
expect(parentStyle.alignItems).toBe('flex-start');
7899

79-
strategy.bottom('70px').right('15em').apply();
100+
positionStrategy.bottom('70px').right('15em');
101+
overlayRef.updatePosition();
80102

81-
expect(element.style.marginTop).toBe('');
82-
expect(element.style.marginLeft).toBe('');
83-
expect(element.style.marginBottom).toBe('70px');
84-
expect(element.style.marginRight).toBe('15em');
103+
expect(elementStyle.marginTop).toBe('');
104+
expect(elementStyle.marginLeft).toBe('');
105+
expect(elementStyle.marginBottom).toBe('70px');
106+
expect(elementStyle.marginRight).toBe('15em');
85107

86108
expect(parentStyle.justifyContent).toBe('flex-end');
87109
expect(parentStyle.alignItems).toBe('flex-end');
88110
});
89111

90112
it('should center the element', () => {
91-
strategy.centerHorizontally().centerVertically().apply();
113+
attachOverlay({
114+
positionStrategy: overlay.position()
115+
.global()
116+
.centerHorizontally()
117+
.centerVertically()
118+
});
92119

93-
let parentStyle = (element.parentNode as HTMLElement).style;
120+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
94121

95122
expect(parentStyle.justifyContent).toBe('center');
96123
expect(parentStyle.alignItems).toBe('center');
97124
});
98125

99126
it('should center the element with an offset', () => {
100-
strategy.centerHorizontally('10px').centerVertically('15px').apply();
127+
attachOverlay({
128+
positionStrategy: overlay.position()
129+
.global()
130+
.centerHorizontally('10px')
131+
.centerVertically('15px')
132+
});
101133

102-
let elementStyle = element.style;
103-
let parentStyle = (element.parentNode as HTMLElement).style;
134+
const elementStyle = overlayRef.overlayElement.style;
135+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
104136

105137
expect(elementStyle.marginLeft).toBe('10px');
106138
expect(elementStyle.marginTop).toBe('15px');
@@ -110,61 +142,140 @@ describe('GlobalPositonStrategy', () => {
110142
});
111143

112144
it('should make the element position: static', () => {
113-
strategy.apply();
145+
attachOverlay({
146+
positionStrategy: overlay.position().global()
147+
});
114148

115-
expect(element.style.position).toBe('static');
149+
expect(overlayRef.overlayElement.style.position).toBe('static');
116150
});
117151

118152
it('should wrap the element in a `cdk-global-overlay-wrapper`', () => {
119-
strategy.apply();
153+
attachOverlay({
154+
positionStrategy: overlay.position().global()
155+
});
120156

121-
let parent = element.parentNode as HTMLElement;
157+
const parent = overlayRef.overlayElement.parentNode as HTMLElement;
122158

123159
expect(parent.classList.contains('cdk-global-overlay-wrapper')).toBe(true);
124160
});
125161

126-
127162
it('should remove the parent wrapper from the DOM', () => {
128-
strategy.apply();
163+
attachOverlay({
164+
positionStrategy: overlay.position().global()
165+
});
129166

130-
expect(document.body.contains(element.parentNode!)).toBe(true);
167+
expect(document.body.contains(overlayRef.overlayElement.parentNode!)).toBe(true);
131168

132-
strategy.dispose();
169+
overlayRef.dispose();
133170

134-
expect(document.body.contains(element.parentNode!)).toBe(false);
171+
expect(document.body.contains(overlayRef.overlayElement.parentNode!)).toBe(false);
135172
});
136173

137174
it('should set the element width', () => {
138-
strategy.width('100px').apply();
175+
attachOverlay({
176+
positionStrategy: overlay.position().global().width('100px')
177+
});
139178

140-
expect(element.style.width).toBe('100px');
179+
expect(overlayRef.overlayElement.style.width).toBe('100px');
141180
});
142181

143182
it('should set the element height', () => {
144-
strategy.height('100px').apply();
183+
attachOverlay({
184+
positionStrategy: overlay.position().global().height('100px')
185+
});
145186

146-
expect(element.style.height).toBe('100px');
187+
expect(overlayRef.overlayElement.style.height).toBe('100px');
147188
});
148189

149190
it('should reset the horizontal position and offset when the width is 100%', () => {
150-
strategy.centerHorizontally().width('100%').apply();
191+
attachOverlay({
192+
positionStrategy: overlay.position()
193+
.global()
194+
.centerHorizontally()
195+
.width('100%')
196+
});
197+
198+
const elementStyle = overlayRef.overlayElement.style;
199+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
151200

152-
expect(element.style.marginLeft).toBe('0px');
153-
expect((element.parentNode as HTMLElement).style.justifyContent).toBe('flex-start');
201+
expect(elementStyle.marginLeft).toBe('0px');
202+
expect(parentStyle.justifyContent).toBe('flex-start');
154203
});
155204

156205
it('should reset the vertical position and offset when the height is 100%', () => {
157-
strategy.centerVertically().height('100%').apply();
206+
attachOverlay({
207+
positionStrategy: overlay.position()
208+
.global()
209+
.centerVertically()
210+
.height('100%')
211+
});
212+
213+
const elementStyle = overlayRef.overlayElement.style;
214+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
158215

159-
expect(element.style.marginTop).toBe('0px');
160-
expect((element.parentNode as HTMLElement).style.alignItems).toBe('flex-start');
216+
expect(elementStyle.marginTop).toBe('0px');
217+
expect(parentStyle.alignItems).toBe('flex-start');
161218
});
162219

163220
it('should not throw when attempting to apply after the overlay has been disposed', () => {
164-
strategy.dispose();
165-
element.parentNode!.removeChild(element);
166-
hasOverlayAttached = false;
221+
const positionStrategy = overlay.position().global();
222+
223+
attachOverlay({positionStrategy});
224+
225+
positionStrategy.dispose();
167226

168-
expect(() => strategy.apply()).not.toThrow();
227+
expect(() => positionStrategy.apply()).not.toThrow();
169228
});
229+
230+
it('should take its width and height from the overlay config', () => {
231+
attachOverlay({
232+
positionStrategy: overlay.position().global(),
233+
width: '500px',
234+
height: '300px'
235+
});
236+
237+
const elementStyle = overlayRef.overlayElement.style;
238+
239+
expect(elementStyle.width).toBe('500px');
240+
expect(elementStyle.height).toBe('300px');
241+
});
242+
243+
it('should update the overlay size when setting it through the position strategy', () => {
244+
attachOverlay({
245+
positionStrategy: overlay.position()
246+
.global()
247+
.width('500px')
248+
.height('300px'),
249+
});
250+
251+
expect(overlayRef.getConfig().width).toBe('500px');
252+
expect(overlayRef.getConfig().height).toBe('300px');
253+
});
254+
255+
it('should take the dimensions from the overlay config, when they are set both in the ' +
256+
'config and the strategy', () => {
257+
attachOverlay({
258+
positionStrategy: overlay.position().global().width('200px').height('100px'),
259+
width: '500px',
260+
height: '300px'
261+
});
262+
263+
const elementStyle = overlayRef.overlayElement.style;
264+
265+
expect(elementStyle.width).toBe('500px');
266+
expect(elementStyle.height).toBe('300px');
267+
});
268+
170269
});
270+
271+
272+
@Component({template: ''})
273+
class BlankPortal {}
274+
275+
@NgModule({
276+
imports: [OverlayModule, PortalModule],
277+
exports: [BlankPortal],
278+
declarations: [BlankPortal],
279+
entryComponents: [BlankPortal],
280+
})
281+
class GlobalOverlayTestModule {}

0 commit comments

Comments
 (0)