diff --git a/packages/@react-aria/overlays/src/calculatePosition.ts b/packages/@react-aria/overlays/src/calculatePosition.ts index e9696c666e0..c21c28ba342 100644 --- a/packages/@react-aria/overlays/src/calculatePosition.ts +++ b/packages/@react-aria/overlays/src/calculatePosition.ts @@ -275,7 +275,9 @@ function computePosition( // height, as `bottom` will be relative to this height. But if the container is static, // then it can only be the `document.body`, and `bottom` will be relative to _its_ // container. - const containerHeight = (isContainerPositioned ? containerOffsetWithBoundary[size] : containerDimensions[TOTAL_SIZE[size]]); + let containerHeight = (isContainerPositioned ? containerDimensions[size] : containerDimensions[TOTAL_SIZE[size]]); + // containerHeight = 400; + // console.log('container height', containerHeight, isContainerPositioned, containerOffsetWithBoundary) position[FLIPPED_DIRECTION[axis]] = Math.floor(containerHeight - childOffset[axis] + offset); } else { position[axis] = Math.floor(childOffset[axis] + childOffset[size] + offset); @@ -292,51 +294,92 @@ function getMaxHeight( padding: number, overlayHeight: number, heightGrowthDirection: HeightGrowthDirection, - containerDimensions: Dimensions + containerDimensions: Dimensions, + isContainerDescendentOfBoundary: boolean ) { - const containerHeight = (isContainerPositioned ? containerOffsetWithBoundary.height : containerDimensions[TOTAL_SIZE.height]); // For cases where position is set via "bottom" instead of "top", we need to calculate the true overlay top // with respect to the container. - let overlayTop = position.top != null ? position.top : (containerHeight - (position.bottom ?? 0) - overlayHeight); + let overlayTop = (position.top != null ? position.top : (containerDimensions[TOTAL_SIZE.height] - (position.bottom ?? 0) - overlayHeight)) - (containerDimensions.scroll.top ?? 0); + // console + // calculate the dimentions of the "boundingRect" which is most restrictive top/bottom of the boundaryRect and the visual view port + let boundaryToContainerTransformOffset = isContainerDescendentOfBoundary ? containerOffsetWithBoundary.top : 0; + // TODO: calculate the below, will need to add to visualViewport + // let viewportToContainerTransformOffset = container is body or html ? 0 : containerDimensions.top + let boundingRect = { + // TODO: This should be boundary top in container coord system vs viewport top in container coord system + // For the viewport top, there are several cases + // 1. pinchzoom case where we want the viewports offset top as top here + // 2. case where container is offset from the boundary and is contained by the boundary. In this case the top we want here is NOT 0, we want to take boundary's top even though is is a negative number OR the visual viewport, whichever is more restrictive + // is there ever a case where we'd use the container over the visualview port? + // TODO: another issue is getting the proper top of the visual viewport with respect to the container if the container is not the layout viewport + // will need viewportToContainerTransformOffset. Also need to mock visualViewport in the tests? + // top: Math.max(boundaryDimensions.top + boundaryToContainerTransformOffset, visualViewport?.offsetTop ), + top: Math.max(boundaryDimensions.top + boundaryToContainerTransformOffset, visualViewport?.offsetTop ?? boundaryDimensions.top + boundaryToContainerTransformOffset), + // TODO: This should be boundary bottom in container coord system vs viewport bottom in container coord system + bottom: Math.min((boundaryDimensions.top + boundaryDimensions.height + boundaryToContainerTransformOffset), visualViewport?.offsetTop + visualViewport?.height) + // bottom: Math.min((boundaryDimensions.top + boundaryDimensions.height + boundaryToContainerTransformOffset), boundaryDimensions.top + boundaryDimensions.height + boundaryToContainerTransformOffset ) // TODO: not sure if containerHeight is appropriate here + }; + // console.log('bounding rect calcs top', boundaryDimensions.top , boundaryToContainerTransformOffset, visualViewport) let maxHeight = heightGrowthDirection !== 'top' ? // We want the distance between the top of the overlay to the bottom of the boundary Math.max(0, - (boundaryDimensions.height + boundaryDimensions.top) // this is the bottom of the boundary + boundingRect.bottom // this is the bottom of the boundary - overlayTop // this is the top of the overlay - ((margins.top ?? 0) + (margins.bottom ?? 0) + padding) // save additional space for margin and padding ) // We want the distance between the bottom of the overlay to the top of the boundary : Math.max(0, (overlayTop + overlayHeight) // this is the bottom of the overlay - - boundaryDimensions.top // this is the top of the boundary + - boundingRect.top // this is the top of the boundary - ((margins.top ?? 0) + (margins.bottom ?? 0) + padding) // save additional space for margin and padding ); - return Math.min(boundaryDimensions.height - (padding * 2), maxHeight); + // console.log('stuff', overlayTop, overlayHeight, boundingRect, viewportHeight, boundaryDimensions); + // console.log('max height', overlayTop, maxHeight, containerHeight, overlayHeight, boundaryDimensions, containerOffsetWithBoundary) + // console.log('final max', maxHeight, boundaryDimensions.height - (padding * 2)) + // console.log('maxHeight, boundingRec, overlayTop', maxHeight, boundingRect, overlayTop, heightGrowthDirection); + return maxHeight; } function getAvailableSpace( boundaryDimensions: Dimensions, // boundary containerOffsetWithBoundary: Offset, - childOffset: Offset, // trigger + childOffset: Offset, // trigger, position based of container's non-viewport 0,0 margins: Position, // overlay padding: number, // overlay <-> boundary - placementInfo: ParsedPlacement + placementInfo: ParsedPlacement, + containerDimensions: Dimensions, //, + isContainerDescendentOfBoundary: boolean ) { + console.log('iscontainer', isContainerDescendentOfBoundary) let {placement, axis, size} = placementInfo; if (placement === axis) { - return Math.max(0, + return Math.max(0, childOffset[axis] // trigger start - - boundaryDimensions[axis] // boundary start + - (containerDimensions.scroll[axis] ?? 0) // transform trigger position to be with respect to viewport 0,0 + - (boundaryDimensions[axis] + (isContainerDescendentOfBoundary ? containerOffsetWithBoundary[axis] : 0)) // boundary start - (margins[axis] ?? 0) // margins usually for arrows or other decorations - margins[FLIPPED_DIRECTION[axis]] - padding); // padding between overlay and boundary } - + console.log('available', + Math.max(0, + (boundaryDimensions[size] + + boundaryDimensions[axis] + + (isContainerDescendentOfBoundary ? containerOffsetWithBoundary[axis] : 0)) + - childOffset[axis] + - childOffset[size] + + (containerDimensions.scroll[axis] ?? 0) + - (margins[axis] ?? 0) + - margins[FLIPPED_DIRECTION[axis]] + - padding) + ) return Math.max(0, - boundaryDimensions[size] + (boundaryDimensions[size] + boundaryDimensions[axis] + + (isContainerDescendentOfBoundary ? containerOffsetWithBoundary[axis] : 0)) - childOffset[axis] - childOffset[size] + + (containerDimensions.scroll[axis] ?? 0) - (margins[axis] ?? 0) - margins[FLIPPED_DIRECTION[axis]] - padding); @@ -358,7 +401,8 @@ export function calculatePositionInternal( isContainerPositioned: boolean, userSetMaxHeight: number | undefined, arrowSize: number, - arrowBoundaryOffset: number + arrowBoundaryOffset: number, + isContainerDescendentOfBoundary: boolean ): PositionResult { let placementInfo = parsePlacement(placementInput); let {size, crossAxis, crossSize, placement, crossPlacement} = placementInfo; @@ -370,7 +414,9 @@ export function calculatePositionInternal( childOffset, margins, padding + offset, - placementInfo + placementInfo, + containerDimensions, + isContainerDescendentOfBoundary ); // Check if the scroll size of the overlay is greater than the available space to determine if we need to flip @@ -384,7 +430,9 @@ export function calculatePositionInternal( childOffset, margins, padding + offset, - flippedPlacementInfo + flippedPlacementInfo, + containerDimensions, + isContainerDescendentOfBoundary ); // If the available space for the flipped position is greater than the original available space, flip. @@ -423,7 +471,8 @@ export function calculatePositionInternal( padding, overlaySize.height, heightGrowthDirection, - containerDimensions + containerDimensions, + isContainerDescendentOfBoundary ); if (userSetMaxHeight && userSetMaxHeight < maxHeight) { @@ -525,17 +574,31 @@ export function calculatePosition(opts: PositionOpts): PositionResult { overlaySize.height += (margins.top ?? 0) + (margins.bottom ?? 0); let scrollSize = getScroll(scrollNode); + + // TODO: note that due to logic inside getContainerDimensions, for cases where the boundary element is the body, we will return + // a height/width that matches the visual viewport size rather than the body's height/width (aka for zoom it will be zoom adjusted size) + // and a top/left that is adjusted as well (will return the top/left of the zoomed in viewport, or 0,0 for a non-zoomed body) + // Otherwise this returns the height/width of a arbitrary boundary element, and its top/left with respect to the viewport (NOTE THIS MEANS IT DOESNT INCLUDE SCROLL) let boundaryDimensions = getContainerDimensions(boundaryElement); let containerDimensions = getContainerDimensions(container); // If the container is the HTML element wrapping the body element, the retrieved scrollTop/scrollLeft will be equal to the // body element's scroll. Set the container's scroll values to 0 since the overlay's edge position value in getDelta don't then need to be further offset // by the container scroll since they are essentially the same containing element and thus in the same coordinate system - let containerOffsetWithBoundary: Offset = boundaryElement.tagName === 'BODY' ? getOffset(container, false) : getPosition(boundaryElement, container, false); + // TODO: we want these offset values to be positive if boundary is contained within the container, but negative if the container is contained with the boundary + // Ideally we'd just use getPosition(boundaryElement, container, false) here but this returns a 0 height, just need to apply Rob's fixes to the test suite + // let containerOffsetWithBoundary: Offset = boundaryElement.tagName === 'BODY' ? getOffset(container, false) : getPosition(boundaryElement, container, false); if (container.tagName === 'HTML' && boundaryElement.tagName === 'BODY') { - containerDimensions.scroll.top = 0; - containerDimensions.scroll.left = 0; + // containerDimensions.scroll.top = 0; + // containerDimensions.scroll.left = 0; } - + // console.log('gawegaweg', getPosition(boundaryElement, container, false)) + let containerOffsetWithBoundary: Offset = getPosition(boundaryElement, container, false); + console.log('contianer offset with boundary', containerDimensions, boundaryDimensions, targetNode.outerHTML, overlayNode.outerHTML) + console.log('container outer', container.outerHTML) + console.log('boundary outer', boundaryElement.outerHTML) + // console.log('container', container, boundaryElement, containerDimensions, boundaryDimensions, containerOffsetWithBoundary) + + let isContainerDescendentOfBoundary = boundaryElement.contains(container); return calculatePositionInternal( placement, childOffset, @@ -552,7 +615,8 @@ export function calculatePosition(opts: PositionOpts): PositionResult { isContainerPositioned, maxHeight, arrowSize, - arrowBoundaryOffset + arrowBoundaryOffset, + isContainerDescendentOfBoundary ); } diff --git a/packages/@react-aria/overlays/test/calculatePosition.test.ts b/packages/@react-aria/overlays/test/calculatePosition.test.ts index 5eea87941b5..3648ff2a1dd 100644 --- a/packages/@react-aria/overlays/test/calculatePosition.test.ts +++ b/packages/@react-aria/overlays/test/calculatePosition.test.ts @@ -109,14 +109,14 @@ describe('calculatePosition', function () { // The tests are all based on top/left positioning. Convert to bottom/right positioning if needed. let pos: {right?: number, top?: number, left?: number, bottom?: number} = {}; if ((placementAxis === 'left' && !flip) || (placementAxis === 'right' && flip)) { - pos.right = boundaryDimensions.width - (expected[0] + overlaySize.width); + pos.right = containerDimensions.width - (expected[0] + overlaySize.width); pos.top = expected[1]; } else if ((placementAxis === 'right' && !flip) || (placementAxis === 'left' && flip)) { pos.left = expected[0]; pos.top = expected[1]; } else if (placementAxis === 'top') { pos.left = expected[0]; - pos.bottom = boundaryDimensions.height - providerOffset - (expected[1] + overlaySize.height); + pos.bottom = containerDimensions.height - (expected[1] + overlaySize.height); } else if (placementAxis === 'bottom') { pos.left = expected[0]; pos.top = expected[1]; diff --git a/packages/@react-aria/overlays/test/useOverlayPosition.test.tsx b/packages/@react-aria/overlays/test/useOverlayPosition.test.tsx index f7f64969d66..a01adb5e5d5 100644 --- a/packages/@react-aria/overlays/test/useOverlayPosition.test.tsx +++ b/packages/@react-aria/overlays/test/useOverlayPosition.test.tsx @@ -14,7 +14,7 @@ import {fireEvent, render} from '@react-spectrum/test-utils-internal'; import React, {useRef} from 'react'; import {useOverlayPosition} from '../'; -function Example({triggerTop = 250, ...props}) { +function Example({triggerTop = 250, containerStyle = {width: 600, height: 600} as React.CSSProperties, ...props}) { let targetRef = useRef(null); let containerRef = useRef(null); let overlayRef = useRef(null); @@ -23,7 +23,7 @@ function Example({triggerTop = 250, ...props}) { return (
Trigger
-
+
placement: {placement} @@ -36,6 +36,13 @@ function Example({triggerTop = 250, ...props}) { let original = window.HTMLElement.prototype.getBoundingClientRect; HTMLElement.prototype.getBoundingClientRect = function () { let rect = original.apply(this); + if (this.tagName === 'BODY') { + return { + ...rect, + height: this.clientHeight, + width: this.clientWidth + }; + } return { ...rect, left: parseInt(this.style.left, 10) || 0, @@ -49,6 +56,7 @@ HTMLElement.prototype.getBoundingClientRect = function () { describe('useOverlayPosition', function () { beforeEach(() => { + document.body.style.margin = '0'; // jsdom defaults to having a margin of 8px, we should fix this down the line Object.defineProperty(HTMLElement.prototype, 'clientHeight', {configurable: true, value: 768}); Object.defineProperty(HTMLElement.prototype, 'clientWidth', {configurable: true, value: 500}); @@ -226,6 +234,7 @@ describe('useOverlayPosition with positioned container', () => { let realGetBoundingClientRect = window.HTMLElement.prototype.getBoundingClientRect; let realGetComputedStyle = window.getComputedStyle; beforeEach(() => { + document.body.style.margin = '0'; Object.defineProperty(HTMLElement.prototype, 'clientHeight', {configurable: true, value: 768}); Object.defineProperty(HTMLElement.prototype, 'clientWidth', {configurable: true, value: 500}); stubs.push( @@ -238,19 +247,7 @@ describe('useOverlayPosition with positioned container', () => { } }), jest.spyOn(window.HTMLElement.prototype, 'getBoundingClientRect').mockImplementation(function (this: HTMLElement) { - if (this.attributes.getNamedItem('data-testid')?.value === 'container') { - // Say, overlay is positioned somewhere - let real = realGetBoundingClientRect.apply(this); - return { - ...real, - top: 150, - left: 0, - width: 400, - height: 400 - }; - } else { - return realGetBoundingClientRect.apply(this); - } + return realGetBoundingClientRect.apply(this); }), jest.spyOn(window, 'getComputedStyle').mockImplementation(element => { if (element.attributes.getNamedItem('data-testid')?.value === 'container') { @@ -260,6 +257,12 @@ describe('useOverlayPosition with positioned container', () => { } else { return realGetComputedStyle(element); } + }), + jest.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(function (this: HTMLElement) { + return parseInt(this.style.width, 10) || 0; + }), + jest.spyOn(HTMLElement.prototype, 'offsetHeight', 'get').mockImplementation(function (this: HTMLElement) { + return parseInt(this.style.height, 10) || 0; }) ); }); @@ -270,7 +273,7 @@ describe('useOverlayPosition with positioned container', () => { }); it('should position the overlay relative to the trigger', function () { - let res = render(); + let res = render(); let overlay = res.getByTestId('overlay'); let arrow = res.getByTestId('arrow'); @@ -281,7 +284,7 @@ describe('useOverlayPosition with positioned container', () => { z-index: 100000; left: 12px; top: 200px; - max-height: 556px; + max-height: 406px; `); expect(overlay).toHaveTextContent('placement: bottom'); @@ -291,7 +294,8 @@ describe('useOverlayPosition with positioned container', () => { }); it('should position the overlay relative to the trigger at top', function () { - let res = render(); + // body default has a margin of 8px, no idea where from + let res = render(); let overlay = res.getByTestId('overlay'); let arrow = res.getByTestId('arrow'); @@ -303,7 +307,7 @@ describe('useOverlayPosition with positioned container', () => { z-index: 100000; left: 12px; bottom: 300px; - max-height: 88px; + max-height: 238px; `); expect(overlay).toHaveTextContent('placement: top');