diff --git a/CHANGELOG.md b/CHANGELOG.md index 04f57951d..dc2c94032 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Call `getDerivedStateFromProps()` from `AutoControlledComponent` in `Dropdown` ([#1416](https://github.com/stardust-ui/react/pull/1416)) - Fix `backgroundHover1` color in the Teams dark theme `colorScheme` @mnajdova ([1437](https://github.com/stardust-ui/react/pull/1437)) - Revert changes with different roots in `Icon` component @layershifter ([#1435](https://github.com/stardust-ui/react/pull/1435)) +- Fix flickering issues with `Dropdown` and `Popup` components @Bugaa92 ([#1434](https://github.com/stardust-ui/react/pull/1434)) ### Features - Add keyboard navigation and screen reader support for `Accordion` @silviuavram ([#1322](https://github.com/stardust-ui/react/pull/1322)) diff --git a/packages/react/src/components/Dropdown/Dropdown.tsx b/packages/react/src/components/Dropdown/Dropdown.tsx index 37334290c..ce510abb8 100644 --- a/packages/react/src/components/Dropdown/Dropdown.tsx +++ b/packages/react/src/components/Dropdown/Dropdown.tsx @@ -660,7 +660,7 @@ class Dropdown extends AutoControlledComponent, Dropdo position={position} offset={offset} rtl={rtl} - eventsEnabled={open} + enabled={open} targetRef={this.containerRef} positioningDependencies={[items.length]} > diff --git a/packages/react/src/components/Menu/MenuItem.tsx b/packages/react/src/components/Menu/MenuItem.tsx index 36213db91..3ffc87ff7 100644 --- a/packages/react/src/components/Menu/MenuItem.tsx +++ b/packages/react/src/components/Menu/MenuItem.tsx @@ -247,7 +247,6 @@ class MenuItem extends AutoControlledComponent, MenuIt align={vertical ? 'top' : 'start'} position={vertical ? 'after' : 'below'} targetRef={this.itemRef} - modifiers={{ flip: { flipVariationsByContent: true } }} > {Menu.create(menu, { defaultProps: { diff --git a/packages/react/src/lib/positioner/Popper.tsx b/packages/react/src/lib/positioner/Popper.tsx index 6b1596a48..caea9b9ab 100644 --- a/packages/react/src/lib/positioner/Popper.tsx +++ b/packages/react/src/lib/positioner/Popper.tsx @@ -22,7 +22,7 @@ const Popper: React.FunctionComponent = props => { const { align, children, - eventsEnabled, + enabled, modifiers: userModifiers, offset, pointerTargetRef, @@ -51,14 +51,33 @@ const Popper: React.FunctionComponent = props => { [rtl, offset, position], ) - React.useEffect( + const scheduleUpdate = React.useCallback(() => { + if (popperRef.current) { + popperRef.current.scheduleUpdate() + } + }, []) + + const destroyInstance = React.useCallback(() => { + if (popperRef.current) { + popperRef.current.destroy() + popperRef.current = null + } + }, []) + + const createInstance = React.useCallback( () => { + destroyInstance() + + if (!enabled || !targetRef.current || !contentRef.current) { + return + } + const pointerTargetRefElement = pointerTargetRef && pointerTargetRef.current const popperHasScrollableParent = getScrollParent(contentRef.current) !== document.body const modifiers: PopperJS.Modifiers = _.merge( { preventOverflow: { padding: 0 } }, - { flip: { padding: 0 } }, + { flip: { padding: 0, flipVariationsByContent: true } }, /** * When the popper box is placed in the context of a scrollable element, we need to set * preventOverflow.escapeWithReference to true and flip.boundariesElement to 'scrollParent' (default is 'viewport') @@ -92,7 +111,6 @@ const Popper: React.FunctionComponent = props => { const options: PopperJS.PopperOptions = { placement: proposedPlacement, - eventsEnabled, positionFixed, modifiers, onCreate: handleUpdate, @@ -100,31 +118,44 @@ const Popper: React.FunctionComponent = props => { } popperRef.current = createPopper(targetRef.current, contentRef.current, options) - return () => popperRef.current.destroy() }, - [computedModifiers, eventsEnabled, userModifiers, positionFixed, proposedPlacement], + // TODO review dependencies for popperHasScrollableParent + [computedModifiers, enabled, userModifiers, positionFixed, proposedPlacement], ) React.useEffect( () => { - popperRef.current.scheduleUpdate() + createInstance() + return destroyInstance }, - [...positioningDependencies, computedPlacement], + [createInstance], ) + React.useEffect(scheduleUpdate, [...positioningDependencies, computedPlacement]) + const child = typeof children === 'function' ? (children as PopperChildrenFn)({ placement: computedPlacement, - scheduleUpdate: () => popperRef.current && popperRef.current.scheduleUpdate(), + scheduleUpdate, }) - : React.Children.only(children) - - return {child as React.ReactElement} + : children + + return ( + { + contentRef.current = contentElement + // for correct positioning we need to create the PopperJS instance immediately after we get a ref to the popper box + createInstance() + }} + > + {React.Children.only(child) as React.ReactElement} + + ) } Popper.defaultProps = { - eventsEnabled: true, + enabled: true, positionFixed: false, positioningDependencies: [], } diff --git a/packages/react/src/lib/positioner/types.ts b/packages/react/src/lib/positioner/types.ts index 1700ab265..2e1857ca9 100644 --- a/packages/react/src/lib/positioner/types.ts +++ b/packages/react/src/lib/positioner/types.ts @@ -1,3 +1,4 @@ +import * as React from 'react' import PopperJS from 'popper.js' export type Position = 'above' | 'below' | 'before' | 'after' @@ -42,9 +43,9 @@ export interface PopperProps extends PositioningProps { /** * Enables events (resize, scroll). - * @prop {Boolean} eventsEnabled=true + * @prop {Boolean} enabled=true */ - eventsEnabled?: boolean + enabled?: boolean /** * List of modifiers used to modify the offsets before they are applied to the Popper box. @@ -66,7 +67,7 @@ export interface PopperProps extends PositioningProps { /** * Ref object containing the target node (the element that we're using as reference for Popper box). */ - targetRef?: React.RefObject + targetRef: React.RefObject /** * Rtl attribute for the component.