From 54e3b7e293521a3c076f8474acf9263060535eb9 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Fri, 12 May 2023 12:27:17 -0400 Subject: [PATCH 1/2] fix Popover controlled mode during first render --- .changeset/poor-bees-sneeze.md | 5 +++++ .../src/core/utils/components/Popover.tsx | 11 +++++++++++ 2 files changed, 16 insertions(+) create mode 100644 .changeset/poor-bees-sneeze.md diff --git a/.changeset/poor-bees-sneeze.md b/.changeset/poor-bees-sneeze.md new file mode 100644 index 00000000000..bcea40f56cb --- /dev/null +++ b/.changeset/poor-bees-sneeze.md @@ -0,0 +1,5 @@ +--- +'@itwin/itwinui-react': patch +--- + +Fixed an issue where using Popover components in controlled mode (through `visible` prop) was appending it to the wrong root element. diff --git a/packages/itwinui-react/src/core/utils/components/Popover.tsx b/packages/itwinui-react/src/core/utils/components/Popover.tsx index 04286b61037..74dffaa5a90 100644 --- a/packages/itwinui-react/src/core/utils/components/Popover.tsx +++ b/packages/itwinui-react/src/core/utils/components/Popover.tsx @@ -40,6 +40,7 @@ export type PopoverProps = { export const Popover = React.forwardRef((props: PopoverProps, ref) => { const [mounted, setMounted] = React.useState(false); const themeInfo = React.useContext(ThemeContext); + const isDomAvailable = useIsDomAvailable(); const tippyRef = React.useRef(null); const refs = useMergedRefs(tippyRef, ref); @@ -74,6 +75,10 @@ export const Popover = React.forwardRef((props: PopoverProps, ref) => { zIndex: 99999, ...props, className: cx('iui-popover', props.className), + // add additional check for isDomAvailable when using in controlled mode, + // because rootRef is not available in first render + visible: + props.visible !== undefined ? props.visible && isDomAvailable : undefined, plugins: [ lazyLoad, removeTabIndex, @@ -156,3 +161,9 @@ export const hideOnEscOrTab = { }; export default Popover; + +const useIsDomAvailable = () => { + const [isDomAvailable, setIsDomAvailable] = React.useState(false); + React.useEffect(() => void setIsDomAvailable(true), []); + return isDomAvailable; +}; From 4d1664fbbc685c7c22290a1fe7f9fc439b65b3a4 Mon Sep 17 00:00:00 2001 From: Mayank <9084735+mayank99@users.noreply.github.com> Date: Mon, 15 May 2023 08:43:18 -0400 Subject: [PATCH 2/2] use hook that's already available --- .../src/core/utils/components/Popover.tsx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/packages/itwinui-react/src/core/utils/components/Popover.tsx b/packages/itwinui-react/src/core/utils/components/Popover.tsx index 74dffaa5a90..b1586fc23ac 100644 --- a/packages/itwinui-react/src/core/utils/components/Popover.tsx +++ b/packages/itwinui-react/src/core/utils/components/Popover.tsx @@ -8,7 +8,7 @@ import cx from 'classnames'; import Tippy from '@tippyjs/react'; import type { TippyProps } from '@tippyjs/react'; import type { Placement, Instance } from 'tippy.js'; -import { useMergedRefs } from '../hooks/useMergedRefs.js'; +import { useMergedRefs, useIsClient } from '../hooks/index.js'; export type PopoverInstance = Instance; import '@itwin/itwinui-css/css/utils.css'; import { ThemeContext } from '../../ThemeProvider/ThemeProvider.js'; @@ -40,7 +40,7 @@ export type PopoverProps = { export const Popover = React.forwardRef((props: PopoverProps, ref) => { const [mounted, setMounted] = React.useState(false); const themeInfo = React.useContext(ThemeContext); - const isDomAvailable = useIsDomAvailable(); + const isDomAvailable = useIsClient(); const tippyRef = React.useRef(null); const refs = useMergedRefs(tippyRef, ref); @@ -161,9 +161,3 @@ export const hideOnEscOrTab = { }; export default Popover; - -const useIsDomAvailable = () => { - const [isDomAvailable, setIsDomAvailable] = React.useState(false); - React.useEffect(() => void setIsDomAvailable(true), []); - return isDomAvailable; -};