Fix object detail view appearing up on every arrow key (#31514)

e2e/test/scenarios/question/nulls.cy.spec.js

@@ -38,7 +38,7 @@ describe("scenarios > question > null", () => {
     cy.findByText("13571").click();
 
     cy.log("'No Results since at least v0.34.3");
-    cy.get("#detail-shortcut").click();
+    cy.findByTestId("detail-shortcut").click();
     cy.findByRole("dialog").within(() => {
       cy.findByText(/Discount/i);
       cy.findByText("Empty");

e2e/test/scenarios/visualizations/reproductions/30039-object-detail-navigation.cy.spec.js

+import { openNativeEditor, restore, runNativeQuery } from "e2e/support/helpers";
+
+describe("issue 30039", () => {
+  beforeEach(() => {
+    restore();
+    cy.signInAsNormalUser();
+  });
+
+  it("should not trigger object detail navigation after the modal was closed (metabase#30039)", () => {
+    openNativeEditor().type("select * from ORDERS LIMIT 2");
+    runNativeQuery();
+    cy.findAllByTestId("detail-shortcut").first().click();
+    cy.findByTestId("object-detail").should("be.visible");
+
+    cy.realPress("{esc}");
+    cy.findByTestId("object-detail").should("not.exist");
+
+    cy.get("@editor").type("{downArrow};");
+    runNativeQuery();
+    cy.findByTestId("object-detail").should("not.exist");
+  });
+});

frontend/src/metabase/App.tsx

-import { ReactNode, useState } from "react";
+import { ReactNode, useEffect, useState } from "react";
 import { connect } from "react-redux";
 import { Location } from "history";
 
-import { useMount } from "react-use";
 import ScrollToTop from "metabase/hoc/ScrollToTop";
 import {
   Archived,
@@ -91,9 +90,9 @@ function App({
 }: AppProps) {
   const [viewportElement, setViewportElement] = useState<HTMLElement | null>();
 
-  useMount(() => {
+  useEffect(() => {
     initializeIframeResizer();
-  });
+  }, []);
 
   return (
     <ErrorBoundary onError={onError}>

frontend/src/metabase/query_builder/components/filters/modals/BulkFilterModal/BulkFilterFieldSearch.tsx

 import { useEffect, useState, useRef } from "react";
 import { t } from "ttag";
 
-import { useMount } from "react-use";
-
 import {
   SearchContainer,
   SearchInput,
@@ -19,7 +17,7 @@ export const FieldSearch = ({
   const [showSearch, setShowSearch] = useState(false);
   const inputRef = useRef<HTMLInputElement | null>(null);
 
-  useMount(() => {
+  useEffect(() => {
     const searchToggleListener = (e: KeyboardEvent) => {
       if ((e.ctrlKey || e.metaKey) && e.key === "k") {
         setShowSearch(true);
@@ -27,7 +25,7 @@ export const FieldSearch = ({
     };
     window.addEventListener("keydown", searchToggleListener);
     return () => window.removeEventListener("keydown", searchToggleListener);
-  });
+  }, []);
 
   const shouldClose = () => {
     const input = inputRef.current;

frontend/src/metabase/query_builder/components/view/ViewHeader.jsx

@@ -2,7 +2,7 @@ import { useEffect, useCallback, useState } from "react";
 import PropTypes from "prop-types";
 import { t } from "ttag";
 import cx from "classnames";
-import { usePrevious, useMount } from "react-use";
+import { usePrevious } from "react-use";
 
 import * as Urls from "metabase/lib/urls";
 import { useDispatch, useSelector } from "metabase/lib/redux";
@@ -213,12 +213,12 @@ function SavedQuestionLeftSide(props) {
 
   const [showSubHeader, setShowSubHeader] = useState(true);
 
-  useMount(() => {
+  useEffect(() => {
     const timerId = setTimeout(() => {
       setShowSubHeader(false);
     }, 4000);
     return () => clearTimeout(timerId);
-  });
+  }, []);
 
   const hasLastEditInfo = question.lastEditInfo() != null;
   const isDataset = question.isDataset();

frontend/src/metabase/query_builder/containers/QueryBuilder.jsx

@@ -293,12 +293,12 @@ function QueryBuilder(props) {
 
   useMount(() => {
     initializeQB(location, params);
-  }, []);
+  });
 
-  useMount(() => {
+  useEffect(() => {
     window.addEventListener("resize", forceUpdateDebounced);
     return () => window.removeEventListener("resize", forceUpdateDebounced);
-  }, []);
+  });
 
   const isExistingModelDirty = useMemo(
     () => isModelQueryDirty || isMetadataDirty,

frontend/src/metabase/visualizations/components/ObjectDetail/ObjectDetail.tsx

@@ -185,12 +185,37 @@ export function ObjectDetailView({
     if (!_.isEmpty(tableForeignKeys)) {
       loadFKReferences();
     }
-    window.addEventListener("keydown", onKeyDown, true);
+  });
 
-    return () => {
-      window.removeEventListener("keydown", onKeyDown, true);
+  useEffect(() => {
+    if (hasNotFoundError) {
+      return;
+    }
+
+    const onKeyDown = (event: KeyboardEvent) => {
+      const capturedKeys: Record<string, () => void> = {
+        ArrowUp: viewPreviousObjectDetail,
+        ArrowDown: viewNextObjectDetail,
+        Escape: closeObjectDetail,
+      };
+
+      if (capturedKeys[event.key]) {
+        event.preventDefault();
+        capturedKeys[event.key]();
+      }
+      if (event.key === "Escape") {
+        closeObjectDetail();
+      }
     };
-  });
+
+    window.addEventListener("keydown", onKeyDown, true);
+    return () => window.removeEventListener("keydown", onKeyDown, true);
+  }, [
+    hasNotFoundError,
+    viewPreviousObjectDetail,
+    viewNextObjectDetail,
+    closeObjectDetail,
+  ]);
 
   useEffect(() => {
     if (maybeLoading && pkIndex !== undefined) {
@@ -255,22 +280,6 @@ export function ObjectDetailView({
     [zoomedRowID, followForeignKey],
   );
 
-  const onKeyDown = (event: KeyboardEvent) => {
-    const capturedKeys: Record<string, () => void> = {
-      ArrowUp: viewPreviousObjectDetail,
-      ArrowDown: viewNextObjectDetail,
-      Escape: closeObjectDetail,
-    };
-
-    if (capturedKeys[event.key]) {
-      event.preventDefault();
-      capturedKeys[event.key]();
-    }
-    if (event.key === "Escape") {
-      closeObjectDetail();
-    }
-  };
-
   if (!data) {
     return null;
   }

frontend/src/metabase/visualizations/components/TableInteractive/TableInteractive.jsx

@@ -1137,7 +1137,6 @@ export default _.compose(
 
 const DetailShortcut = forwardRef((_props, ref) => (
   <div
-    id="detail-shortcut"
     className="TableInteractive-cellWrapper cursor-pointer"
     ref={ref}
     style={{
@@ -1148,6 +1147,7 @@ const DetailShortcut = forwardRef((_props, ref) => (
       width: SIDEBAR_WIDTH,
       zIndex: 3,
     }}
+    data-testid="detail-shortcut"
   >
     <Tooltip tooltip={t`View Details`}>
       <Button