diff --git a/app/packages/core/src/components/Modal/Modal.tsx b/app/packages/core/src/components/Modal/Modal.tsx index 5b4b0d9ee0..6443fa8a2b 100644 --- a/app/packages/core/src/components/Modal/Modal.tsx +++ b/app/packages/core/src/components/Modal/Modal.tsx @@ -1,17 +1,17 @@ import { HelpPanel, JSONPanel } from "@fiftyone/components"; import { OPERATOR_PROMPT_AREAS, OperatorPromptArea } from "@fiftyone/operators"; import * as fos from "@fiftyone/state"; -import React, { useCallback, useEffect, useMemo, useRef } from "react"; +import React, { useCallback, useMemo, useRef } from "react"; import ReactDOM from "react-dom"; import { useRecoilCallback, useRecoilValue } from "recoil"; import styled from "styled-components"; import { ModalActionsRow } from "../Actions"; import Sidebar from "../Sidebar"; -import { useLookerHelpers } from "./hooks"; -import { modalContext } from "./modal-context"; import ModalNavigation from "./ModalNavigation"; import { ModalSpace } from "./ModalSpace"; import { TooltipInfo } from "./TooltipInfo"; +import { useLookerHelpers, useTooltipEventHandler } from "./hooks"; +import { modalContext } from "./modal-context"; import { useModalSidebarRenderEntry } from "./use-sidebar-render-entry"; const ModalWrapper = styled.div` @@ -156,9 +156,9 @@ const Modal = () => { if (activeLookerRef.current) { // we handle close logic in modal + other places return; - } else { - await modalCloseHandler(); } + + await modalCloseHandler(); } }, [] @@ -168,7 +168,7 @@ const Modal = () => { const isFullScreen = useRecoilValue(fos.fullscreen); - const { onNavigate } = useLookerHelpers(); + const { closePanels } = useLookerHelpers(); const screenParams = useMemo(() => { return isFullScreen @@ -178,14 +178,21 @@ const Modal = () => { const activeLookerRef = useRef(); - // this is so that other components can add event listeners to the active looker - const onLookerSetSubscribers = useRef<((looker: fos.Lookers) => void)[]>([]); + const addTooltipEventHandler = useTooltipEventHandler(); + const removeTooltipEventHanlderRef = useRef | null>(null); - const onLookerSet = useCallback((looker: fos.Lookers) => { - onLookerSetSubscribers.current.forEach((sub) => sub(looker)); + const onLookerSet = useCallback( + (looker: fos.Lookers) => { + looker.addEventListener("close", modalCloseHandler); - looker.addEventListener("close", modalCloseHandler); - }, []); + // remove previous event listener + removeTooltipEventHanlderRef.current?.(); + removeTooltipEventHanlderRef.current = addTooltipEventHandler(looker); + }, + [modalCloseHandler, addTooltipEventHandler] + ); const setActiveLookerRef = useCallback( (looker: fos.Lookers) => { @@ -200,7 +207,6 @@ const Modal = () => { value={{ activeLookerRef, setActiveLookerRef, - onLookerSetSubscribers, }} > { - + diff --git a/app/packages/core/src/components/Modal/ModalNavigation.tsx b/app/packages/core/src/components/Modal/ModalNavigation.tsx index 4b5dc4459b..c7c8a4ad8f 100644 --- a/app/packages/core/src/components/Modal/ModalNavigation.tsx +++ b/app/packages/core/src/components/Modal/ModalNavigation.tsx @@ -44,7 +44,7 @@ const Arrow = styled.span<{ } `; -const ModalNavigation = ({ onNavigate }: { onNavigate: () => void }) => { +const ModalNavigation = ({ closePanels }: { closePanels: () => void }) => { const showModalNavigationControls = useRecoilValue( fos.showModalNavigationControls ); @@ -75,10 +75,10 @@ const ModalNavigation = ({ onNavigate }: { onNavigate: () => void }) => { createDebouncedNavigator({ isNavigationIllegalWhen: () => modalRef.current?.hasNext === false, navigateFn: (offset) => navigation?.next(offset).then(setModal), - onNavigationStart: onNavigate, + onNavigationStart: closePanels, debounceTime: 150, }), - [navigation, onNavigate, setModal] + [navigation, closePanels, setModal] ); const previousNavigator = useMemo( @@ -86,10 +86,10 @@ const ModalNavigation = ({ onNavigate }: { onNavigate: () => void }) => { createDebouncedNavigator({ isNavigationIllegalWhen: () => modalRef.current?.hasPrevious === false, navigateFn: (offset) => navigation?.previous(offset).then(setModal), - onNavigationStart: onNavigate, + onNavigationStart: closePanels, debounceTime: 150, }), - [navigation, onNavigate, setModal] + [navigation, closePanels, setModal] ); useEffect(() => { diff --git a/app/packages/core/src/components/Modal/ModalSamplePlugin.tsx b/app/packages/core/src/components/Modal/ModalSamplePlugin.tsx index 44e06ec6ea..60f95b3b77 100644 --- a/app/packages/core/src/components/Modal/ModalSamplePlugin.tsx +++ b/app/packages/core/src/components/Modal/ModalSamplePlugin.tsx @@ -1,10 +1,9 @@ import { ErrorBoundary } from "@fiftyone/components"; import * as fos from "@fiftyone/state"; -import React, { Suspense, useCallback, useEffect, useMemo } from "react"; -import { useRecoilCallback, useRecoilValue, useSetRecoilState } from "recoil"; +import React, { Suspense, useEffect } from "react"; +import { useRecoilValue, useSetRecoilState } from "recoil"; import styled from "styled-components"; import Group from "./Group"; -import { useModalContext } from "./hooks"; import { Sample2D } from "./Sample2D"; import { Sample3d } from "./Sample3d"; @@ -23,56 +22,9 @@ export const ModalSample = React.memo(() => { const isGroup = useRecoilValue(fos.isGroup); const is3D = useRecoilValue(fos.is3DDataset); - const tooltip = fos.useTooltip(); const setIsTooltipLocked = useSetRecoilState(fos.isTooltipLocked); const setTooltipDetail = useSetRecoilState(fos.tooltipDetail); - const tooltipEventHandler = useRecoilCallback( - ({ snapshot, set }) => - (e) => { - const isTooltipLocked = snapshot - .getLoadable(fos.isTooltipLocked) - .getValue(); - - if (e.detail) { - set(fos.tooltipDetail, e.detail); - if (!isTooltipLocked && e.detail?.coordinates) { - tooltip.setCoords(e.detail.coordinates); - } - } else if (!isTooltipLocked) { - set(fos.tooltipDetail, null); - } - }, - [tooltip] - ); - - const { activeLookerRef, onLookerSetSubscribers } = useModalContext(); - - const addTooltipEventListener = useMemo(() => { - return (looker: fos.Lookers) => { - looker.addEventListener("tooltip", tooltipEventHandler); - }; - }, []); - - useEffect(() => { - onLookerSetSubscribers.current.push(addTooltipEventListener); - - return () => { - activeLookerRef?.current?.removeEventListener( - "tooltip", - tooltipEventHandler - ); - onLookerSetSubscribers.current = onLookerSetSubscribers.current.filter( - (fn) => fn !== addTooltipEventListener - ); - }; - }, [ - activeLookerRef, - addTooltipEventListener, - onLookerSetSubscribers, - tooltipEventHandler, - ]); - useEffect(() => { // reset tooltip state when modal is closed setIsTooltipLocked(false); diff --git a/app/packages/core/src/components/Modal/hooks.ts b/app/packages/core/src/components/Modal/hooks.ts index ac61249265..80c9201362 100644 --- a/app/packages/core/src/components/Modal/hooks.ts +++ b/app/packages/core/src/components/Modal/hooks.ts @@ -16,7 +16,7 @@ export const useLookerHelpers = () => { jsonPanelRef.current = jsonPanel; helpPanelRef.current = helpPanel; - const onNavigate = useCallback(() => { + const closePanels = useCallback(() => { jsonPanelRef.current?.close(); helpPanelRef.current?.close(); }, []); @@ -24,7 +24,7 @@ export const useLookerHelpers = () => { return { jsonPanel, helpPanel, - onNavigate, + closePanels, }; }; @@ -78,3 +78,38 @@ export const useModalContext = () => { return ctx; }; + +export const useTooltipEventHandler = () => { + const tooltip = fos.useTooltip(); + + const tooltipEventHandler = useRecoilCallback( + ({ snapshot, set }) => + (e) => { + const isTooltipLocked = snapshot + .getLoadable(fos.isTooltipLocked) + .getValue(); + + if (e.detail) { + set(fos.tooltipDetail, e.detail); + if (!isTooltipLocked && e.detail?.coordinates) { + tooltip.setCoords(e.detail.coordinates); + } + } else if (!isTooltipLocked) { + set(fos.tooltipDetail, null); + } + }, + [tooltip] + ); + + return useCallback( + (looker: fos.Lookers) => { + looker.removeEventListener("tooltip", tooltipEventHandler); + looker.addEventListener("tooltip", tooltipEventHandler); + + return () => { + looker.removeEventListener("tooltip", tooltipEventHandler); + }; + }, + [tooltipEventHandler] + ); +}; diff --git a/app/packages/core/src/components/Modal/modal-context.ts b/app/packages/core/src/components/Modal/modal-context.ts index aa1ae907e5..eef0524d32 100644 --- a/app/packages/core/src/components/Modal/modal-context.ts +++ b/app/packages/core/src/components/Modal/modal-context.ts @@ -4,7 +4,6 @@ import React, { createContext } from "react"; interface ModalContextT { activeLookerRef: React.MutableRefObject; setActiveLookerRef: (looker: Lookers) => void; - onLookerSetSubscribers: React.MutableRefObject<((looker: Lookers) => void)[]>; } export const modalContext = createContext(undefined);