Skip to content

Commit 731ba44

Browse files
committed
fix(overlay): incorrect bounding box styles if position is exactly zero
Handles the edge case where the bounding box styles won't be set correctly, causing the overlay to jump to a wrong position, if either of its values works out to exactly zero.
1 parent c28549d commit 731ba44

File tree

2 files changed

+108
-4
lines changed

2 files changed

+108
-4
lines changed

src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,6 +1119,110 @@ describe('FlexibleConnectedPositionStrategy', () => {
11191119
expect(Math.floor(overlayRect.height)).toBe(OVERLAY_HEIGHT);
11201120
});
11211121

1122+
it('should set the proper styles when the `bottom` value is exactly zero', () => {
1123+
originElement.style.position = 'fixed';
1124+
originElement.style.bottom = '0';
1125+
originElement.style.left = '200px';
1126+
1127+
positionStrategy
1128+
.withFlexibleWidth()
1129+
.withFlexibleHeight()
1130+
.withPositions([{
1131+
overlayY: 'bottom',
1132+
overlayX: 'start',
1133+
originY: 'bottom',
1134+
originX: 'start'
1135+
}]);
1136+
1137+
attachOverlay({positionStrategy});
1138+
1139+
const boundingBox = overlayContainer
1140+
.getContainerElement()
1141+
.querySelector('.cdk-overlay-connected-position-bounding-box') as HTMLElement;
1142+
1143+
// Ensure that `0px` is set explicitly, rather than the
1144+
// property being left blank due to zero being falsy.
1145+
expect(boundingBox.style.bottom).toBe('0px');
1146+
});
1147+
1148+
it('should set the proper styles when the `top` value is exactly zero', () => {
1149+
originElement.style.position = 'fixed';
1150+
originElement.style.top = '0';
1151+
originElement.style.left = '200px';
1152+
1153+
positionStrategy
1154+
.withFlexibleWidth()
1155+
.withFlexibleHeight()
1156+
.withPositions([{
1157+
overlayY: 'top',
1158+
overlayX: 'start',
1159+
originY: 'top',
1160+
originX: 'start'
1161+
}]);
1162+
1163+
attachOverlay({positionStrategy});
1164+
1165+
const boundingBox = overlayContainer
1166+
.getContainerElement()
1167+
.querySelector('.cdk-overlay-connected-position-bounding-box') as HTMLElement;
1168+
1169+
// Ensure that `0px` is set explicitly, rather than the
1170+
// property being left blank due to zero being falsy.
1171+
expect(boundingBox.style.top).toBe('0px');
1172+
});
1173+
1174+
it('should set the proper styles when the `left` value is exactly zero', () => {
1175+
originElement.style.position = 'fixed';
1176+
originElement.style.left = '0';
1177+
originElement.style.top = '200px';
1178+
1179+
positionStrategy
1180+
.withFlexibleWidth()
1181+
.withFlexibleHeight()
1182+
.withPositions([{
1183+
overlayY: 'top',
1184+
overlayX: 'start',
1185+
originY: 'top',
1186+
originX: 'start'
1187+
}]);
1188+
1189+
attachOverlay({positionStrategy});
1190+
1191+
const boundingBox = overlayContainer
1192+
.getContainerElement()
1193+
.querySelector('.cdk-overlay-connected-position-bounding-box') as HTMLElement;
1194+
1195+
// Ensure that `0px` is set explicitly, rather than the
1196+
// property being left blank due to zero being falsy.
1197+
expect(boundingBox.style.left).toBe('0px');
1198+
});
1199+
1200+
it('should set the proper styles when the `right` value is exactly zero', () => {
1201+
originElement.style.position = 'fixed';
1202+
originElement.style.right = '0';
1203+
originElement.style.top = '200px';
1204+
1205+
positionStrategy
1206+
.withFlexibleWidth()
1207+
.withFlexibleHeight()
1208+
.withPositions([{
1209+
overlayY: 'top',
1210+
overlayX: 'end',
1211+
originY: 'top',
1212+
originX: 'end'
1213+
}]);
1214+
1215+
attachOverlay({positionStrategy});
1216+
1217+
const boundingBox = overlayContainer
1218+
.getContainerElement()
1219+
.querySelector('.cdk-overlay-connected-position-bounding-box') as HTMLElement;
1220+
1221+
// Ensure that `0px` is set explicitly, rather than the
1222+
// property being left blank due to zero being falsy.
1223+
expect(boundingBox.style.right).toBe('0px');
1224+
});
1225+
11221226
});
11231227

11241228
describe('onPositionChange with scrollable view properties', () => {

src/cdk/overlay/position/flexible-connected-position-strategy.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -635,8 +635,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
635635
styles.height = '100%';
636636
} else {
637637
styles.height = `${boundingBoxRect.height}px`;
638-
styles.top = boundingBoxRect.top ? `${boundingBoxRect.top}px` : '';
639-
styles.bottom = boundingBoxRect.bottom ? `${boundingBoxRect.bottom}px` : '';
638+
styles.top = boundingBoxRect.top != null ? `${boundingBoxRect.top}px` : '';
639+
styles.bottom = boundingBoxRect.bottom != null ? `${boundingBoxRect.bottom}px` : '';
640640
}
641641

642642
if (!this._hasFlexibleWidth || this._isPushed) {
@@ -645,8 +645,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
645645
styles.width = '100%';
646646
} else {
647647
styles.width = `${boundingBoxRect.width}px`;
648-
styles.left = boundingBoxRect.left ? `${boundingBoxRect.left}px` : '';
649-
styles.right = boundingBoxRect.right ? `${boundingBoxRect.right}px` : '';
648+
styles.left = boundingBoxRect.left != null ? `${boundingBoxRect.left}px` : '';
649+
styles.right = boundingBoxRect.right != null ? `${boundingBoxRect.right}px` : '';
650650
}
651651

652652
const maxHeight = this._overlayRef.getConfig().maxHeight;

0 commit comments

Comments
 (0)