Skip to content

Commit 9f2c937

Browse files
crisbetojelbourn
authored andcommitted
fix(overlay): don't reset global overlay alignment when width is 100% and there's a maxWidth (#17842)
A long time ago we introduced some logic that clears the `justifyContent` from a global overlay if it's `width` is set to 100%, in order to ensure that the element is flush against the viewport edge. Some time later we added a `maxWidth` option, but we never accounted for it which means that if an element is set to be `width: 100%; maxWidth: '500px'`, we'll reset the alignment incorrectly. These changes tweak the logic so it only resets if there is no `maxWidth` or if it's set to 100%. Fixes #17841.
1 parent 3e7982a commit 9f2c937

File tree

2 files changed

+79
-6
lines changed

2 files changed

+79
-6
lines changed

src/cdk/overlay/position/global-position-strategy.spec.ts

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ describe('GlobalPositonStrategy', () => {
195195
attachOverlay({
196196
positionStrategy: overlay.position()
197197
.global()
198-
.centerHorizontally()
198+
.centerHorizontally('10px')
199199
.width('100%')
200200
});
201201

@@ -206,11 +206,45 @@ describe('GlobalPositonStrategy', () => {
206206
expect(parentStyle.justifyContent).toBe('flex-start');
207207
});
208208

209+
it('should reset the horizontal position and offset when the width is 100% and the ' +
210+
'maxWidth is 100%', () => {
211+
attachOverlay({
212+
maxWidth: '100%',
213+
positionStrategy: overlay.position()
214+
.global()
215+
.centerHorizontally('10px')
216+
.width('100%')
217+
});
218+
219+
const elementStyle = overlayRef.overlayElement.style;
220+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
221+
222+
expect(elementStyle.marginLeft).toBe('0px');
223+
expect(parentStyle.justifyContent).toBe('flex-start');
224+
});
225+
226+
it('should not reset the horizontal position and offset when the width is 100% and' +
227+
'there is a defined maxWidth', () => {
228+
attachOverlay({
229+
maxWidth: '500px',
230+
positionStrategy: overlay.position()
231+
.global()
232+
.centerHorizontally('10px')
233+
.width('100%')
234+
});
235+
236+
const elementStyle = overlayRef.overlayElement.style;
237+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
238+
239+
expect(elementStyle.marginLeft).toBe('10px');
240+
expect(parentStyle.justifyContent).toBe('center');
241+
});
242+
209243
it('should reset the vertical position and offset when the height is 100%', () => {
210244
attachOverlay({
211245
positionStrategy: overlay.position()
212246
.global()
213-
.centerVertically()
247+
.centerVertically('10px')
214248
.height('100%')
215249
});
216250

@@ -221,6 +255,40 @@ describe('GlobalPositonStrategy', () => {
221255
expect(parentStyle.alignItems).toBe('flex-start');
222256
});
223257

258+
it('should reset the vertical position and offset when the height is 100% and the ' +
259+
'maxHeight is 100%', () => {
260+
attachOverlay({
261+
maxHeight: '100%',
262+
positionStrategy: overlay.position()
263+
.global()
264+
.centerVertically('10px')
265+
.height('100%')
266+
});
267+
268+
const elementStyle = overlayRef.overlayElement.style;
269+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
270+
271+
expect(elementStyle.marginTop).toBe('0px');
272+
expect(parentStyle.alignItems).toBe('flex-start');
273+
});
274+
275+
it('should not reset the vertical position and offset when the height is 100% and ' +
276+
'there is a defined maxHeight', () => {
277+
attachOverlay({
278+
maxHeight: '500px',
279+
positionStrategy: overlay.position()
280+
.global()
281+
.centerVertically('10px')
282+
.height('100%')
283+
});
284+
285+
const elementStyle = overlayRef.overlayElement.style;
286+
const parentStyle = (overlayRef.overlayElement.parentNode as HTMLElement).style;
287+
288+
expect(elementStyle.marginTop).toBe('10px');
289+
expect(parentStyle.alignItems).toBe('center');
290+
});
291+
224292
it('should not throw when attempting to apply after the overlay has been disposed', () => {
225293
const positionStrategy = overlay.position().global();
226294

src/cdk/overlay/position/global-position-strategy.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,14 +164,19 @@ export class GlobalPositionStrategy implements PositionStrategy {
164164
const styles = this._overlayRef.overlayElement.style;
165165
const parentStyles = this._overlayRef.hostElement.style;
166166
const config = this._overlayRef.getConfig();
167+
const {width, height, maxWidth, maxHeight} = config;
168+
const shouldBeFlushHorizontally = (width === '100%' || width === '100vw') &&
169+
(!maxWidth || maxWidth === '100%' || maxWidth === '100vw');
170+
const shouldBeFlushVertically = (height === '100%' || height === '100vh') &&
171+
(!maxHeight || maxHeight === '100%' || maxHeight === '100vh');
167172

168173
styles.position = this._cssPosition;
169-
styles.marginLeft = config.width === '100%' ? '0' : this._leftOffset;
170-
styles.marginTop = config.height === '100%' ? '0' : this._topOffset;
174+
styles.marginLeft = shouldBeFlushHorizontally ? '0' : this._leftOffset;
175+
styles.marginTop = shouldBeFlushVertically ? '0' : this._topOffset;
171176
styles.marginBottom = this._bottomOffset;
172177
styles.marginRight = this._rightOffset;
173178

174-
if (config.width === '100%') {
179+
if (shouldBeFlushHorizontally) {
175180
parentStyles.justifyContent = 'flex-start';
176181
} else if (this._justifyContent === 'center') {
177182
parentStyles.justifyContent = 'center';
@@ -189,7 +194,7 @@ export class GlobalPositionStrategy implements PositionStrategy {
189194
parentStyles.justifyContent = this._justifyContent;
190195
}
191196

192-
parentStyles.alignItems = config.height === '100%' ? 'flex-start' : this._alignItems;
197+
parentStyles.alignItems = shouldBeFlushVertically ? 'flex-start' : this._alignItems;
193198
}
194199

195200
/**

0 commit comments

Comments
 (0)