Skip to content

Commit 1d8e9af

Browse files
crisbetommalerba
authored andcommitted
fix(overlay): connected overlay directive inputs not updating position strategy (#13066)
Ensures that all of the `@Input` changes get propagated to the underlying position strategy. Up until now we did it on a case-by-case basis which meant that a lot of them weren't, because we never added an extra call inside the `ngOnChanges`.
1 parent e7007a2 commit 1d8e9af

File tree

1 file changed

+33
-45
lines changed

1 file changed

+33
-45
lines changed

src/cdk/overlay/overlay-directives.ts

Lines changed: 33 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
128128
this._offsetX = offsetX;
129129

130130
if (this._position) {
131-
this._setPositions(this._position);
131+
this._updatePositionStrategy(this._position);
132132
}
133133
}
134134

@@ -139,7 +139,7 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
139139
this._offsetY = offsetY;
140140

141141
if (this._position) {
142-
this._setPositions(this._position);
142+
this._updatePositionStrategy(this._position);
143143
}
144144
}
145145

@@ -240,20 +240,10 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
240240

241241
ngOnChanges(changes: SimpleChanges) {
242242
if (this._position) {
243-
if (changes['positions']) {
244-
this._position.withPositions(this.positions);
245-
}
246-
247-
if (changes['lockPosition']) {
248-
this._position.withLockedPosition(this.lockPosition);
249-
}
243+
this._updatePositionStrategy(this._position);
250244

251-
if (changes['origin']) {
252-
this._position.setOrigin(this.origin.elementRef);
253-
254-
if (this.open) {
255-
this._position.apply();
256-
}
245+
if (changes['origin'] && this.open) {
246+
this._position.apply();
257247
}
258248
}
259249

@@ -269,6 +259,14 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
269259
}
270260

271261
this._overlayRef = this._overlay.create(this._buildConfig());
262+
263+
this._overlayRef.keydownEvents().subscribe((event: KeyboardEvent) => {
264+
this.overlayKeydown.next(event);
265+
266+
if (event.keyCode === ESCAPE) {
267+
this._detachOverlay();
268+
}
269+
});
272270
}
273271

274272
/** Builds the overlay config based on the directive's inputs */
@@ -308,51 +306,41 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
308306
return overlayConfig;
309307
}
310308

311-
/** Returns the position strategy of the overlay to be set on the overlay config */
312-
private _createPositionStrategy(): FlexibleConnectedPositionStrategy {
313-
const strategy = this._overlay.position()
314-
.flexibleConnectedTo(this.origin.elementRef)
309+
/** Updates the state of a position strategy, based on the values of the directive inputs. */
310+
private _updatePositionStrategy(positionStrategy: FlexibleConnectedPositionStrategy) {
311+
const positions: ConnectedPosition[] = this.positions.map(currentPosition => ({
312+
originX: currentPosition.originX,
313+
originY: currentPosition.originY,
314+
overlayX: currentPosition.overlayX,
315+
overlayY: currentPosition.overlayY,
316+
offsetX: currentPosition.offsetX || this.offsetX,
317+
offsetY: currentPosition.offsetY || this.offsetY
318+
}));
319+
320+
return positionStrategy
321+
.setOrigin(this.origin.elementRef)
322+
.withPositions(positions)
315323
.withFlexibleDimensions(this.flexibleDimensions)
316324
.withPush(this.push)
317325
.withGrowAfterOpen(this.growAfterOpen)
318326
.withViewportMargin(this.viewportMargin)
319327
.withLockedPosition(this.lockPosition);
328+
}
329+
330+
/** Returns the position strategy of the overlay to be set on the overlay config */
331+
private _createPositionStrategy(): FlexibleConnectedPositionStrategy {
332+
const strategy = this._overlay.position().flexibleConnectedTo(this.origin.elementRef);
320333

321-
this._setPositions(strategy);
334+
this._updatePositionStrategy(strategy);
322335
strategy.positionChanges.subscribe(p => this.positionChange.emit(p));
323336

324337
return strategy;
325338
}
326339

327-
/**
328-
* Sets the primary and fallback positions of a positions strategy,
329-
* based on the current directive inputs.
330-
*/
331-
private _setPositions(positionStrategy: FlexibleConnectedPositionStrategy) {
332-
const positions: ConnectedPosition[] = this.positions.map(pos => ({
333-
originX: pos.originX,
334-
originY: pos.originY,
335-
overlayX: pos.overlayX,
336-
overlayY: pos.overlayY,
337-
offsetX: pos.offsetX || this.offsetX,
338-
offsetY: pos.offsetY || this.offsetY
339-
}));
340-
341-
positionStrategy.withPositions(positions);
342-
}
343-
344340
/** Attaches the overlay and subscribes to backdrop clicks if backdrop exists */
345341
private _attachOverlay() {
346342
if (!this._overlayRef) {
347343
this._createOverlay();
348-
349-
this._overlayRef!.keydownEvents().subscribe((event: KeyboardEvent) => {
350-
this.overlayKeydown.next(event);
351-
352-
if (event.keyCode === ESCAPE) {
353-
this._detachOverlay();
354-
}
355-
});
356344
} else {
357345
// Update the overlay size, in case the directive's inputs have changed
358346
this._overlayRef.updateSize({

0 commit comments

Comments
 (0)