diff --git a/.storybook-v3/config.js b/.storybook-v3/config.js index c73c6ea4868..b48a3e2647d 100644 --- a/.storybook-v3/config.js +++ b/.storybook-v3/config.js @@ -24,8 +24,8 @@ addParameters({ addDecorator(withA11y); addDecorator(story => ( - -
{story()}
+ + {story()} )); diff --git a/.storybook-v3/custom-addons/provider/index.js b/.storybook-v3/custom-addons/provider/index.js index 9f4cc27d65b..daef46251e8 100644 --- a/.storybook-v3/custom-addons/provider/index.js +++ b/.storybook-v3/custom-addons/provider/index.js @@ -5,6 +5,8 @@ import {Provider} from '@react-spectrum/provider'; import {themes, defaultTheme} from '../../constants'; import {StatusLight} from '@react-spectrum/statuslight'; +document.body.style.margin = 0; + const providerValuesFromUrl = Object.entries(getQueryParams()).reduce((acc, [k, v]) => { if (k.includes('providerSwitcher-')) { return { ...acc, [k.replace('providerSwitcher-', '')]: v }; @@ -47,7 +49,7 @@ function ProviderUpdater(props) { return ( -
Status
{statusMap[props.options.status || 'negative']}
+
Status
{statusMap[props.options.status || 'negative']}
{storyReady && props.children}
); diff --git a/packages/@react-aria/collections/src/CollectionView.tsx b/packages/@react-aria/collections/src/CollectionView.tsx index 6cc2eb01958..23a13d78185 100644 --- a/packages/@react-aria/collections/src/CollectionView.tsx +++ b/packages/@react-aria/collections/src/CollectionView.tsx @@ -13,7 +13,7 @@ import {chain} from '@react-aria/utils'; import {Collection, Layout} from '@react-stately/collections'; import {CollectionItem} from './CollectionItem'; -import React, {FocusEvent, HTMLAttributes, Key, ReactElement, useCallback, useEffect, useRef} from 'react'; +import React, {FocusEvent, HTMLAttributes, Key, ReactElement, RefObject, useCallback, useEffect, useRef} from 'react'; import {ReusableView} from '@react-stately/collections'; import {ScrollView} from './ScrollView'; import {useCollectionState} from '@react-stately/collections'; @@ -32,7 +32,7 @@ interface CollectionViewProps extends HTMLAttributes(props: CollectionViewProps) { +function CollectionView(props: CollectionViewProps, ref: RefObject) { let {children: renderView, renderWrapper, layout, collection, focusedKey, sizeToFit, ...otherProps} = props; let { visibleViews, @@ -50,7 +50,8 @@ export function CollectionView(props: CollectionViewProps(); + let fallbackRef = useRef(); + ref = ref || fallbackRef; // Scroll to the focusedKey when it changes. Actually focusing the focusedKey // is up to the implementation using CollectionView since we don't have refs @@ -72,11 +73,11 @@ export function CollectionView(props: CollectionViewProps { isFocusWithin.current = ref.current.contains(e.relatedTarget as Element); - }, []); + }, [ref]); // When the focused item is scrolled out of view and is removed from the DOM, // move focus to the collection view as a whole if focus was within before. @@ -106,6 +107,11 @@ export function CollectionView(props: CollectionViewProps(props: CollectionViewProps & {ref?: RefObject}) => ReactElement; +export {_CollectionView as CollectionView}; + function defaultRenderWrapper( parent: ReusableView | null, reusableView: ReusableView diff --git a/packages/@react-aria/overlays/src/calculatePosition.ts b/packages/@react-aria/overlays/src/calculatePosition.ts index 92c72a410bf..20bddbb95bb 100644 --- a/packages/@react-aria/overlays/src/calculatePosition.ts +++ b/packages/@react-aria/overlays/src/calculatePosition.ts @@ -10,6 +10,7 @@ * governing permissions and limitations under the License. */ +import {Axis, Placement, PlacementAxis, SizeAxis} from '@react-types/overlays'; import getCss from 'dom-helpers/style'; import getOffset from 'dom-helpers/query/offset'; import getPosition from 'dom-helpers/query/position'; @@ -19,7 +20,53 @@ import ownerDocument from 'dom-helpers/ownerDocument'; interface Position { top?: number, - left?: number + left?: number, + bottom?: number, + right?: number +} + +interface Dimensions { + width: number, + height: number, + top: number, + left: number, + scroll: Position +} + +interface ParsedPlacement { + placement: PlacementAxis, + crossPlacement: PlacementAxis, + axis: Axis, + crossAxis: Axis, + size: SizeAxis, + crossSize: SizeAxis +} + +interface Offset { + top: number, + left: number, + width: number, + height: number +} + +interface PositionOpts { + placement: Placement, + targetNode: HTMLElement, + overlayNode: HTMLElement, + scrollNode: HTMLElement, + padding: number, + shouldFlip: boolean, + boundaryElement: HTMLElement, + offset: number, + crossOffset: number +} + +export interface PositionResult { + position?: Position, + arrowOffsetLeft?: number, + arrowOffsetTop?: number, + maxHeight?: number, + placement: PlacementAxis } const AXIS = { @@ -48,8 +95,8 @@ const AXIS_SIZE = { const PARSED_PLACEMENT_CACHE = {}; -function getContainerDimensions(containerNode) { - let width, height, top = 0, left = 0; +function getContainerDimensions(containerNode: Element): Dimensions { + let width = 0, height = 0, top = 0, left = 0; let scroll: Position = {}; if (containerNode.tagName === 'BODY') { @@ -71,12 +118,27 @@ function getContainerDimensions(containerNode) { return {width, height, scroll, top, left}; } -function getDelta(axis, offset, size, containerDimensions, padding) { - const containerScroll = containerDimensions.scroll[axis]; - const containerHeight = containerDimensions[AXIS_SIZE[axis]]; +function getScroll(node: HTMLElement): Offset { + return { + top: node.scrollTop, + left: node.scrollLeft, + width: node.scrollWidth, + height: node.scrollHeight + }; +} + +function getDelta( + axis: Axis, + offset: number, + size: number, + containerDimensions: Dimensions, + padding: number +) { + let containerScroll = containerDimensions.scroll[axis]; + let containerHeight = containerDimensions[AXIS_SIZE[axis]]; - const startEdgeOffset = offset - padding - containerScroll; - const endEdgeOffset = offset + padding - containerScroll + size; + let startEdgeOffset = offset - padding - containerScroll; + let endEdgeOffset = offset + padding - containerScroll + size; if (startEdgeOffset < 0) { return -startEdgeOffset; @@ -87,24 +149,8 @@ function getDelta(axis, offset, size, containerDimensions, padding) { } } -function shouldFlip(axis, offset, size, padding, placement, flipContainerDimensions, containerOffsetWithBoundary) { - const containerScroll = flipContainerDimensions.scroll[axis]; - const containerHeight = flipContainerDimensions[AXIS_SIZE[axis]]; - - const startEdgeOffset = containerOffsetWithBoundary[axis] + offset - padding - containerScroll; - const endEdgeOffset = containerOffsetWithBoundary[axis] + offset + padding - containerScroll + size; - - if (startEdgeOffset < 0 && (placement === 'top' || placement === 'left')) { - return true; - } else if (endEdgeOffset > containerHeight && (placement === 'bottom' || placement === 'right')) { - return true; - } else { - return false; - } -} - -function getMargins(node) { - const style = window.getComputedStyle(node); +function getMargins(node: HTMLElement): Position { + let style = window.getComputedStyle(node); return { top: parseInt(style.marginTop, 10) || 0, bottom: parseInt(style.marginBottom, 10) || 0, @@ -113,13 +159,14 @@ function getMargins(node) { }; } -function parsePlacement(input) { +function parsePlacement(input: Placement): ParsedPlacement { if (PARSED_PLACEMENT_CACHE[input]) { return PARSED_PLACEMENT_CACHE[input]; } + let [placement, crossPlacement] = input.split(' '); - let axis = AXIS[placement] || 'right'; - let crossAxis = CROSS_AXIS[axis]; + let axis: Axis = AXIS[placement] || 'right'; + let crossAxis: Axis = CROSS_AXIS[axis]; if (!AXIS[crossPlacement]) { crossPlacement = 'center'; @@ -131,8 +178,15 @@ function parsePlacement(input) { return PARSED_PLACEMENT_CACHE[input]; } -function computePosition(childOffset, containerDimensions, overlaySize, placementInfo, offset, crossOffset) { - const {placement, crossPlacement, axis, crossAxis, size, crossSize} = placementInfo; +function computePosition( + childOffset: Offset, + boundaryDimensions: Dimensions, + overlaySize: Offset, + placementInfo: ParsedPlacement, + offset: number, + crossOffset: number +) { + let {placement, crossPlacement, axis, crossAxis, size, crossSize} = placementInfo; let position: Position = {}; position[crossAxis] = childOffset[crossAxis] + crossOffset; @@ -144,12 +198,12 @@ function computePosition(childOffset, containerDimensions, overlaySize, placemen // Ensure overlay sticks to target(ignore for overlays smaller than target) if (childOffset[crossSize] < overlaySize[crossSize]) { - const positionForPositiveSideOverflow = Math.min(position[crossAxis], childOffset[crossAxis]); + let positionForPositiveSideOverflow = Math.min(position[crossAxis], childOffset[crossAxis]); position[crossAxis] = Math.max(positionForPositiveSideOverflow, childOffset[crossAxis] - overlaySize[crossSize] + childOffset[crossSize]); } if (placement === axis) { - position[axis] = childOffset[axis] - overlaySize[size] + offset; + position[FLIPPED_DIRECTION[axis]] = boundaryDimensions[size] - childOffset[axis] - offset; } else { position[axis] = childOffset[axis] + childOffset[size] + offset; } @@ -157,20 +211,76 @@ function computePosition(childOffset, containerDimensions, overlaySize, placemen return position; } -export function calculatePositionInternal(placementInput, containerDimensions, childOffset, overlaySize, margins, padding, flip, boundaryDimensions, containerOffsetWithBoundary, offset, crossOffset) { - let placementInfo = parsePlacement(placementInput); - const {axis, size, crossAxis, crossSize, placement, crossPlacement} = placementInfo; - let position = computePosition(childOffset, containerDimensions, overlaySize, placementInfo, offset, crossOffset); - let normalizedOffset = offset; +function getMaxHeight( + position: Position, + boundaryDimensions: Dimensions, + containerOffsetWithBoundary: Offset, + childOffset: Offset, + margins: Position, + padding: number +) { + return position.top != null + ? Math.max(0, boundaryDimensions.height + boundaryDimensions.top + boundaryDimensions.scroll.top + containerOffsetWithBoundary.top - position.top - margins.top - margins.bottom - padding) + : Math.max(0, childOffset.top - boundaryDimensions.top - boundaryDimensions.scroll.top - containerOffsetWithBoundary.top - margins.top - margins.bottom - padding); +} - // First check if placement should be flipped - if (flip && shouldFlip(axis, position[axis], overlaySize[size], padding, placement, boundaryDimensions, containerOffsetWithBoundary)) { - const flippedPlacementInfo = parsePlacement(`${FLIPPED_DIRECTION[placement]} ${crossPlacement}`); - const {axis, size} = flippedPlacementInfo; - const flippedPosition = computePosition(childOffset, containerDimensions, overlaySize, flippedPlacementInfo, -1 * offset, crossOffset); +function getAvailableSpace( + boundaryDimensions: Dimensions, + containerOffsetWithBoundary: Offset, + childOffset: Offset, + margins: Position, + padding: number, + placementInfo: ParsedPlacement +) { + let {placement, axis, size} = placementInfo; + if (placement === axis) { + return Math.max(0, childOffset[axis] - boundaryDimensions[axis] - boundaryDimensions.scroll[axis] + containerOffsetWithBoundary[axis] - margins[axis] - margins[FLIPPED_DIRECTION[axis]] - padding); + } + + return Math.max(0, boundaryDimensions[size] + boundaryDimensions[axis] + boundaryDimensions.scroll[axis] - containerOffsetWithBoundary[axis] - childOffset[axis] - childOffset[size] - margins[axis] - margins[FLIPPED_DIRECTION[axis]] - padding); +} - // Check if flipped placement has enough space otherwise flip is not possible - if (!shouldFlip(axis, flippedPosition[axis], overlaySize[size], padding, FLIPPED_DIRECTION[placement], boundaryDimensions, containerOffsetWithBoundary)) { +export function calculatePositionInternal( + placementInput: Placement, + childOffset: Offset, + overlaySize: Offset, + scrollSize: Offset, + margins: Position, + padding: number, + flip: boolean, + boundaryDimensions: Dimensions, + containerOffsetWithBoundary: Offset, + offset: number, + crossOffset: number +): PositionResult { + let placementInfo = parsePlacement(placementInput); + let {axis, size, crossAxis, crossSize, placement, crossPlacement} = placementInfo; + let position = computePosition(childOffset, boundaryDimensions, overlaySize, placementInfo, offset, crossOffset); + let normalizedOffset = offset; + let space = getAvailableSpace( + boundaryDimensions, + containerOffsetWithBoundary, + childOffset, + margins, + padding + offset, + placementInfo + ); + + // Check if the scroll size of the overlay is greater than the available space to determine if we need to flip + if (flip && scrollSize[size] > space) { + let flippedPlacementInfo = parsePlacement(`${FLIPPED_DIRECTION[placement]} ${crossPlacement}` as Placement); + let flippedPosition = computePosition(childOffset, boundaryDimensions, overlaySize, flippedPlacementInfo, -1 * offset, crossOffset); + let flippedSpace = getAvailableSpace( + boundaryDimensions, + containerOffsetWithBoundary, + childOffset, + margins, + padding + offset, + flippedPlacementInfo + ); + + // If the available space for the flipped position is greater than the original available space, flip. + if (flippedSpace > space) { placementInfo = flippedPlacementInfo; position = flippedPosition; normalizedOffset = -1 * offset; @@ -180,20 +290,26 @@ export function calculatePositionInternal(placementInput, containerDimensions, c let delta = getDelta(crossAxis, position[crossAxis], overlaySize[crossSize], boundaryDimensions, padding); position[crossAxis] += delta; - let maxHeight = Math.max(0, boundaryDimensions.height + boundaryDimensions.top + boundaryDimensions.scroll.top - containerOffsetWithBoundary.top - position.top - margins.top - margins.bottom - padding); - overlaySize.height = Math.min(overlaySize.height, maxHeight); + let maxHeight = getMaxHeight( + position, + boundaryDimensions, + containerOffsetWithBoundary, + childOffset, + margins, + padding + (axis === 'top' ? offset : 0) + ); - position = computePosition(childOffset, containerDimensions, overlaySize, placementInfo, normalizedOffset, crossOffset); - delta = delta = getDelta(crossAxis, position[crossAxis], overlaySize[crossSize], boundaryDimensions, padding); + overlaySize.height = Math.min(overlaySize.height, maxHeight); + position = computePosition(childOffset, boundaryDimensions, overlaySize, placementInfo, normalizedOffset, crossOffset); + delta = getDelta(crossAxis, position[crossAxis], overlaySize[crossSize], boundaryDimensions, padding); position[crossAxis] += delta; - const arrowPosition: Position = {}; + let arrowPosition: Position = {}; arrowPosition[crossAxis] = childOffset[crossSize] > overlaySize[crossSize] ? null : (childOffset[crossAxis] - position[crossAxis] + childOffset[crossSize] / 2); return { - positionLeft: position.left, - positionTop: position.top, + position, maxHeight: maxHeight, arrowOffsetLeft: arrowPosition.left, arrowOffsetTop: arrowPosition.top, @@ -201,23 +317,48 @@ export function calculatePositionInternal(placementInput, containerDimensions, c }; } -export function calculatePosition(placementInput, overlayNode, target, container, padding, flip, boundariesElement, offset, crossOffset) { - const isBodyContainer = container.tagName === 'BODY'; - const childOffset = isBodyContainer ? getOffset(target) : getPosition(target, container); +export function calculatePosition(opts: PositionOpts): PositionResult { + let { + placement, + targetNode, + overlayNode, + scrollNode, + padding, + shouldFlip, + boundaryElement, + offset, + crossOffset + } = opts; + + let container = overlayNode.offsetParent || document.body; + let isBodyContainer = container.tagName === 'BODY'; + let childOffset: Offset = isBodyContainer ? getOffset(targetNode) : getPosition(targetNode, container); if (!isBodyContainer) { - childOffset.top += parseInt(getCss(target, 'marginTop'), 10) || 0; - childOffset.left += parseInt(getCss(target, 'marginLeft'), 10) || 0; + childOffset.top += parseInt(getCss(targetNode, 'marginTop'), 10) || 0; + childOffset.left += parseInt(getCss(targetNode, 'marginLeft'), 10) || 0; } - const overlaySize = getOffset(overlayNode); - const margins = getMargins(overlayNode); + let overlaySize: Offset = getOffset(overlayNode); + let margins = getMargins(overlayNode); overlaySize.width += margins.left + margins.right; overlaySize.height += margins.top + margins.bottom; - const containerDimensions = getContainerDimensions(container); - const boundaryContainer = boundariesElement === 'container' ? container : boundariesElement; - const boundaryDimensions = getContainerDimensions(boundaryContainer); - const containerOffsetWithBoundary = boundaryContainer.tagName === 'BODY' ? getOffset(container) : getPosition(container, boundaryContainer); - return calculatePositionInternal(placementInput, containerDimensions, childOffset, overlaySize, margins, padding, flip, boundaryDimensions, containerOffsetWithBoundary, offset, crossOffset); + let scrollSize = getScroll(scrollNode); + let boundaryDimensions = getContainerDimensions(boundaryElement); + let containerOffsetWithBoundary: Offset = boundaryElement.tagName === 'BODY' ? getOffset(container) : getPosition(container, boundaryElement); + + return calculatePositionInternal( + placement, + childOffset, + overlaySize, + scrollSize, + margins, + padding, + shouldFlip, + boundaryDimensions, + containerOffsetWithBoundary, + offset, + crossOffset + ); } diff --git a/packages/@react-aria/overlays/src/useOverlayPosition.ts b/packages/@react-aria/overlays/src/useOverlayPosition.ts index 89ce75f3727..c2ac6afaf7b 100644 --- a/packages/@react-aria/overlays/src/useOverlayPosition.ts +++ b/packages/@react-aria/overlays/src/useOverlayPosition.ts @@ -10,8 +10,9 @@ * governing permissions and limitations under the License. */ -import {calculatePosition} from './calculatePosition'; +import {calculatePosition, PositionResult} from './calculatePosition'; import {HTMLAttributes, RefObject, useEffect, useState} from 'react'; +import {PlacementAxis} from '@react-types/overlays'; import {useLocale} from '@react-aria/i18n'; export type Placement = 'bottom' | 'bottom left' | 'bottom right' | 'bottom start' | 'bottom end' | @@ -25,38 +26,29 @@ export interface PositionProps { offset?: number, crossOffset?: number, shouldFlip?: boolean, - boundaryElement?: Element, + boundaryElement?: HTMLElement, isOpen?: boolean } interface AriaPositionProps extends PositionProps { - containerRef: RefObject, - targetRef: RefObject, - overlayRef: RefObject, + targetRef: RefObject, + overlayRef: RefObject, + scrollRef?: RefObject, shouldUpdatePosition?: boolean } interface PositionAria { overlayProps: HTMLAttributes, arrowProps: HTMLAttributes, - placement: Placement -} - -interface PositionState { - positionLeft?: number, - positionTop?: number, - arrowOffsetLeft?: number, - arrowOffsetTop?: number, - maxHeight?: number, - placement: Placement + placement: PlacementAxis } export function useOverlayPosition(props: AriaPositionProps): PositionAria { let {direction} = useLocale(); let { - containerRef, targetRef, overlayRef, + scrollRef = overlayRef, placement = 'bottom' as Placement, containerPadding = 8, shouldFlip = true, @@ -66,13 +58,12 @@ export function useOverlayPosition(props: AriaPositionProps): PositionAria { shouldUpdatePosition = true, isOpen = true } = props; - let [position, setPosition] = useState({ - positionLeft: 0, - positionTop: 0, + let [position, setPosition] = useState({ + position: {}, arrowOffsetLeft: undefined, arrowOffsetTop: undefined, maxHeight: undefined, - placement + placement: undefined }); let deps = [ @@ -80,7 +71,7 @@ export function useOverlayPosition(props: AriaPositionProps): PositionAria { placement, overlayRef.current, targetRef.current, - containerRef.current, + scrollRef.current, containerPadding, shouldFlip, boundaryElement, @@ -91,22 +82,22 @@ export function useOverlayPosition(props: AriaPositionProps): PositionAria { ]; let updatePosition = () => { - if (shouldUpdatePosition === false || !overlayRef.current || !targetRef.current || !containerRef.current) { + if (shouldUpdatePosition === false || !isOpen || !overlayRef.current || !targetRef.current || !scrollRef.current) { return; } setPosition( - calculatePosition( - translateRTL(placement, direction), - overlayRef.current, - targetRef.current, - containerRef.current, - containerPadding, + calculatePosition({ + placement: translateRTL(placement, direction), + overlayNode: overlayRef.current, + targetNode: targetRef.current, + scrollNode: scrollRef.current, + padding: containerPadding, shouldFlip, boundaryElement, offset, crossOffset - ) + }) ); }; @@ -121,8 +112,7 @@ export function useOverlayPosition(props: AriaPositionProps): PositionAria { style: { position: 'absolute', zIndex: 100000, // should match the z-index in ModalTrigger - left: position.positionLeft, - top: position.positionTop, + ...position.position, maxHeight: position.maxHeight } }, diff --git a/packages/@react-aria/overlays/src/useOverlayTrigger.ts b/packages/@react-aria/overlays/src/useOverlayTrigger.ts index e6bd55443bf..6c42bf4b642 100644 --- a/packages/@react-aria/overlays/src/useOverlayTrigger.ts +++ b/packages/@react-aria/overlays/src/useOverlayTrigger.ts @@ -38,7 +38,7 @@ export function useOverlayTrigger(props: OverlayTriggerProps): OverlayTriggerAri let onScroll = (e: MouseEvent) => { // Ignore if scrolling an scrollable region outside the trigger's tree. let target = e.target as HTMLElement; - if (target === document.body || !ref.current || !target.contains(ref.current)) { + if (!ref.current || !target.contains(ref.current)) { return; } @@ -47,9 +47,9 @@ export function useOverlayTrigger(props: OverlayTriggerProps): OverlayTriggerAri } }; - document.body.addEventListener('scroll', onScroll, true); + window.addEventListener('scroll', onScroll, true); return () => { - document.body.removeEventListener('scroll', onScroll, true); + window.removeEventListener('scroll', onScroll, true); }; }, [isOpen, onClose, ref]); diff --git a/packages/@react-aria/overlays/test/calculatePosition.test.js b/packages/@react-aria/overlays/test/calculatePosition.test.js index 35c07f360d9..6c7cc276c5d 100644 --- a/packages/@react-aria/overlays/test/calculatePosition.test.js +++ b/packages/@react-aria/overlays/test/calculatePosition.test.js @@ -37,6 +37,9 @@ const containerDimensions = { left: 0 }; +// Reset JSDom's default margin on the body +document.body.style.margin = 0; + function createElementWithDimensions(elemName, dimensions, margins) { margins = margins || {}; let elem = document.createElement(elemName); @@ -64,6 +67,9 @@ function createElementWithDimensions(elemName, dimensions, margins) { bottom: dimensions.bottom || 0 }); + jest.spyOn(elem, 'scrollWidth', 'get').mockImplementation(() => dimensions.width || 0); + jest.spyOn(elem, 'scrollHeight', 'get').mockImplementation(() => dimensions.height || 0); + return elem; } @@ -95,12 +101,28 @@ const PROVIDER_OFFSET = 50; describe('calculatePosition', function () { function checkPositionCommon(title, expected, placement, targetDimension, boundaryDimensions, offset, crossOffset, flip, providerOffset = 0) { const placementAxis = placement.split(' ')[0]; + + // The tests are all based on top/left positioning. Convert to bottom/right positioning if needed. + let pos = {}; + if ((placementAxis === 'left' && !flip) || (placementAxis === 'right' && flip)) { + pos.right = boundaryDimensions.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); + } else if (placementAxis === 'bottom') { + pos.left = expected[0]; + pos.top = expected[1]; + } + const expectedPosition = { - positionLeft: expected[0], - positionTop: expected[1], + position: pos, arrowOffsetLeft: expected[2], arrowOffsetTop: expected[3], - maxHeight: expected[4] - providerOffset, + maxHeight: expected[4] - (placementAxis !== 'top' ? providerOffset : 0), placement: flip ? FLIPPED_DIRECTION[placementAxis] : placementAxis }; @@ -121,8 +143,19 @@ describe('calculatePosition', function () { }); parentElement.appendChild(boundariesElem); + it(title, function () { - const result = calculatePosition(placement, overlay, target, container, 50, flip, boundariesElem, offset, crossOffset); + const result = calculatePosition({ + placement, + overlayNode: overlay, + targetNode: target, + scrollNode: overlay, + padding: 50, + shouldFlip: flip, + boundaryElement: boundariesElem, + offset, + crossOffset + }); expect(result).toEqual(expectedPosition); document.documentElement.removeChild(parentElement); }); @@ -182,34 +215,34 @@ describe('calculatePosition', function () { }, { placement: 'top', - noOffset: [200, 50, 100, undefined, 500], - offsetBefore: [50, -200, 0, undefined, 750], - offsetAfter: [350, 300, 200, undefined, 250], - mainAxisOffset: [200, 60, 100, undefined, 490], - crossAxisOffset: [210, 50, 90, undefined, 500] + noOffset: [200, 50, 100, undefined, 200], + offsetBefore: [50, -200, 0, undefined, 0], + offsetAfter: [350, 300, 200, undefined, 450], + mainAxisOffset: [200, 60, 100, undefined, 190], + crossAxisOffset: [210, 50, 90, undefined, 200] }, { placement: 'top left', - noOffset: [250, 50, 50, undefined, 500], - offsetBefore: [50, -200, 0, undefined, 750], - offsetAfter: [350, 300, 200, undefined, 250], - mainAxisOffset: [250, 60, 50, undefined, 490], - crossAxisOffset: [250, 50, 50, undefined, 500] + noOffset: [250, 50, 50, undefined, 200], + offsetBefore: [50, -200, 0, undefined, 0], + offsetAfter: [350, 300, 200, undefined, 450], + mainAxisOffset: [250, 60, 50, undefined, 190], + crossAxisOffset: [250, 50, 50, undefined, 200] }, { placement: 'top right', - noOffset: [150, 50, 150, undefined, 500], - offsetBefore: [50, -200, 0, undefined, 750], - offsetAfter: [350, 300, 200, undefined, 250], - mainAxisOffset: [150, 60, 150, undefined, 490], - crossAxisOffset: [160, 50, 140, undefined, 500] + noOffset: [150, 50, 150, undefined, 200], + offsetBefore: [50, -200, 0, undefined, 0], + offsetAfter: [350, 300, 200, undefined, 450], + mainAxisOffset: [150, 60, 150, undefined, 190], + crossAxisOffset: [160, 50, 140, undefined, 200] }, { placement: 'bottom', noOffset: [200, 350, 100, undefined, 200], offsetBefore: [50, 100, 0, undefined, 450], offsetAfter: [350, 600, 200, undefined, 0], - mainAxisOffset: [200, 360, 100, undefined, 190], + mainAxisOffset: [200, 360, 100, undefined, 180], crossAxisOffset: [210, 350, 90, undefined, 200] }, { @@ -217,7 +250,7 @@ describe('calculatePosition', function () { noOffset: [250, 350, 50, undefined, 200], offsetBefore: [50, 100, 0, undefined, 450], offsetAfter: [350, 600, 200, undefined, 0], - mainAxisOffset: [250, 360, 50, undefined, 190], + mainAxisOffset: [250, 360, 50, undefined, 180], crossAxisOffset: [250, 350, 50, undefined, 200] }, { @@ -225,7 +258,7 @@ describe('calculatePosition', function () { noOffset: [150, 350, 150, undefined, 200], offsetBefore: [50, 100, 0, undefined, 450], offsetAfter: [350, 600, 200, undefined, 0], - mainAxisOffset: [150, 360, 150, undefined, 190], + mainAxisOffset: [150, 360, 150, undefined, 180], crossAxisOffset: [160, 350, 140, undefined, 200] }, { @@ -318,7 +351,17 @@ describe('calculatePosition', function () { target.style = 'margin:20px'; document.body.appendChild(target); - const {positionTop} = calculatePosition('bottom', overlayNode, target, container, 0, false, 'container', 0, 0); + let {position: {top: positionTop}} = calculatePosition({ + placement: 'bottom', + overlayNode, + targetNode: target, + scrollNode: overlayNode, + padding: 0, + shouldFlip: false, + boundaryElement: container, + offset: 0, + crossOffset: 0 + }); expect(positionTop).toBe(0); document.body.removeChild(target); diff --git a/packages/@react-aria/overlays/test/useOverlayPosition.test.js b/packages/@react-aria/overlays/test/useOverlayPosition.test.js index 8aed3fa2e66..22d877ddff2 100644 --- a/packages/@react-aria/overlays/test/useOverlayPosition.test.js +++ b/packages/@react-aria/overlays/test/useOverlayPosition.test.js @@ -70,11 +70,11 @@ describe('useOverlayPosition', function () { expect(overlay).toHaveStyle(` left: 8px; - top: 100px; - max-height: 660px; + top: 500px; + max-height: 260px; `); - expect(overlay).toHaveTextContent('placement: top'); + expect(overlay).toHaveTextContent('placement: bottom'); let innerHeight = window.innerHeight; window.innerHeight = 1000; @@ -105,7 +105,7 @@ describe('useOverlayPosition', function () { expect(overlay).toHaveStyle(` left: 8px; top: 370px; - max-height: 390px; + max-height: 370px; `); }); }); diff --git a/packages/@react-aria/overlays/test/useOverlayTrigger.test.js b/packages/@react-aria/overlays/test/useOverlayTrigger.test.js index 7d51286645e..aa7b04d6338 100644 --- a/packages/@react-aria/overlays/test/useOverlayTrigger.test.js +++ b/packages/@react-aria/overlays/test/useOverlayTrigger.test.js @@ -50,11 +50,11 @@ describe('useOverlayTrigger', function () { expect(onClose).not.toHaveBeenCalled(); }); - it('should not close the overlay when the body scrolls', function () { + it('should close the overlay when the body scrolls', function () { let onClose = jest.fn(); render(); fireEvent.scroll(document.body); - expect(onClose).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/@react-spectrum/dialog/src/Dialog.tsx b/packages/@react-spectrum/dialog/src/Dialog.tsx index 2f0d8be5b32..0e8d0b1b553 100644 --- a/packages/@react-spectrum/dialog/src/Dialog.tsx +++ b/packages/@react-spectrum/dialog/src/Dialog.tsx @@ -54,7 +54,7 @@ export function Dialog(props: SpectrumDialogProps) { let formatMessage = useMessageFormatter(intlMessages); let {styleProps} = useStyleProps(otherProps); - size = type === 'popover' ? undefined : (size || 'L'); + size = type === 'popover' ? 'S' : (size || 'L'); if (type === 'fullscreen' || type === 'fullscreenTakeover') { size = type; } diff --git a/packages/@react-spectrum/dialog/src/DialogTrigger.tsx b/packages/@react-spectrum/dialog/src/DialogTrigger.tsx index f042704cce1..98625451134 100644 --- a/packages/@react-spectrum/dialog/src/DialogTrigger.tsx +++ b/packages/@react-spectrum/dialog/src/DialogTrigger.tsx @@ -11,13 +11,12 @@ */ import {DialogContext} from './context'; -import {DOMRefValue} from '@react-types/shared'; import {Modal, Overlay, Popover, Tray} from '@react-spectrum/overlays'; import {PressResponder} from '@react-aria/interactions'; import React, {Fragment, ReactElement, useRef} from 'react'; import {SpectrumDialogClose, SpectrumDialogProps, SpectrumDialogTriggerProps} from '@react-types/dialog'; -import {unwrapDOMRef, useMediaQuery} from '@react-spectrum/utils'; import {useControlledState} from '@react-stately/utils'; +import {useMediaQuery} from '@react-spectrum/utils'; import {useOverlayPosition, useOverlayTrigger} from '@react-aria/overlays'; export function DialogTrigger(props: SpectrumDialogTriggerProps) { @@ -122,11 +121,9 @@ DialogTrigger.getCollectionNode = function (props: SpectrumDialogTriggerProps) { }; function PopoverTrigger({isOpen, onPress, onClose, targetRef, trigger, content, hideArrow, ...props}) { - let containerRef = useRef>(); let triggerRef = useRef(); let overlayRef = useRef(); let {overlayProps, placement, arrowProps} = useOverlayPosition({ - containerRef: unwrapDOMRef(containerRef), targetRef: targetRef || triggerRef, overlayRef, placement: props.placement, @@ -150,7 +147,7 @@ function PopoverTrigger({isOpen, onPress, onClose, targetRef, trigger, content, }; let overlay = ( - + {content} diff --git a/packages/@react-spectrum/listbox/src/ListBox.tsx b/packages/@react-spectrum/listbox/src/ListBox.tsx index a44647af23c..d9fe962db95 100644 --- a/packages/@react-spectrum/listbox/src/ListBox.tsx +++ b/packages/@react-spectrum/listbox/src/ListBox.tsx @@ -10,22 +10,31 @@ * governing permissions and limitations under the License. */ +import {DOMRef} from '@react-types/shared'; import {ListBoxBase, useListBoxLayout} from './ListBoxBase'; -import React from 'react'; +import React, {ReactElement} from 'react'; import {SpectrumMenuProps} from '@react-types/menu'; +import {useDOMRef} from '@react-spectrum/utils'; import {useListState} from '@react-stately/list'; -export function ListBox(props: SpectrumMenuProps) { +function ListBox(props: SpectrumMenuProps, ref: DOMRef) { let state = useListState({ ...props, selectionMode: props.selectionMode || 'single' }); let layout = useListBoxLayout(state); + let domRef = useDOMRef(ref); return ( ); } + +// forwardRef doesn't support generic parameters, so cast the result to the correct type +// https://stackoverflow.com/questions/58469229/react-with-typescript-generics-while-using-react-forwardref +const _ListBox = React.forwardRef(ListBox) as (props: SpectrumMenuProps & {ref?: DOMRef}) => ReactElement; +export {_ListBox as ListBox}; diff --git a/packages/@react-spectrum/listbox/src/ListBoxBase.tsx b/packages/@react-spectrum/listbox/src/ListBoxBase.tsx index 94cc4128733..99e3201bba4 100644 --- a/packages/@react-spectrum/listbox/src/ListBoxBase.tsx +++ b/packages/@react-spectrum/listbox/src/ListBoxBase.tsx @@ -19,7 +19,7 @@ import {ListBoxSection} from './ListBoxSection'; import {ListLayout, Node} from '@react-stately/collections'; import {ListState} from '@react-stately/list'; import {mergeProps} from '@react-aria/utils'; -import React, {HTMLAttributes, ReactElement, useMemo} from 'react'; +import React, {HTMLAttributes, ReactElement, RefObject, useMemo} from 'react'; import {ReusableView} from '@react-stately/collections'; import styles from '@adobe/spectrum-css-temp/components/menu/vars.css'; import {useCollator} from '@react-aria/i18n'; @@ -53,7 +53,7 @@ export function useListBoxLayout(state: ListState) { return layout; } -export function ListBoxBase(props: ListBoxBaseProps) { +function ListBoxBase(props: ListBoxBaseProps, ref: RefObject) { let {layout, state, selectOnPressUp, focusOnPointerEnter, domProps = {}} = props; let {listBoxProps} = useListBox({ ...props, @@ -92,6 +92,7 @@ export function ListBoxBase(props: ListBoxBaseProps) { {...filterDOMProps(props)} {...styleProps} {...mergeProps(listBoxProps, domProps)} + ref={ref} focusedKey={state.selectionManager.focusedKey} sizeToFit="height" className={ @@ -118,3 +119,8 @@ export function ListBoxBase(props: ListBoxBaseProps) { ); } + +// forwardRef doesn't support generic parameters, so cast the result to the correct type +// https://stackoverflow.com/questions/58469229/react-with-typescript-generics-while-using-react-forwardref +const _ListBoxBase = React.forwardRef(ListBoxBase) as (props: ListBoxBaseProps & {ref?: RefObject}) => ReactElement; +export {_ListBoxBase as ListBoxBase}; diff --git a/packages/@react-spectrum/menu/src/Menu.tsx b/packages/@react-spectrum/menu/src/Menu.tsx index 3ff7ac2db20..c684f5b7116 100644 --- a/packages/@react-spectrum/menu/src/Menu.tsx +++ b/packages/@react-spectrum/menu/src/Menu.tsx @@ -10,36 +10,47 @@ * governing permissions and limitations under the License. */ -import {classNames, DOMEventPropNames, filterDOMProps, useStyleProps} from '@react-spectrum/utils'; +import {classNames, DOMEventPropNames, filterDOMProps, useDOMRef, useStyleProps} from '@react-spectrum/utils'; +import {DOMRef} from '@react-types/shared'; import {MenuContext} from './context'; import {MenuItem} from './MenuItem'; import {MenuSection} from './MenuSection'; import {mergeProps} from '@react-aria/utils'; -import React, {useContext, useRef} from 'react'; +import React, {ReactElement, useContext, useEffect} from 'react'; import {SpectrumMenuProps} from '@react-types/menu'; import styles from '@adobe/spectrum-css-temp/components/menu/vars.css'; import {useMenu} from '@react-aria/menu'; import {useTreeState} from '@react-stately/tree'; -export function Menu(props: SpectrumMenuProps) { +function Menu(props: SpectrumMenuProps, ref: DOMRef) { let contextProps = useContext(MenuContext); let completeProps = { ...mergeProps(contextProps, props), selectionMode: props.selectionMode || 'none' }; - let ref = useRef(); + let domRef = useDOMRef(ref); let state = useTreeState(completeProps); - let {menuProps} = useMenu({...completeProps, ref}, state); + let {menuProps} = useMenu({...completeProps, ref: domRef}, state); let {styleProps} = useStyleProps(completeProps); + // Sync ref from context with DOM ref. + useEffect(() => { + if (contextProps && contextProps.ref) { + contextProps.ref.current = domRef.current; + return () => { + contextProps.ref.current = null; + }; + } + }, [contextProps, domRef]); + return (
    (props: SpectrumMenuProps) {
); } + +// forwardRef doesn't support generic parameters, so cast the result to the correct type +// https://stackoverflow.com/questions/58469229/react-with-typescript-generics-while-using-react-forwardref +const _Menu = React.forwardRef(Menu) as (props: SpectrumMenuProps & {ref?: DOMRef}) => ReactElement; +export {_Menu as Menu}; diff --git a/packages/@react-spectrum/menu/src/MenuTrigger.tsx b/packages/@react-spectrum/menu/src/MenuTrigger.tsx index dd909857d4c..3c604f91da8 100644 --- a/packages/@react-spectrum/menu/src/MenuTrigger.tsx +++ b/packages/@react-spectrum/menu/src/MenuTrigger.tsx @@ -10,7 +10,6 @@ * governing permissions and limitations under the License. */ -import {DOMRefValue} from '@react-types/shared'; import {FocusScope} from '@react-aria/focus'; import {FocusStrategy, SpectrumMenuTriggerProps} from '@react-types/menu'; import {MenuContext} from './context'; @@ -19,14 +18,14 @@ import {Placement, useOverlayPosition} from '@react-aria/overlays'; import {PressResponder} from '@react-aria/interactions'; import {Provider} from '@react-spectrum/provider'; import React, {Fragment, useRef, useState} from 'react'; -import {unwrapDOMRef, useMediaQuery} from '@react-spectrum/utils'; import {useControlledState} from '@react-stately/utils'; +import {useMediaQuery} from '@react-spectrum/utils'; import {useMenuTrigger} from '@react-aria/menu'; export function MenuTrigger(props: SpectrumMenuTriggerProps) { - let containerRef = useRef>(); let menuPopoverRef = useRef(); let menuTriggerRef = useRef(); + let menuRef = useRef(); let { children, onOpenChange, @@ -59,9 +58,9 @@ export function MenuTrigger(props: SpectrumMenuTriggerProps) { ); let {overlayProps, placement} = useOverlayPosition({ - containerRef: unwrapDOMRef(containerRef), targetRef: menuTriggerRef, overlayRef: menuPopoverRef, + scrollRef: menuRef, placement: `${direction} ${align}` as Placement, shouldFlip: shouldFlip, isOpen @@ -70,6 +69,7 @@ export function MenuTrigger(props: SpectrumMenuTriggerProps) { let isMobile = useMediaQuery('(max-width: 700px)'); let menuContext = { ...menuProps, + ref: menuRef, focusStrategy, onClose, closeOnSelect, @@ -84,7 +84,7 @@ export function MenuTrigger(props: SpectrumMenuTriggerProps) { let overlay; if (isMobile) { overlay = ( - setOpen(false)}> + {menu} @@ -97,7 +97,7 @@ export function MenuTrigger(props: SpectrumMenuTriggerProps) { ref={menuPopoverRef} placement={placement} hideArrow - onClose={() => setOpen(false)}> + onClose={onClose}> {menu} @@ -113,7 +113,7 @@ export function MenuTrigger(props: SpectrumMenuTriggerProps) { - + {overlay} diff --git a/packages/@react-spectrum/menu/src/context.ts b/packages/@react-spectrum/menu/src/context.ts index 72a54a03194..9aca531d8cb 100644 --- a/packages/@react-spectrum/menu/src/context.ts +++ b/packages/@react-spectrum/menu/src/context.ts @@ -11,14 +11,15 @@ */ import {FocusStrategy} from '@react-types/menu'; -import React, {HTMLAttributes, useContext} from 'react'; +import React, {HTMLAttributes, MutableRefObject, useContext} from 'react'; export interface MenuContextValue extends HTMLAttributes { onClose?: () => void, closeOnSelect?: boolean, focusStrategy?: FocusStrategy, wrapAround?: boolean, - autoFocus?: boolean + autoFocus?: boolean, + ref?: MutableRefObject } export const MenuContext = React.createContext({}); diff --git a/packages/@react-spectrum/menu/stories/Menu.stories.tsx b/packages/@react-spectrum/menu/stories/Menu.stories.tsx index 8d157d2422d..6cbe4f56941 100644 --- a/packages/@react-spectrum/menu/stories/Menu.stories.tsx +++ b/packages/@react-spectrum/menu/stories/Menu.stories.tsx @@ -145,7 +145,7 @@ storiesOf('Menu', module)
- + Edit...
diff --git a/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx b/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx index 302fc3a209a..29bbe53996e 100644 --- a/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx +++ b/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx @@ -51,11 +51,11 @@ storiesOf('MenuTrigger', module) ) .add( 'single selected key (controlled)', - () => render({}, {selectedKeys: ['Kangaroo']}) + () => render({}, {selectedKeys: ['Kangaroo'], selectionMode: 'single'}) ) .add( 'single default selected key (uncontrolled)', - () => render({}, {defaultSelectedKeys: ['Kangaroo']}) + () => render({}, {defaultSelectedKeys: ['Kangaroo'], selectionMode: 'single'}) ) .add( 'multiple selected key (controlled)', @@ -65,10 +65,6 @@ storiesOf('MenuTrigger', module) 'multiple default selected key (uncontrolled)', () => render({}, {defaultSelectedKeys: ['Kangaroo', 'Devon'], selectionMode: 'multiple'}) ) - .add( - 'autofocus=false', - () => render({}, {defaultSelectedKeys: ['Kangaroo', 'Devon'], selectionMode: 'multiple', autoFocus: false}) - ) .add( 'align="end"', () => render({align: 'end'}) diff --git a/packages/@react-spectrum/overlays/src/Overlay.tsx b/packages/@react-spectrum/overlays/src/Overlay.tsx index f95f08993c7..67e38aa6ebe 100644 --- a/packages/@react-spectrum/overlays/src/Overlay.tsx +++ b/packages/@react-spectrum/overlays/src/Overlay.tsx @@ -54,7 +54,7 @@ function Overlay(props: OverlayProps, ref: DOMRef) { } let contents = ( - + { children: ReactNode, - placement?: Placement, + placement?: PlacementAxis, arrowProps?: HTMLAttributes, hideArrow?: boolean, isOpen?: boolean, @@ -50,7 +50,7 @@ function Popover(props: PopoverProps, ref: RefObject) { classNames( styles, 'spectrum-Popover', - `spectrum-Popover--${placement.split(' ')[0]}`, + `spectrum-Popover--${placement}`, { 'spectrum-Popover--withTip': !hideArrow, 'is-open': isOpen diff --git a/packages/@react-spectrum/overlays/test/Overlay.test.js b/packages/@react-spectrum/overlays/test/Overlay.test.js index f0818509c62..e15d875e81a 100644 --- a/packages/@react-spectrum/overlays/test/Overlay.test.js +++ b/packages/@react-spectrum/overlays/test/Overlay.test.js @@ -56,6 +56,5 @@ describe('Overlay', function () { let overlayNode = overlayRef.current.UNSAFE_getDOMNode(); expect(overlayNode).not.toBe(providerRef.current); expect(overlayNode.parentNode).toBe(document.body); - expect(overlayNode).toHaveStyle('position: absolute; z-index: 100000'); }); }); diff --git a/packages/@react-spectrum/picker/src/Picker.tsx b/packages/@react-spectrum/picker/src/Picker.tsx index fef1b27b56b..0ee8cc72429 100644 --- a/packages/@react-spectrum/picker/src/Picker.tsx +++ b/packages/@react-spectrum/picker/src/Picker.tsx @@ -60,6 +60,7 @@ function Picker(props: SpectrumPickerProps, ref: DOMRef) { let containerRef = useRef>(); let popoverRef = useRef(); let triggerRef = useRef>(); + let listboxRef = useRef(); // We create the listbox layout in Picker and pass it to ListBoxBase below // so that the layout information can be cached even while the listbox is not mounted. @@ -72,9 +73,9 @@ function Picker(props: SpectrumPickerProps, ref: DOMRef) { }, state); let {overlayProps, placement} = useOverlayPosition({ - containerRef: unwrapDOMRef(containerRef), targetRef: unwrapDOMRef(triggerRef), overlayRef: popoverRef, + scrollRef: listboxRef, placement: `${direction} ${align}` as Placement, shouldFlip: shouldFlip, isOpen: state.isOpen @@ -85,6 +86,7 @@ function Picker(props: SpectrumPickerProps, ref: DOMRef) { let listbox = ( >(); let triggerRef = useRef(); let overlayRef = useRef(); @@ -50,16 +47,13 @@ export function TooltipTrigger(props: TooltipTriggerProps) { let {overlayProps, placement, arrowProps} = useOverlayPosition({ placement: props.placement, - containerRef: unwrapDOMRef(containerRef), targetRef: targetRef || triggerRef, overlayRef, isOpen }); - delete overlayProps.style.position; - let overlay = ( - + {React.cloneElement(content, {placement: placement, arrowProps: arrowProps, ref: overlayRef, UNSAFE_style: overlayProps.style, isOpen: open, ...tooltipProps})} ); diff --git a/packages/@react-types/overlays/src/index.d.ts b/packages/@react-types/overlays/src/index.d.ts index 5f3545fb05a..5e7db5fa594 100644 --- a/packages/@react-types/overlays/src/index.d.ts +++ b/packages/@react-types/overlays/src/index.d.ts @@ -15,6 +15,10 @@ export type Placement = 'bottom' | 'bottom left' | 'bottom right' | 'bottom star 'left' | 'left top' | 'left bottom' | 'start' | 'start top' | 'start bottom' | 'right' | 'right top' | 'right bottom' | 'end' | 'end top' | 'end bottom'; +export type Axis = 'top' | 'bottom' | 'left' | 'right'; +export type SizeAxis = 'width' | 'height'; +export type PlacementAxis = Axis | 'center'; + export interface PositionProps { placement?: Placement, containerPadding?: number,