Skip to content

Commit 07bfd5b

Browse files
crisbetoandrewseguin
authored andcommitted
fix(overlay): incorrectly calculating using minWidth and minHeight as a string (#18540)
We use `minWidth` and `minHeight` to figure out whether an overlay can fit with its flexible dimensions. The problem is that the values can be either a string or a number, and if they're a string, the comparison breaks down because the value we're comparing it against is always a number. These changes also add a couple of extra tests for `minWidth` and `minHeight` since we didn't have any. Fixes #18486. (cherry picked from commit 0d7e2f0)
1 parent 2454e18 commit 07bfd5b

File tree

2 files changed

+113
-2
lines changed

2 files changed

+113
-2
lines changed

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

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2079,6 +2079,100 @@ describe('FlexibleConnectedPositionStrategy', () => {
20792079
expect(boundingBoxStyle.maxHeight).toBeFalsy();
20802080
});
20812081

2082+
it('should collapse the overlay vertically if overlay is outside of viewport, but taller ' +
2083+
'than the minHeight', () => {
2084+
const bottomOffset = OVERLAY_HEIGHT / 2;
2085+
originElement.style.bottom = `${bottomOffset}px`;
2086+
originElement.style.left = '50%';
2087+
originElement.style.position = 'fixed';
2088+
2089+
positionStrategy
2090+
.withFlexibleDimensions()
2091+
.withPush(true)
2092+
.withPositions([{
2093+
overlayY: 'top',
2094+
overlayX: 'start',
2095+
originY: 'bottom',
2096+
originX: 'start',
2097+
}]);
2098+
2099+
attachOverlay({positionStrategy, minHeight: bottomOffset - 1});
2100+
const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
2101+
2102+
expect(Math.floor(overlayRect.height)).toBe(bottomOffset);
2103+
});
2104+
2105+
it('should collapse the overlay vertically if overlay is outside of viewport, but taller ' +
2106+
'than the minHeight that is set as a pixel string', () => {
2107+
const bottomOffset = OVERLAY_HEIGHT / 2;
2108+
originElement.style.bottom = `${bottomOffset}px`;
2109+
originElement.style.left = '50%';
2110+
originElement.style.position = 'fixed';
2111+
2112+
positionStrategy
2113+
.withFlexibleDimensions()
2114+
.withPush(true)
2115+
.withPositions([{
2116+
overlayY: 'top',
2117+
overlayX: 'start',
2118+
originY: 'bottom',
2119+
originX: 'start',
2120+
}]);
2121+
2122+
attachOverlay({positionStrategy, minHeight: `${bottomOffset - 1}px`});
2123+
const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
2124+
2125+
expect(Math.floor(overlayRect.height)).toBe(bottomOffset);
2126+
});
2127+
2128+
it('should collapse the overlay horizontally if overlay is outside of viewport, but wider ' +
2129+
'than the minWidth', () => {
2130+
const rightOffset = OVERLAY_WIDTH / 2;
2131+
originElement.style.top = '50%';
2132+
originElement.style.right = `${rightOffset}px`;
2133+
originElement.style.position = 'fixed';
2134+
2135+
positionStrategy
2136+
.withFlexibleDimensions()
2137+
.withPush(true)
2138+
.withPositions([{
2139+
overlayY: 'top',
2140+
overlayX: 'start',
2141+
originY: 'top',
2142+
originX: 'end',
2143+
}]);
2144+
2145+
attachOverlay({positionStrategy, minWidth: rightOffset});
2146+
const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
2147+
2148+
expect(Math.floor(overlayRect.width)).toBe(rightOffset);
2149+
});
2150+
2151+
it('should collapse the overlay horizontally if overlay is outside of viewport, but wider ' +
2152+
'than the minWidth that is set as a pixel string', () => {
2153+
const rightOffset = OVERLAY_WIDTH / 2;
2154+
originElement.style.top = '50%';
2155+
originElement.style.right = `${rightOffset}px`;
2156+
originElement.style.position = 'fixed';
2157+
2158+
positionStrategy
2159+
.withFlexibleDimensions()
2160+
.withPush(true)
2161+
.withPositions([{
2162+
overlayY: 'top',
2163+
overlayX: 'start',
2164+
originY: 'top',
2165+
originX: 'end',
2166+
}]);
2167+
2168+
attachOverlay({positionStrategy, minWidth: `${rightOffset}px`});
2169+
const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
2170+
2171+
expect(Math.floor(overlayRect.width)).toBe(rightOffset);
2172+
});
2173+
2174+
2175+
20822176
});
20832177

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

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ import {OverlayContainer} from '../overlay-container';
2929
/** Class to be added to the overlay bounding box. */
3030
const boundingBoxClass = 'cdk-overlay-connected-position-bounding-box';
3131

32+
/** Regex used to split a string on its CSS units. */
33+
const cssUnitPattern = /([A-Za-z%]+)$/;
34+
3235
/** Possible values that can be set as the origin of a FlexibleConnectedPositionStrategy. */
3336
export type FlexibleConnectedPositionStrategyOrigin = ElementRef | HTMLElement | Point & {
3437
width?: number;
@@ -567,8 +570,8 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
567570
if (this._hasFlexibleDimensions) {
568571
const availableHeight = viewport.bottom - point.y;
569572
const availableWidth = viewport.right - point.x;
570-
const minHeight = this._overlayRef.getConfig().minHeight;
571-
const minWidth = this._overlayRef.getConfig().minWidth;
573+
const minHeight = getPixelValue(this._overlayRef.getConfig().minHeight);
574+
const minWidth = getPixelValue(this._overlayRef.getConfig().minWidth);
572575

573576
const verticalFit = fit.fitsInViewportVertically ||
574577
(minHeight != null && minHeight <= availableHeight);
@@ -1199,3 +1202,17 @@ function extendStyles(destination: CSSStyleDeclaration,
11991202

12001203
return destination;
12011204
}
1205+
1206+
1207+
/**
1208+
* Extracts the pixel value as a number from a value, if it's a number
1209+
* or a CSS pixel string (e.g. `1337px`). Otherwise returns null.
1210+
*/
1211+
function getPixelValue(input: number|string|null|undefined): number|null {
1212+
if (typeof input !== 'number' && input != null) {
1213+
const [value, units] = input.split(cssUnitPattern);
1214+
return (!units || units === 'px') ? parseFloat(value) : null;
1215+
}
1216+
1217+
return input || null;
1218+
}

0 commit comments

Comments
 (0)