41994 command pallet search descriptions (#42117)

* Adding Descriptions to search results in palette

* Palette Results List component

* adjusting e2e and unit tests

* type fix

* removing duplicate theme font

* fix typo, palette types adjustment

e2e/test/scenarios/onboarding/command-palette.cy.spec.js

 import { USERS } from "e2e/support/cypress_data";
-import { ORDERS_DASHBOARD_ID } from "e2e/support/cypress_sample_instance_data";
+import {
+  ORDERS_DASHBOARD_ID,
+  ORDERS_COUNT_QUESTION_ID,
+} from "e2e/support/cypress_sample_instance_data";
 import {
   restore,
   openCommandPalette,
@@ -22,6 +25,11 @@ describe("command palette", () => {
   });
 
   it("should render a searchable command palette", () => {
+    //Add a description for a check
+    cy.request("PUT", `/api/card/${ORDERS_COUNT_QUESTION_ID}`, {
+      description: "The best question",
+    });
+
     //Request to have an item in the recents list
     cy.request(`/api/dashboard/${ORDERS_DASHBOARD_ID}`);
     cy.visit("/");
@@ -56,10 +64,9 @@ describe("command palette", () => {
       cy.log("Should search entities and docs");
       commandPaletteSearch().type("Orders, Count");
 
-      cy.findByRole("option", { name: "Orders, Count" }).should(
-        "contain.text",
-        "Our analytics",
-      );
+      cy.findByRole("option", { name: "Orders, Count" })
+        .should("contain.text", "Our analytics")
+        .should("contain.text", "The best question");
 
       cy.findByText('Search documentation for "Orders, Count"').should("exist");
 

frontend/src/metabase/palette/components/PaletteResultItem.tsx

 import { t } from "ttag";
 
 import { color } from "metabase/lib/colors";
-import { Flex, Text, Icon, Box, type IconName } from "metabase/ui";
+import { Flex, Text, Icon, Box } from "metabase/ui";
 
-import type { PaletteAction } from "../types";
+import type { PaletteActionImpl } from "../types";
 
 interface PaletteResultItemProps {
-  item: PaletteAction;
+  item: PaletteActionImpl;
   active: boolean;
 }
 
@@ -35,22 +35,29 @@ export const PaletteResultItem = ({ item, active }: PaletteResultItemProps) => {
       c={active ? color("white") : color("text-dark")}
       aria-label={item.name}
     >
-      <Flex gap=".5rem" style={{ minWidth: 0 }}>
-        {item.icon && (
-          <Icon
-            aria-hidden
-            name={(item.icon as IconName) || "click"}
-            color={iconColor}
-            style={{
-              flexBasis: "16px",
-            }}
-          />
-        )}
+      {/** Icon Container */}
+      {item.icon && (
+        <Icon
+          aria-hidden
+          name={item.icon || "click"}
+          color={iconColor}
+          style={{
+            flexBasis: "16px",
+          }}
+        />
+      )}
+      {/**Text container */}
+      <Flex
+        direction="column"
+        style={{
+          flexGrow: 1,
+          flexBasis: 0,
+          overflowX: "hidden",
+        }}
+      >
         <Box
           component="span"
           style={{
-            flexGrow: 1,
-            flexBasis: 0,
             textOverflow: "ellipsis",
             overflow: "hidden",
             whiteSpace: "nowrap",
@@ -80,7 +87,20 @@ export const PaletteResultItem = ({ item, active }: PaletteResultItemProps) => {
             >{`— ${parentName}`}</Text>
           )}
         </Box>
+        <Text
+          component="span"
+          color={active ? "white" : "text-light"}
+          fw="normal"
+          style={{
+            textOverflow: "ellipsis",
+            overflow: "hidden",
+            whiteSpace: "nowrap",
+          }}
+        >
+          {item.subtitle}
+        </Text>
       </Flex>
+      {/** Active container */}
       {active && (
         <Flex
           aria-hidden

frontend/src/metabase/palette/components/PaletteResults.tsx

-import { useKBar, useMatches, KBarResults } from "kbar";
-import { useState, useMemo } from "react";
-import { useDebounce, useKeyPressEvent } from "react-use";
+import { useKBar, useMatches } from "kbar";
+import { useMemo } from "react";
+import { useKeyPressEvent } from "react-use";
 import _ from "underscore";
 
 import { color } from "metabase/lib/colors";
-import { SEARCH_DEBOUNCE_DURATION } from "metabase/lib/constants";
 import { Flex, Box } from "metabase/ui";
 
 import { useCommandPalette } from "../hooks/useCommandPalette";
-import type { PaletteAction } from "../types";
-import { processResults, findClosesestActionIndex } from "../utils";
+import type { PaletteActionImpl } from "../types";
+import { processResults, findClosestActionIndex } from "../utils";
 
 import { PaletteResultItem } from "./PaletteResultItem";
+import { PaletteResultList } from "./PaletteResultsList";
 
 const PAGE_SIZE = 4;
 
 export const PaletteResults = () => {
   // Used for finding actions within the list
-  const { searchQuery, query } = useKBar(state => ({
-    searchQuery: state.searchQuery,
-  }));
-  const trimmedQuery = searchQuery.trim();
+  const { query } = useKBar();
 
-  // Used for finding objects across the Metabase instance
-  const [debouncedSearchText, setDebouncedSearchText] = useState(trimmedQuery);
-
-  useDebounce(
-    () => {
-      setDebouncedSearchText(trimmedQuery);
-    },
-    SEARCH_DEBOUNCE_DURATION,
-    [trimmedQuery],
-  );
-
-  useCommandPalette({
-    query: trimmedQuery,
-    debouncedSearchText,
-  });
+  useCommandPalette();
 
   const { results } = useMatches();
 
-  const processedResults = useMemo(() => processResults(results), [results]);
+  const processedResults = useMemo(
+    () => processResults(results as (PaletteActionImpl | string)[]),
+    [results],
+  );
 
   useKeyPressEvent("End", () => {
     const lastIndex = processedResults.length - 1;
@@ -53,26 +39,26 @@ export const PaletteResults = () => {
 
   useKeyPressEvent("PageDown", () => {
     query.setActiveIndex(i =>
-      findClosesestActionIndex(processedResults, i, PAGE_SIZE),
+      findClosestActionIndex(processedResults, i, PAGE_SIZE),
     );
   });
 
   useKeyPressEvent("PageUp", () => {
     query.setActiveIndex(i =>
-      findClosesestActionIndex(processedResults, i, -PAGE_SIZE),
+      findClosestActionIndex(processedResults, i, -PAGE_SIZE),
     );
   });
 
   return (
     <Flex align="stretch" direction="column" p="0.75rem 0">
-      <KBarResults
+      <PaletteResultList
         items={processedResults} // items needs to be a stable reference, otherwise the activeIndex will constantly be hijacked
         maxHeight={530}
         onRender={({
           item,
           active,
         }: {
-          item: string | PaletteAction;
+          item: string | PaletteActionImpl;
           active: boolean;
         }) => {
           const isFirst = processedResults[0] === item;

frontend/src/metabase/palette/components/PaletteResults.unit.spec.tsx

@@ -10,10 +10,9 @@ import {
 import {
   renderWithProviders,
   screen,
-  mockGetBoundingClientRect,
   within,
   waitFor,
-  mockOffsetHeightAndWidth,
+  mockScrollTo,
 } from "__support__/ui";
 import { getAdminPaths } from "metabase/admin/app/reducers";
 import {
@@ -74,6 +73,7 @@ const dashboard = createMockCollectionItem({
   model: "dashboard",
   name: "Bar Dashboard",
   collection: collection_1,
+  description: "Such Bar. Much Wow.",
 });
 
 const recents_1 = createMockRecentItem({
@@ -93,8 +93,7 @@ const recents_2 = createMockRecentItem({
   }),
 });
 
-mockGetBoundingClientRect();
-mockOffsetHeightAndWidth(10); // This is absurdley small, but it allows all the items to render in the "virtual list"
+mockScrollTo();
 
 const setup = ({ query }: { query?: string } = {}) => {
   setupDatabasesEndpoints([DATABASE]);
@@ -165,6 +164,10 @@ describe("PaletteResults", () => {
     expect(
       await screen.findByRole("option", { name: "Bar Dashboard" }),
     ).toBeInTheDocument();
+
+    expect(
+      await screen.findByRole("option", { name: "Bar Dashboard" }),
+    ).toHaveTextContent("Such Bar. Much Wow.");
     expect(
       await screen.findByText('Search documentation for "Bar"'),
     ).toBeInTheDocument();

frontend/src/metabase/palette/components/PaletteResultsList.tsx

+/**
+ * This component was actually copied from the kbar library, but
+ * modified to remove virtualization of the list. This was due to virtualization
+ * libraries not handling dynamically sized lists where the list changes from render to
+ * render very well (it seemed to recompute when the list length changed, not the contents)
+ *
+ * Original can be found at https://github.com/timc1/kbar/blob/846b2c1a89f6cbff1ce947b82d04cb96a5066fbb/src/KBarResults.tsx
+ */
+
+import { useKBar, KBAR_LISTBOX, getListboxItemId } from "kbar";
+import * as React from "react";
+
+import type { PaletteActionImpl } from "../types";
+
+const START_INDEX = 0;
+
+interface RenderParams<T = PaletteActionImpl | string> {
+  item: T;
+  active: boolean;
+}
+
+interface PaletteResultListProps {
+  items: (PaletteActionImpl | string)[];
+  onRender: (params: RenderParams) => React.ReactElement;
+  maxHeight?: number;
+}
+
+export const PaletteResultList: React.FC<PaletteResultListProps> = props => {
+  const activeRef = React.useRef<HTMLDivElement>(null);
+  const parentRef = React.useRef<HTMLDivElement>(null);
+
+  // store a ref to all items so we do not have to pass
+  // them as a dependency when setting up event listeners.
+  const itemsRef = React.useRef(props.items);
+  itemsRef.current = props.items;
+
+  const { query, search, currentRootActionId, activeIndex, options } = useKBar(
+    state => ({
+      search: state.searchQuery,
+      currentRootActionId: state.currentRootActionId,
+      activeIndex: state.activeIndex,
+    }),
+  );
+
+  React.useEffect(() => {
+    const handler = (event: KeyboardEvent) => {
+      if (event.isComposing) {
+        return;
+      }
+
+      if (event.key === "ArrowUp" || (event.ctrlKey && event.key === "p")) {
+        event.preventDefault();
+        event.stopPropagation();
+        query.setActiveIndex(index => {
+          let nextIndex = index > START_INDEX ? index - 1 : index;
+          // avoid setting active index on a group
+          if (typeof itemsRef.current[nextIndex] === "string") {
+            if (nextIndex === 0) {
+              return index;
+            }
+            nextIndex -= 1;
+          }
+          return nextIndex;
+        });
+      } else if (
+        event.key === "ArrowDown" ||
+        (event.ctrlKey && event.key === "n")
+      ) {
+        event.preventDefault();
+        event.stopPropagation();
+        query.setActiveIndex(index => {
+          let nextIndex =
+            index < itemsRef.current.length - 1 ? index + 1 : index;
+          // avoid setting active index on a group
+          if (typeof itemsRef.current[nextIndex] === "string") {
+            if (nextIndex === itemsRef.current.length - 1) {
+              return index;
+            }
+            nextIndex += 1;
+          }
+          return nextIndex;
+        });
+      } else if (event.key === "Enter") {
+        event.preventDefault();
+        event.stopPropagation();
+        // storing the active dom element in a ref prevents us from
+        // having to calculate the current action to perform based
+        // on the `activeIndex`, which we would have needed to add
+        // as part of the dependencies array.
+        activeRef.current?.click();
+      }
+    };
+    window.addEventListener("keydown", handler, { capture: true });
+    return () =>
+      window.removeEventListener("keydown", handler, { capture: true });
+  }, [query]);
+
+  React.useEffect(() => {
+    if (activeIndex > 1) {
+      activeRef.current?.scrollIntoView({
+        behavior: "smooth",
+        block: "nearest",
+      });
+    } else {
+      parentRef.current?.scrollTo({ top: 0, behavior: "smooth" });
+    }
+  }, [activeIndex]);
+
+  React.useEffect(() => {
+    // TODO(tim): fix scenario where async actions load in
+    // and active index is reset to the first item. i.e. when
+    // users register actions and bust the `useRegisterActions`
+    // cache, we won't want to reset their active index as they
+    // are navigating the list.
+    query.setActiveIndex(
+      // avoid setting active index on a group
+      typeof props.items[START_INDEX] === "string"
+        ? START_INDEX + 1
+        : START_INDEX,
+    );
+  }, [search, currentRootActionId, props.items, query]);
+
+  const execute = React.useCallback(
+    (item: RenderParams["item"]) => {
+      if (typeof item === "string") {
+        return;
+      }
+      if (item.command) {
+        item.command.perform(item);
+        query.toggle();
+      } else {
+        query.setSearch("");
+        query.setCurrentRootAction(item.id);
+      }
+      options.callbacks?.onSelectAction?.(item);
+    },
+    [query, options],
+  );
+
+  return (
+    <div
+      ref={parentRef}
+      style={{
+        maxHeight: props.maxHeight || 400,
+        overflow: "auto",
+      }}
+    >
+      <div
+        role="listbox"
+        id={KBAR_LISTBOX}
+        style={{
+          position: "relative",
+          width: "100%",
+        }}
+      >
+        {props.items.map((item, index) => {
+          const handlers = typeof item !== "string" && {
+            onPointerMove: () =>
+              activeIndex !== index && query.setActiveIndex(index),
+            onPointerDown: () => query.setActiveIndex(index),
+            onClick: () => execute(item),
+          };
+          const active = index === activeIndex;
+
+          return (
+            <div
+              ref={active ? activeRef : null}
+              id={getListboxItemId(index)}
+              role="option"
+              aria-selected={active}
+              key={typeof item === "string" ? item : item.id}
+              {...handlers}
+            >
+              {React.cloneElement(
+                props.onRender({
+                  item,
+                  active,
+                }),
+              )}
+            </div>
+          );
+        })}
+      </div>
+    </div>
+  );
+};

frontend/src/metabase/palette/hooks/useCommandPalette.tsx

-import { useRegisterActions } from "kbar";
-import { useMemo } from "react";
+import { useRegisterActions, useKBar } from "kbar";
+import { useMemo, useState } from "react";
 import { push } from "react-router-redux";
+import { useDebounce } from "react-use";
 import { t } from "ttag";
 
 import { getAdminPaths } from "metabase/admin/app/selectors";
 import { getSectionsWithPlugins } from "metabase/admin/settings/selectors";
-import { useListRecentItemsQuery, skipToken } from "metabase/api";
-import { useSearchListQuery } from "metabase/common/hooks";
+import { useListRecentItemsQuery, useSearchQuery } from "metabase/api";
 import { ROOT_COLLECTION } from "metabase/entities/collections";
 import Search from "metabase/entities/search";
+import { SEARCH_DEBOUNCE_DURATION } from "metabase/lib/constants";
 import { getIcon } from "metabase/lib/icon";
 import { getName } from "metabase/lib/name";
 import { useDispatch, useSelector } from "metabase/lib/redux";
@@ -20,42 +21,52 @@ import {
   getSettings,
 } from "metabase/selectors/settings";
 import { getShowMetabaseLinks } from "metabase/selectors/whitelabel";
-import type { SearchResult } from "metabase-types/api";
 
 import type { PaletteAction } from "../types";
 
-export type PalettePageId = "root" | "admin_settings";
-
-export const useCommandPalette = ({
-  query,
-  debouncedSearchText,
-}: {
-  query: string;
-  debouncedSearchText: string;
-}) => {
+export const useCommandPalette = () => {
   const dispatch = useDispatch();
   const docsUrl = useSelector(state => getDocsUrl(state, {}));
   const showMetabaseLinks = useSelector(getShowMetabaseLinks);
 
-  const hasQuery = query.length > 0;
+  // Used for finding actions within the list
+  const { searchQuery } = useKBar(state => ({
+    searchQuery: state.searchQuery,
+  }));
+  const trimmedQuery = searchQuery.trim();
+
+  // Used for finding objects across the Metabase instance
+  const [debouncedSearchText, setDebouncedSearchText] = useState(trimmedQuery);
+
+  useDebounce(
+    () => {
+      setDebouncedSearchText(trimmedQuery);
+    },
+    SEARCH_DEBOUNCE_DURATION,
+    [trimmedQuery],
+  );
+
+  const hasQuery = searchQuery.length > 0;
 
   const {
-    data: searchResults,
+    currentData: searchResults,
+    isFetching: isSearchLoading,
     error: searchError,
-    isLoading: isSearchLoading,
-  } = useSearchListQuery<SearchResult>({
-    enabled: !!debouncedSearchText,
-    query: { q: debouncedSearchText, limit: 20 },
-    reload: true,
-  });
-
-  const { data: recentItems } = useListRecentItemsQuery(
-    debouncedSearchText ? skipToken : undefined,
+  } = useSearchQuery(
+    {
+      q: debouncedSearchText,
+      limit: 20,
+    },
     {
+      skip: !debouncedSearchText,
       refetchOnMountOrArgChange: true,
     },
   );
 
+  const { data: recentItems } = useListRecentItemsQuery(undefined, {
+    refetchOnMountOrArgChange: true,
+  });
+
   const adminPaths = useSelector(getAdminPaths);
   const settingValues = useSelector(getSettings);
   const settingsSections = useMemo<Record<string, any>>(
@@ -67,15 +78,15 @@ export const useCommandPalette = ({
     const ret: PaletteAction[] = [
       {
         id: "search_docs",
-        name: query
-          ? `Search documentation for "${query}"`
+        name: debouncedSearchText
+          ? `Search documentation for "${debouncedSearchText}"`
           : t`View documentation`,
         section: "docs",
-        keywords: query, // Always match the query string
+        keywords: debouncedSearchText, // Always match the debouncedSearchText string
         icon: "document",
         perform: () => {
-          if (query) {
-            window.open(getDocsSearchUrl({ query }));
+          if (debouncedSearchText) {
+            window.open(getDocsSearchUrl({ debouncedSearchText }));
           } else {
             window.open(docsUrl);
           }
@@ -83,7 +94,7 @@ export const useCommandPalette = ({
       },
     ];
     return ret;
-  }, [query, docsUrl]);
+  }, [debouncedSearchText, docsUrl]);
 
   const showDocsAction = showMetabaseLinks && hasQuery;
 
@@ -98,7 +109,7 @@ export const useCommandPalette = ({
         {
           id: "search-is-loading",
           name: "Loading...",
-          keywords: query,
+          keywords: searchQuery,
           section: "search",
         },
       ];
@@ -111,15 +122,16 @@ export const useCommandPalette = ({
         },
       ];
     } else if (debouncedSearchText) {
-      if (searchResults?.length) {
-        return searchResults.map(result => {
+      if (searchResults?.data?.length) {
+        return searchResults.data.map(result => {
           const wrappedResult = Search.wrapEntity(result, dispatch);
           return {
-            id: `search-result-${result.id}`,
+            id: `search-result-${result.model}-${result.id}`,
             name: result.name,
             icon: wrappedResult.getIcon().name,
             section: "search",
             keywords: debouncedSearchText,
+            subtitle: result.description || "",
             perform: () => {
               dispatch(closeModal());
               dispatch(push(wrappedResult.getUrl()));
@@ -145,8 +157,8 @@ export const useCommandPalette = ({
     return [];
   }, [
     dispatch,
-    query,
     debouncedSearchText,
+    searchQuery,
     isSearchLoading,
     searchError,
     searchResults,

frontend/src/metabase/palette/types.ts

-import type { Action } from "kbar";
+import type { Action, ActionImpl } from "kbar";
 
-export interface PaletteAction extends Action {
+import type { IconName } from "metabase/ui";
+
+interface PaletteActionExtras {
   extra?: {
     parentCollection?: string | null;
     isVerified?: boolean;
     database?: string | null;
   };
 }
+
+export type PaletteAction = Action &
+  PaletteActionExtras & {
+    subtitle?: Action["subtitle"];
+    icon?: IconName;
+  };
+
+export type PaletteActionImpl = ActionImpl &
+  PaletteActionExtras & {
+    subtitle?: Action["subtitle"];
+    icon?: IconName;
+  };

frontend/src/metabase/palette/utils.ts

-import type { ActionImpl } from "kbar";
 import { t } from "ttag";
 import _ from "underscore";
 
+import type { PaletteActionImpl } from "./types";
+
 export const processResults = (
-  results: (string | ActionImpl)[],
-): (string | ActionImpl)[] => {
+  results: (string | PaletteActionImpl)[],
+): (string | PaletteActionImpl)[] => {
   const groupedResults = _.groupBy(
-    results.filter((r): r is ActionImpl => !(typeof r === "string")),
+    results.filter((r): r is PaletteActionImpl => !(typeof r === "string")),
     "section",
   );
 
@@ -19,7 +20,10 @@ export const processResults = (
   return [...recent, ...actions.slice(0, 6), ...admin, ...search, ...docs];
 };
 
-export const processSection = (sectionName: string, items?: ActionImpl[]) => {
+export const processSection = (
+  sectionName: string,
+  items?: PaletteActionImpl[],
+) => {
   if (items && items.length > 0) {
     return [sectionName, ...items];
   } else {
@@ -27,20 +31,20 @@ export const processSection = (sectionName: string, items?: ActionImpl[]) => {
   }
 };
 
-export const findClosesestActionIndex = (
-  actions: (string | ActionImpl)[],
+export const findClosestActionIndex = (
+  actions: (string | PaletteActionImpl)[],
   index: number,
   diff: number,
 ): number => {
   if (index + diff < 0) {
-    return findClosesestActionIndex(actions, -1, 1);
+    return findClosestActionIndex(actions, -1, 1);
   } else if (index + diff > actions.length - 1) {
-    return findClosesestActionIndex(actions, actions.length, -1);
+    return findClosestActionIndex(actions, actions.length, -1);
   } else if (typeof actions[index + diff] === "string") {
     if (diff < 0) {
-      return findClosesestActionIndex(actions, index, diff - 1);
+      return findClosestActionIndex(actions, index, diff - 1);
     } else {
-      return findClosesestActionIndex(actions, index, diff + 1);
+      return findClosestActionIndex(actions, index, diff + 1);
     }
   }
 

frontend/src/metabase/palette/utils.unit.spec.ts

-import type { ActionImpl } from "kbar";
-
+import type { PaletteActionImpl } from "./types";
 import { processResults, processSection } from "./utils";
 
 interface mockAction {
@@ -10,7 +9,7 @@ interface mockAction {
 const createMockAction = ({
   name,
   section = "basic",
-}: mockAction): ActionImpl => ({ name, section } as ActionImpl);
+}: mockAction): PaletteActionImpl => ({ name, section } as PaletteActionImpl);
 
 describe("command palette utils", () => {
   describe("processSection", () => {
@@ -25,7 +24,7 @@ describe("command palette utils", () => {
       expect(result[0]).toBe("Basic");
     });
     it("should return an empty list if there are no items", () => {
-      const items: ActionImpl[] = [];
+      const items: PaletteActionImpl[] = [];
       const result = processSection("Basic", items);
       expect(result).toHaveLength(0);
     });

frontend/test/__support__/ui.tsx

@@ -319,6 +319,13 @@ export const mockScrollBy = () => {
   window.Element.prototype.scrollBy = jest.fn();
 };
 
+/**
+ * jsdom doesn't have scrollBy, so we need to mock it
+ */
+export const mockScrollTo = () => {
+  window.Element.prototype.scrollTo = jest.fn();
+};
+
 /**
  * jsdom doesn't have DataTransfer
  */