fix false positives in toast (#5127)

* fix false positives in toast * cleanup * schema bugfix * use suspense and events * add grid handling, cleanup * yolo * fetch cleanup * lint --------- Co-authored-by: Benjamin Kane <[email protected]>
voxel51 · Nov 18, 2024 · c96506e · c96506e
1 parent 9a13da7
commit c96506e
Show file tree

Hide file tree

Showing 12 changed files with 158 additions and 88 deletions.
diff --git a/app/packages/components/src/components/Selector/Selector.tsx b/app/packages/components/src/components/Selector/Selector.tsx
@@ -32,6 +32,7 @@ export interface SelectorProps<T> {
   onMouseEnter?: React.MouseEventHandler;
   cy?: string;
   noResults?: string;
+  DuringSuspense?: (props: React.PropsWithChildren) => React.JSX.Element;
 }
 
 function Selector<T>(props: SelectorProps<T>) {
@@ -52,6 +53,7 @@ function Selector<T>(props: SelectorProps<T>) {
     onMouseEnter,
     cy,
     noResults,
+    DuringSuspense = ({ children }) => <>{children}</>,
     ...otherProps
   } = props;
 
@@ -205,7 +207,9 @@ function Selector<T>(props: SelectorProps<T>) {
               >
                 <Suspense
                   fallback={
-                    <LoadingDots style={{ float: "right" }} text="Loading" />
+                    <DuringSuspense>
+                      <LoadingDots style={{ float: "right" }} text="Loading" />
+                    </DuringSuspense>
                   }
                 >
                   <SearchResults

diff --git a/app/packages/core/src/components/Filters/NumericFieldFilter/NumericFieldFilter.tsx b/app/packages/core/src/components/Filters/NumericFieldFilter/NumericFieldFilter.tsx
@@ -4,6 +4,7 @@ import styled from "styled-components";
 import FieldLabelAndInfo from "../../FieldLabelAndInfo";
 import { LightningBolt } from "../../Sidebar/Entries/FilterablePathEntry/Icon";
 import { Button } from "../../utils";
+import useQueryPerformanceTimeout from "../use-query-performance-timeout";
 import Box from "./Box";
 import RangeSlider from "./RangeSlider";
 import useShow from "./use-show";
@@ -63,7 +64,7 @@ const NumericFieldFilter = ({ color, modal, named = true, path }: Props) => {
           )}
         />
       )}
-      <Suspense fallback={<Box text="Loading" />}>
+      <Suspense fallback={<Loading modal={modal} path={path} />}>
         {showLoadButton ? (
           <Box>
             <Button
@@ -86,4 +87,9 @@ const NumericFieldFilter = ({ color, modal, named = true, path }: Props) => {
   );
 };
 
+const Loading = ({ modal, path }: { modal: boolean; path: string }) => {
+  useQueryPerformanceTimeout(modal, path);
+  return <Box text="Loading" />;
+};
+
 export default NumericFieldFilter;
diff --git a/app/packages/core/src/components/Filters/StringFilter/StringFilter.tsx b/app/packages/core/src/components/Filters/StringFilter/StringFilter.tsx
@@ -7,6 +7,7 @@ import { useRecoilValue } from "recoil";
 import styled from "styled-components";
 import FieldLabelAndInfo from "../../FieldLabelAndInfo";
 import { isInKeypointsField } from "../state";
+import useQueryPerformanceTimeout from "../use-query-performance-timeout";
 import Checkboxes from "./Checkboxes";
 import ResultComponent from "./Result";
 import useOnSelect from "./useOnSelect";
@@ -128,6 +129,7 @@ const StringFilter = ({
             containerStyle={{ borderBottomColor: color, zIndex: 1000 }}
             toKey={(value) => String(value.value)}
             id={path}
+            DuringSuspense={withQueryPerformanceTimeout(modal, path)}
           />
         )}
         <Checkboxes
@@ -145,4 +147,11 @@ const StringFilter = ({
   );
 };
 
+const withQueryPerformanceTimeout = (modal: boolean, path: string) => {
+  return ({ children }: React.PropsWithChildren) => {
+    useQueryPerformanceTimeout(modal, path);
+    return <>{children}</>;
+  };
+};
+
 export default StringFilter;
diff --git a/app/packages/core/src/components/Filters/StringFilter/useOnSelect.ts b/app/packages/core/src/components/Filters/StringFilter/useOnSelect.ts
@@ -26,6 +26,7 @@ export default function (
         }
         selected.add(value);
         set(selectedAtom, [...selected].sort());
+
         return "";
       },
     [modal, path, selectedAtom]

diff --git a/app/packages/core/src/components/Filters/StringFilter/useSelected.ts b/app/packages/core/src/components/Filters/StringFilter/useSelected.ts
@@ -33,7 +33,6 @@ export default function (
 
   return {
     results,
-
     useSearch:
       path === "_label_tags" && queryPerformance && !isFrameField && !modal
         ? undefined

diff --git a/app/packages/core/src/components/Filters/use-query-performance-timeout.ts b/app/packages/core/src/components/Filters/use-query-performance-timeout.ts
@@ -0,0 +1,26 @@
+import { pathCanBeOptimized } from "@fiftyone/state";
+import { useEffect } from "react";
+import { useRecoilValue } from "recoil";
+import { QP_WAIT, QueryPerformanceToastEvent } from "../QueryPerformanceToast";
+
+export default function useQueryPerformanceTimeout(
+  modal: boolean,
+  path: string
+) {
+  const shouldOptimize = useRecoilValue(pathCanBeOptimized(path));
+  useEffect(() => {
+    if (modal || !shouldOptimize) {
+      return;
+    }
+
+    const timeout = setTimeout(() => {
+      window.dispatchEvent(
+        new QueryPerformanceToastEvent(path, shouldOptimize.isFrameField)
+      );
+    }, QP_WAIT);
+
+    return () => {
+      clearTimeout(timeout);
+    };
+  }, [modal, path, shouldOptimize]);
+}
diff --git a/app/packages/core/src/components/Grid/Grid.tsx b/app/packages/core/src/components/Grid/Grid.tsx
@@ -12,6 +12,7 @@ import React, {
 } from "react";
 import { useRecoilValue } from "recoil";
 import { v4 as uuid } from "uuid";
+import { QP_WAIT, QueryPerformanceToastEvent } from "../QueryPerformanceToast";
 import { gridCrop, gridSpacing, pageParameters } from "./recoil";
 import useAt from "./useAt";
 import useEscape from "./useEscape";
@@ -121,7 +122,17 @@ function Grid() {
     }
 
     const element = document.getElementById(id);
+
+    const info = fos.getQueryPerformancePath();
+    const timeout = setTimeout(() => {
+      if (info) {
+        window.dispatchEvent(
+          new QueryPerformanceToastEvent(info.path, info.isFrameField)
+        );
+      }
+    }, QP_WAIT);
     const mount = () => {
+      clearTimeout(timeout);
       document.dispatchEvent(new CustomEvent("grid-mount"));
     };
 
@@ -130,6 +141,7 @@ function Grid() {
     spotlight.addEventListener("rowchange", set);
 
     return () => {
+      clearTimeout(timeout);
       freeVideos();
       spotlight.removeEventListener("load", mount);
       spotlight.removeEventListener("rowchange", set);

diff --git a/app/packages/core/src/components/QueryPerformanceToast.tsx b/app/packages/core/src/components/QueryPerformanceToast.tsx
@@ -1,18 +1,35 @@
-import { Toast } from "@fiftyone/components";
+import { useTrackEvent } from "@fiftyone/analytics";
+import { Toast, useTheme } from "@fiftyone/components";
 import { QP_MODE, QP_MODE_SUMMARY } from "@fiftyone/core";
 import { getBrowserStorageEffectForKey } from "@fiftyone/state";
-import { Box, Button, Typography } from "@mui/material";
 import { Bolt } from "@mui/icons-material";
+import { Box, Button, Typography } from "@mui/material";
 import React, { useEffect, useState } from "react";
 import { createPortal } from "react-dom";
-import { atom, useRecoilState, useRecoilValue } from "recoil";
-import { useTheme } from "@fiftyone/components";
-import * as atoms from "@fiftyone/state/src/recoil/atoms";
-import * as fos from "@fiftyone/state";
-import { useTrackEvent } from "@fiftyone/analytics";
+import { atom, useRecoilState } from "recoil";
 
 const SHOWN_FOR = 10000;
 
+export const QP_WAIT = 5151;
+
+declare global {
+  interface WindowEventMap
+    extends GlobalEventHandlersEventMap,
+      WindowEventHandlersEventMap {
+    queryperformance: QueryPerformanceToastEvent;
+  }
+}
+export class QueryPerformanceToastEvent extends Event {
+  isFrameField: boolean;
+  path: string;
+
+  constructor(path: string, isFrameField: boolean) {
+    super("queryperformance");
+    this.path = path;
+    this.isFrameField = isFrameField;
+  }
+}
+
 const hideQueryPerformanceToast = atom({
   key: "hideQueryPerformanceToast",
   default: false,
@@ -34,46 +51,35 @@ const QueryPerformanceToast = ({
   },
   text = "View Documentation",
 }) => {
-  const [path, setPath] = useState("");
-  const indexed = useRecoilValue(fos.pathHasIndexes(path));
-  const [shown, setShown] = useState(false);
+  const [data, setData] = useState<{
+    path: string;
+    isFrameField: boolean;
+  } | null>(null);
   const [disabled, setDisabled] = useRecoilState(hideQueryPerformanceToast);
-  const queryPerformance = useRecoilValue(fos.queryPerformance);
   const element = document.getElementById("queryPerformance");
   const theme = useTheme();
-  const frameFields = useRecoilValue(atoms.frameFields);
   const trackEvent = useTrackEvent();
 
   useEffect(() => {
-    const listen = (event) => {
+    const listen = (event: QueryPerformanceToastEvent) => {
       onDispatch(event);
-      setPath(event.path);
-      setShown(true);
+      setData({ path: event.path, isFrameField: event.isFrameField });
     };
     window.addEventListener("queryperformance", listen);
     return () => window.removeEventListener("queryperformance", listen);
-  }, []);
-
-  const onHandleClose = (event, reason) => {
-    setShown(false);
-  };
+  }, [onDispatch]);
 
   if (!element) {
     throw new Error("no query performance element");
   }
 
-  if (!shown || disabled || !queryPerformance) {
-    return null;
-  }
-
-  // don't show the toast if the path is already indexed
-  if (path && indexed) {
+  if (!data || disabled) {
     return null;
   }
 
   return createPortal(
     <Toast
-      onHandleClose={onHandleClose}
+      onHandleClose={() => setData(null)}
       duration={SHOWN_FOR}
       layout={{
         bottom: "100px !important",
@@ -87,13 +93,11 @@ const QueryPerformanceToast = ({
             variant="contained"
             size="small"
             onClick={() => {
-              onClick(
-                frameFields.some((frame) =>
-                  path.includes(`frames.${frame.path}`)
-                )
-              );
-              trackEvent("query_performance_toast_clicked", { path: path });
-              setShown(false);
+              onClick(data.isFrameField);
+              trackEvent("query_performance_toast_clicked", {
+                path: data.path,
+              });
+              setData(null);
             }}
             sx={{
               marginLeft: "auto",
@@ -116,10 +120,13 @@ const QueryPerformanceToast = ({
               size="small"
               onClick={() => {
                 setDisabled(true);
-                trackEvent("query_performance_toast_dismissed", { path: path });
-                setShown(false);
+                trackEvent("query_performance_toast_dismissed", {
+                  path: data.path,
+                });
+                setData(null);
               }}
-              style={{ marginLeft: "auto", color: theme.text.secondary }} // Right align the button
+              // Right align the button
+              style={{ marginLeft: "auto", color: theme.text.secondary }}
             >
               Dismiss
             </Button>

diff --git a/app/packages/state/src/hooks/useCreateLooker.ts b/app/packages/state/src/hooks/useCreateLooker.ts
@@ -125,7 +125,6 @@ export default <T extends AbstractLooker<BaseState>>(
         let config: ConstructorParameters<T>[1] = {
           enableTimeline,
           fieldSchema: {
-            ...fieldSchema,
             frames: {
               name: "frames",
               ftype: LIST_FIELD,
@@ -134,6 +133,7 @@ export default <T extends AbstractLooker<BaseState>>(
               fields: frameFieldSchema,
               dbField: null,
             },
+            ...fieldSchema,
           },
           sources: urls,
           frameNumber: create === FrameLooker ? frameNumber : undefined,