Skip to content

Commit

Permalink
fix(Popover): arrow offset when scrolling offscreen (#5287)
Browse files Browse the repository at this point in the history
* fix(Popover): arrow offset when scrolling offscreen

* bump ro
  • Loading branch information
jquense committed Jul 10, 2020
1 parent 2892beb commit 0e86a51
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 27 deletions.
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -76,7 +76,7 @@
"invariant": "^2.2.4",
"prop-types": "^15.7.2",
"prop-types-extra": "^1.1.0",
"react-overlays": "^3.2.0",
"react-overlays": "^4.0.0",
"react-transition-group": "^4.0.0",
"uncontrollable": "^7.0.0",
"warning": "^4.0.3"
Expand Down
7 changes: 4 additions & 3 deletions src/DropdownMenu.tsx
Expand Up @@ -4,6 +4,7 @@ import React, { useContext } from 'react';
import {
useDropdownMenu,
UseDropdownMenuValue,
UseDropdownMenuOptions,
} from 'react-overlays/DropdownMenu';
import useMergedRefs from '@restart/hooks/useMergedRefs';
import NavbarContext from './NavbarContext';
Expand All @@ -23,7 +24,7 @@ export interface DropdownMenuProps extends BsPrefixPropsWithChildren {
alignRight?: boolean;
onSelect?: SelectCallback;
rootCloseEvent?: 'click' | 'mousedown';
popperConfig?: { modifiers?: any };
popperConfig?: UseDropdownMenuOptions['popperConfig'];
}

type DropdownMenu = BsPrefixRefForwardingComponent<'div', DropdownMenuProps>;
Expand Down Expand Up @@ -92,7 +93,7 @@ const DropdownMenu: DropdownMenu = React.forwardRef(
renderOnMount,
// Need to define the default "as" during prop destructuring to be compatible with styled-components github.com/react-bootstrap/react-bootstrap/issues/3595
as: Component = 'div',
popperConfig = {},
popperConfig,
...props
}: DropdownMenuProps,
ref,
Expand All @@ -116,7 +117,7 @@ const DropdownMenu: DropdownMenu = React.forwardRef(
usePopper: !isNavbar,
popperConfig: {
...popperConfig,
modifiers: marginModifiers.concat(popperConfig.modifiers || []),
modifiers: marginModifiers.concat(popperConfig?.modifiers || []),
},
}) as UseDropdownMenuValueHack;

Expand Down
12 changes: 6 additions & 6 deletions src/Overlay.tsx
Expand Up @@ -182,19 +182,19 @@ function Overlay({
props: overlayProps,
arrowProps,
show,
// @ts-ignore
state,
scheduleUpdate,
update,
forceUpdate: _,
placement,
outOfBoundaries,
state,
...props
}) => {
wrapRefs(overlayProps, arrowProps);
const popper = Object.assign(popperRef.current, {
state,
scheduleUpdate,
scheduleUpdate: update,
placement,
outOfBoundaries,
outOfBoundaries:
state?.modifiersData.hide?.isReferenceHidden || false,
});

if (typeof overlay === 'function')
Expand Down
14 changes: 9 additions & 5 deletions src/OverlayTrigger.tsx
Expand Up @@ -5,6 +5,7 @@ import useTimeout from '@restart/hooks/useTimeout';
import safeFindDOMNode from 'react-overlays/safeFindDOMNode';
import warning from 'warning';
import { useUncontrolledProp } from 'uncontrollable';
import { Modifier } from 'react-overlays/esm/usePopper';
import Overlay, { OverlayChildren, OverlayProps } from './Overlay';

export type OverlayTriggerType = 'hover' | 'click' | 'focus';
Expand Down Expand Up @@ -264,13 +265,14 @@ function OverlayTrigger({

// We add aria-describedby in the case where the overlay is a role="tooltip"
// for other cases describedby isn't appropriate (e.g. a popover with inputs) so we don't add it.
const ariaModifier = {
const ariaModifier: Modifier<'ariaDescribedBy', {}> = {
name: 'ariaDescribedBy',
enabled: true,
phase: 'afterWrite',
effect: ({ state }) => {
return () => {
state.elements.reference.removeAttribute('aria-describedby');
if ('removeAttribute' in state.elements.reference)
state.elements.reference.removeAttribute('aria-describedby');
};
},
fn: ({ state }) => {
Expand All @@ -279,7 +281,11 @@ function OverlayTrigger({
if (!show || !reference) return;

const role = popper.getAttribute('role') || '';
if (popper.id && role.toLowerCase() === 'tooltip') {
if (
popper.id &&
role.toLowerCase() === 'tooltip' &&
'setAttribute' in reference
) {
reference.setAttribute('aria-describedby', popper.id);
}
},
Expand Down Expand Up @@ -318,8 +324,6 @@ function OverlayTrigger({
{...props}
popperConfig={{
...popperConfig,
// TODO: fix typing
// @ts-ignore
modifiers,
}}
show={show}
Expand Down
7 changes: 1 addition & 6 deletions src/Popover.tsx
Expand Up @@ -109,12 +109,7 @@ const Popover: Popover = (React.forwardRef<HTMLDivElement, PopoverProps>(
)}
{...props}
>
<div
className="arrow"
{...arrowProps}
// this prevents an error if you render a Popover without arrow props, like in a test
style={arrowProps ? { ...arrowProps.style, margin: 0 } : undefined}
/>
<div className="arrow" {...arrowProps} />
{content ? <PopoverContent>{children}</PopoverContent> : children}
</div>
);
Expand Down
39 changes: 37 additions & 2 deletions src/usePopperMarginModifiers.tsx
Expand Up @@ -37,7 +37,7 @@ export default function usePopperMarginModifiers(): [
overlayRef.current = overlay;
}, []);

const modifier = useMemo(() => {
const offset = useMemo(() => {
return {
name: 'offset',
options: {
Expand All @@ -61,5 +61,40 @@ export default function usePopperMarginModifiers(): [
},
};
}, [margins]);
return [callback, [modifier]];

// Converts popover arrow margin to arrow modifier padding
const popoverArrowMargins = useMemo(() => {
return {
name: 'popoverArrowMargins',
enabled: true,
phase: 'main',
requiresIfExists: ['arrow'],
effect({ state }) {
if (
!overlayRef.current ||
!state.elements.arrow ||
!hasClass(overlayRef.current, 'popover') ||
!state.modifiersData['arrow#persistent']
) {
return undefined;
}

const { top, right } = getMargins(state.elements.arrow);
const padding = top || right;
state.modifiersData['arrow#persistent'].padding = {
top: padding,
left: padding,
right: padding,
bottom: padding,
};
state.elements.arrow.style.margin = '0';

return () => {
if (state.elements.arrow) state.elements.arrow.style.margin = '';
};
},
};
}, []);

return [callback, [offset, popoverArrowMargins]];
}
8 changes: 4 additions & 4 deletions yarn.lock
Expand Up @@ -7644,10 +7644,10 @@ react-lifecycles-compat@^3.0.4:
resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362"
integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA==

react-overlays@^3.2.0:
version "3.2.0"
resolved "https://registry.yarnpkg.com/react-overlays/-/react-overlays-3.2.0.tgz#ed8335adb0871e701b0fc8396c44dfd2467e7a55"
integrity sha512-YTgCmw6l4uBOYylSnc3V8WLX+A0EoGnzDrqkYz0K7MUKbMBZFpaxLXH4EF9eZbspd+syZHQ5XAABI7n/zak1EA==
react-overlays@^4.0.0:
version "4.0.0"
resolved "https://registry.yarnpkg.com/react-overlays/-/react-overlays-4.0.0.tgz#7fbcb60d12fee3733e9e4dd216b225e94fb3befe"
integrity sha512-LpznWocwgeB5oWKg6cDdkqKP7MbX4ClKbJqgZGUMXPRBBYcqrgM6TjjZ/8DeurNU//GuqwQMjhmo/JVma4XEWw==
dependencies:
"@babel/runtime" "^7.4.5"
"@popperjs/core" "^2.0.0"
Expand Down

0 comments on commit 0e86a51

Please sign in to comment.