Address code review: remove unnecessary wrapper divs and use toggle instead of always opening

Co-authored-by: iethree <30528226+iethree@users.noreply.github.com>

frontend/src/metabase/actions/containers/ActionCreator/FormCreator/FieldSettingsPopover.tsx

@@ -1,12 +1,12 @@
 import type { ChangeEvent } from "react";
-import { useMemo } from "react";
+import { useMemo, useState } from "react";
 import { t } from "ttag";
 
 import { getInputTypes } from "metabase/actions/constants";
-import TippyPopoverWithTrigger from "metabase/common/components/PopoverWithTrigger/TippyPopoverWithTrigger";
 import Radio from "metabase/common/components/Radio";
 import Toggle from "metabase/common/components/Toggle";
 import { useUniqueId } from "metabase/common/hooks/use-unique-id";
+import { Popover } from "metabase/ui";
 import { TextInput } from "metabase/ui/components/inputs/TextInput";
 import type {
   FieldSettings,
@@ -32,25 +32,26 @@ export function FieldSettingsPopover({
   fieldSettings,
   onChange,
 }: FieldSettingsPopoverProps) {
+  const [opened, setOpened] = useState(false);
+
   return (
-    <TippyPopoverWithTrigger
-      placement="bottom-end"
-      triggerContent={
+    <Popover position="bottom-end" opened={opened} onChange={setOpened}>
+      <Popover.Target>
         <SettingsTriggerIcon
           name="gear"
           size={16}
           tooltip={t`Change field settings`}
           aria-label={t`Field settings`}
+          onClick={() => setOpened(!opened)}
         />
-      }
-      maxWidth={400}
-      popoverContent={() => (
+      </Popover.Target>
+      <Popover.Dropdown maw={400}>
         <FormCreatorPopoverBody
           fieldSettings={fieldSettings}
           onChange={onChange}
         />
-      )}
-    />
+      </Popover.Dropdown>
+    </Popover>
   );
 }
 

frontend/src/metabase/actions/containers/ActionCreator/FormCreator/OptionEditor.tsx

@@ -3,8 +3,7 @@ import { useEffect, useState } from "react";
 import { t } from "ttag";
 
 import Button from "metabase/common/components/Button";
-import TippyPopoverWithTrigger from "metabase/common/components/PopoverWithTrigger/TippyPopoverWithTrigger";
-import { Icon } from "metabase/ui";
+import { Icon, Popover } from "metabase/ui";
 import type { FieldType, FieldValueOptions } from "metabase-types/api";
 
 import {
@@ -58,6 +57,7 @@ export const OptionPopover = ({
   options,
   onChange,
 }: OptionEditorProps) => {
+  const [opened, setOpened] = useState(false);
   const [text, setText] = useState(optionsToText(options));
   const [error, setError] = useState<string | null>(null);
 
@@ -83,7 +83,7 @@ export const OptionPopover = ({
     }
   };
 
-  const handleSave = (closePopover: () => void) => {
+  const handleSave = () => {
     setError(null);
 
     const nextOptions = transformOptionsIfNeeded(
@@ -96,18 +96,21 @@ export const OptionPopover = ({
       setError(error);
     } else {
       onChange(nextOptions);
-      closePopover();
+      setOpened(false);
     }
   };
 
   return (
-    <TippyPopoverWithTrigger
-      placement="bottom-end"
-      triggerContent={
-        <Icon name="list" size={20} tooltip={t`Change options`} />
-      }
-      maxWidth={400}
-      popoverContent={({ closePopover }) => (
+    <Popover position="bottom-end" opened={opened} onChange={setOpened}>
+      <Popover.Target>
+        <Icon
+          name="list"
+          size={20}
+          tooltip={t`Change options`}
+          onClick={() => setOpened(!opened)}
+        />
+      </Popover.Target>
+      <Popover.Dropdown maw={400}>
         <OptionEditorContainer>
           <TextArea
             value={text}
@@ -118,15 +121,11 @@ export const OptionPopover = ({
             {t`Press enter to add another option`}
           </AddMorePrompt>
           {hasError && <ErrorMessage>{error}</ErrorMessage>}
-          <Button
-            disabled={!canSave}
-            onClick={() => handleSave(closePopover)}
-            small
-          >
+          <Button disabled={!canSave} onClick={handleSave} small>
             {t`Save`}
           </Button>
         </OptionEditorContainer>
-      )}
-    />
+      </Popover.Dropdown>
+    </Popover>
   );
 };

frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupSelect/GroupSelect.tsx

@@ -1,4 +1,5 @@
 import cx from "classnames";
+import { useState } from "react";
 import { t } from "ttag";
 
 import { GroupSummary } from "metabase/admin/people/components/GroupSummary";
@@ -7,7 +8,6 @@ import type {
   UserGroupType,
   UserGroupsType,
 } from "metabase/admin/types";
-import PopoverWithTrigger from "metabase/common/components/PopoverWithTrigger";
 import Select from "metabase/common/components/Select";
 import CS from "metabase/css/core/index.css";
 import {
@@ -18,7 +18,7 @@ import {
   isDefaultGroup,
 } from "metabase/lib/groups";
 import { isNotNull } from "metabase/lib/types";
-import { Icon } from "metabase/ui";
+import { Icon, Popover } from "metabase/ui";
 
 type GroupSelectProps = {
   groups: UserGroupsType;
@@ -58,6 +58,8 @@ export const GroupSelect = ({
   isCurrentUser = false,
   emptyListMessage = t`No groups`,
 }: GroupSelectProps) => {
+  const [opened, setOpened] = useState(false);
+
   const triggerElement = (
     <div className={cx(CS.flex, CS.alignCenter)}>
       <GroupSummary
@@ -71,9 +73,14 @@ export const GroupSelect = ({
 
   if (groups.length === 0) {
     return (
-      <PopoverWithTrigger triggerElement={triggerElement}>
-        <span className={CS.p1}>{emptyListMessage}</span>
-      </PopoverWithTrigger>
+      <Popover opened={opened} onChange={setOpened}>
+        <Popover.Target>
+          <div onClick={() => setOpened(!opened)}>{triggerElement}</div>
+        </Popover.Target>
+        <Popover.Dropdown>
+          <span className={CS.p1}>{emptyListMessage}</span>
+        </Popover.Dropdown>
+      </Popover>
     );
   }
 

frontend/src/metabase/common/components/AutocompleteInput/AutocompleteInput.tsx

@@ -1,10 +1,10 @@
 import type * as React from "react";
-import { useMemo, useRef } from "react";
+import { useMemo, useRef, useState } from "react";
 
-import TippyPopoverWithTrigger from "metabase/common/components/PopoverWithTrigger/TippyPopoverWithTrigger";
 import SelectList from "metabase/common/components/SelectList";
 import { useListKeyboardNavigation } from "metabase/common/hooks/use-list-keyboard-navigation";
 import { composeEventHandlers } from "metabase/lib/compose-event-handlers";
+import { Popover } from "metabase/ui";
 import type { VisualizationSettings } from "metabase-types/api";
 
 import type { InputProps } from "../Input";
@@ -46,6 +46,7 @@ const AutocompleteInput = ({
   onChangeSettings,
   ...rest
 }: AutocompleteInputProps) => {
+  const [opened, setOpened] = useState(false);
   const optionsListRef = useRef<HTMLUListElement>(null);
   const inputRef = useRef<HTMLDivElement | null>(null);
   const filteredOptions = useMemo(() => {
@@ -71,16 +72,31 @@ const AutocompleteInput = ({
     } else {
       onChange(option);
     }
+    setOpened(false);
   };
 
   const handleChange: InputProps["onChange"] = (e) => {
     onChange(e.target.value);
   };
 
+  const handleShowPopover = () => {
+    if (filteredOptions.length > 0) {
+      setOpened(true);
+    }
+  };
+
+  const handleClosePopover = () => {
+    setOpened(false);
+  };
+
   return (
-    <TippyPopoverWithTrigger
-      sizeToFit
-      renderTrigger={({ onClick: handleShowPopover, closePopover }) => (
+    <Popover
+      opened={opened && filteredOptions.length > 0}
+      onChange={setOpened}
+      position="bottom-start"
+      width="target"
+    >
+      <Popover.Target>
         <Input
           ref={inputRef}
           role="combobox"
@@ -95,36 +111,28 @@ const AutocompleteInput = ({
           onChange={composeEventHandlers(handleChange, handleShowPopover)}
           onBlur={composeEventHandlers<React.FocusEvent<HTMLInputElement>>(
             onBlur,
-            closePopover,
+            handleClosePopover,
           )}
         />
-      )}
-      placement="bottom-start"
-      popoverContent={({ closePopover }) => {
-        if (filteredOptions.length === 0) {
-          return null;
-        }
-
-        return (
-          <OptionsList ref={optionsListRef} onMouseDown={handleListMouseDown}>
-            {filteredOptions.map((item, index) => (
-              <SelectList.Item
-                isSelected={cursorIndex === index}
-                key={item}
-                id={item}
-                name={item}
-                onSelect={(item) => {
-                  handleOptionSelect(String(item));
-                  closePopover();
-                }}
-              >
-                {item}
-              </SelectList.Item>
-            ))}
-          </OptionsList>
-        );
-      }}
-    />
+      </Popover.Target>
+      <Popover.Dropdown p={0}>
+        <OptionsList ref={optionsListRef} onMouseDown={handleListMouseDown}>
+          {filteredOptions.map((item, index) => (
+            <SelectList.Item
+              isSelected={cursorIndex === index}
+              key={item}
+              id={item}
+              name={item}
+              onSelect={(item) => {
+                handleOptionSelect(String(item));
+              }}
+            >
+              {item}
+            </SelectList.Item>
+          ))}
+        </OptionsList>
+      </Popover.Dropdown>
+    </Popover>
   );
 };
 

frontend/src/metabase/common/components/ColorPicker/ColorPicker.tsx

@@ -1,7 +1,7 @@
 import type { HTMLAttributes, Ref } from "react";
-import { forwardRef } from "react";
+import { forwardRef, useState } from "react";
 
-import TippyPopoverWithTrigger from "metabase/common/components/PopoverWithTrigger/TippyPopoverWithTrigger";
+import { Popover } from "metabase/ui";
 
 import ColorPickerContent from "./ColorPickerContent";
 import ColorPickerTrigger from "./ColorPickerTrigger";
@@ -22,22 +22,25 @@ const ColorPicker = forwardRef(function ColorPicker(
   { value, placeholder, isAuto, onChange, ...props }: ColorPickerProps,
   ref: Ref<HTMLDivElement>,
 ) {
+  const [opened, setOpened] = useState(false);
+
   return (
-    <TippyPopoverWithTrigger
-      disableContentSandbox
-      renderTrigger={({ onClick }) => (
+    <Popover opened={opened} onChange={setOpened}>
+      <Popover.Target>
         <ColorPickerTrigger
           {...props}
           ref={ref}
           value={value}
           placeholder={placeholder}
           isAuto={isAuto}
-          onClick={onClick}
+          onClick={() => setOpened(!opened)}
           onChange={onChange}
         />
-      )}
-      popoverContent={<ColorPickerContent value={value} onChange={onChange} />}
-    />
+      </Popover.Target>
+      <Popover.Dropdown>
+        <ColorPickerContent value={value} onChange={onChange} />
+      </Popover.Dropdown>
+    </Popover>
   );
 });
 

frontend/src/metabase/common/components/ColorPicker/ColorPillPicker.tsx

@@ -1,8 +1,7 @@
 import { useDebouncedCallback } from "@mantine/hooks";
-import { type Ref, forwardRef } from "react";
+import { type Ref, forwardRef, useState } from "react";
 
-import TippyPopoverWithTrigger from "metabase/common/components/PopoverWithTrigger/TippyPopoverWithTrigger";
-import { Group } from "metabase/ui";
+import { Group, Popover } from "metabase/ui";
 
 import { ColorPill } from "../ColorPill";
 
@@ -43,18 +42,18 @@ export const ColorPillPicker = forwardRef(function ColorPillPicker(
   }: ColorPillPickerProps,
   ref: Ref<HTMLDivElement>,
 ) {
+  const [opened, setOpened] = useState(false);
   const debouncedUpdate = useDebouncedCallback(onChange, debounceMs);
   const color = previewValue ?? originalColor;
 
   return (
-    <TippyPopoverWithTrigger
-      disableContentSandbox
-      renderTrigger={({ onClick }) => (
+    <Popover opened={opened} onChange={setOpened}>
+      <Popover.Target>
         <Group {...props} ref={ref} wrap="nowrap">
-          <ColorPill color={color} onClick={onClick} />
+          <ColorPill color={color} onClick={() => setOpened(!opened)} />
         </Group>
-      )}
-      popoverContent={
+      </Popover.Target>
+      <Popover.Dropdown className={S.ColorPillPicker}>
         <ColorPickerContent
           value={color}
           onChange={(nextColor) => {
@@ -65,8 +64,7 @@ export const ColorPillPicker = forwardRef(function ColorPillPicker(
             debouncedUpdate(colorWithDefault);
           }}
         />
-      }
-      className={S.ColorPillPicker}
-    />
+      </Popover.Dropdown>
+    </Popover>
   );
 });

frontend/src/metabase/common/components/TabButton/TabButton.tsx

@@ -21,9 +21,9 @@ import {
 } from "react";
 import { t } from "ttag";
 
-import ControlledPopoverWithTrigger from "metabase/common/components/PopoverWithTrigger/ControlledPopoverWithTrigger";
 import { useTranslateContent } from "metabase/i18n/hooks";
 import { lighten } from "metabase/lib/colors";
+import { Popover } from "metabase/ui";
 
 import type { TabContextType } from "../Tab";
 import {
@@ -157,29 +157,30 @@ const _TabButton = forwardRef(function TabButton(
         />
       </TabButtonInputWrapper>
       {showMenu && (
-        <ControlledPopoverWithTrigger
-          visible={isMenuOpen}
-          onOpen={() => setIsMenuOpen(true)}
-          onClose={() => setIsMenuOpen(false)}
-          renderTrigger={({ onClick }) => (
+        <Popover
+          opened={isMenuOpen}
+          onChange={setIsMenuOpen}
+          position="bottom-start"
+        >
+          <Popover.Target>
             <MenuButton
               icon="chevrondown"
               iconSize={10}
               isSelected={isSelected}
               isOpen={isMenuOpen}
-              onClick={onClick}
+              onClick={() => setIsMenuOpen(!isMenuOpen)}
               ref={menuButtonRef}
               disabled={disabled}
             />
-          )}
-          popoverContent={({ closePopover }) => (
+          </Popover.Target>
+          <Popover.Dropdown>
             <TabButtonMenu
               menuItems={menuItems}
               value={value}
-              closePopover={closePopover}
+              closePopover={() => setIsMenuOpen(false)}
             />
-          )}
-        />
+          </Popover.Dropdown>
+        </Popover>
       )}
     </TabButtonRoot>
   );