Tidy types

e2e/test/scenarios/admin-2/sso/shared/group-mappings-widget.js

@@ -87,13 +87,8 @@ const addGroupsToMapping = (mappingName, groups) => {
 
   groups.forEach((group) => {
     popover().within(() => {
-      cy.findByText(group).click();
-
-      cy.findByText(group)
-        .closest("[data-element-id=list-section]")
-        .within(() => {
-          cy.icon("check");
-        });
+      cy.findByLabelText(group).click();
+      cy.findByLabelText(group).should("be.checked");
     });
   });
 

enterprise/frontend/src/metabase-enterprise/auth/components/SettingsJWTForm/SettingsJWTForm.tsx

@@ -156,7 +156,7 @@ export const SettingsJWTForm = () => {
               </FormSection>
               <FormSection title={"Group Sync"} data-testid="jwt-group-schema">
                 <GroupMappingsWidget
-                  setting={{ key: "jwt-group-sync" }}
+                  settingKey="jwt-group-sync"
                   onChange={handleSubmit}
                   settingValues={settingValues}
                   mappingSetting="jwt-group-mappings"

enterprise/frontend/src/metabase-enterprise/auth/components/SettingsSAMLForm/SettingsSAMLForm.tsx

@@ -253,7 +253,7 @@ export function SettingsSAMLForm() {
                   <GroupMappingsWidget
                     isFormik
                     // map to legacy setting props
-                    setting={{ key: "saml-group-sync" }}
+                    settingKey="saml-group-sync"
                     onChange={handleSubmit}
                     settingValues={settingValues}
                     mappingSetting="saml-group-mappings"

frontend/src/metabase-types/api/mocks/settings.ts

@@ -291,6 +291,7 @@ export const createMockSettings = (
   "jwt-attribute-groups": null,
   "jwt-attribute-tenant": null,
   "jwt-group-sync": false,
+  "jwt-group-mappings": null,
   "ldap-configured?": false,
   "ldap-enabled": false,
   "ldap-port": 389, // default value from API

frontend/src/metabase-types/api/settings.ts

@@ -709,6 +709,7 @@ export interface EnterpriseSettings extends Settings {
   "jwt-attribute-groups": string | null;
   "jwt-attribute-tenant": string | null;
   "jwt-group-sync": boolean | null;
+  "jwt-group-mappings": Record<string, GroupId[]> | null;
   "saml-enabled": boolean;
   "saml-configured": boolean;
   "saml-user-provisioning-enabled?": boolean;

frontend/src/metabase/admin/settings/components/SettingsLdapForm.tsx

@@ -202,7 +202,7 @@ export const SettingsLdapForm = () => {
                 <Stack gap="md">
                   <GroupMappingsWidget
                     isFormik
-                    setting={{ key: "ldap-group-sync" }}
+                    settingKey="ldap-group-sync"
                     onChange={handleSubmit}
                     settingValues={settingValues}
                     mappingSetting="ldap-group-mappings"

frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.styled.tsx

@@ -1,14 +0,0 @@
-// eslint-disable-next-line no-restricted-imports
-import styled from "@emotion/styled";
-
-export const ModalHeader = styled.h2`
-  padding: var(--mantine-spacing-xl);
-`;
-
-export const ModalSubtitle = styled.p`
-  padding: 0 var(--mantine-spacing-xl);
-`;
-
-export const ModalRadioRoot = styled.div`
-  padding: var(--mantine-spacing-xs) var(--mantine-spacing-xl);
-`;

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

@@ -5,17 +5,7 @@ import type {
   DeleteMappingModalValueType,
   GroupIds,
 } from "metabase/admin/types";
-import { Button } from "metabase/common/components/Button";
-import { Modal } from "metabase/common/components/Modal";
-import { ModalFooter } from "metabase/common/components/ModalContent";
-import { Radio } from "metabase/common/components/Radio";
-import CS from "metabase/css/core/index.css";
-
-import {
-  ModalHeader,
-  ModalRadioRoot,
-  ModalSubtitle,
-} from "./DeleteGroupMappingModal.styled";
+import { Button, Flex, Modal, Radio, Stack, Text } from "metabase/ui";
 
 export type DeleteGroupMappingModalProps = {
   name: string;
@@ -28,7 +18,7 @@ export type DeleteGroupMappingModalProps = {
   onHide: () => void;
 };
 
-const DeleteGroupMappingModal = ({
+export const DeleteGroupMappingModal = ({
   name,
   groupIds,
   onConfirm,
@@ -36,8 +26,8 @@ const DeleteGroupMappingModal = ({
 }: DeleteGroupMappingModalProps) => {
   const [value, setValue] = useState<DeleteMappingModalValueType>("nothing");
 
-  const handleChange = (newValue: DeleteMappingModalValueType) => {
-    setValue(newValue);
+  const handleChange = (newValue: string) => {
+    setValue(newValue as DeleteMappingModalValueType);
   };
 
   const handleConfirm = () => {
@@ -64,48 +54,44 @@ const DeleteGroupMappingModal = ({
       : t`What should happen with the group itself in Metabase?`;
 
   return (
-    <Modal>
-      <div>
-        <ModalHeader>{t`Remove this group mapping?`}</ModalHeader>
-        <ModalSubtitle>{subtitle}</ModalSubtitle>
-        <ModalRadioRoot>
-          <p>{whatShouldHappenText}</p>
+    <Modal
+      title={t`Remove this group mapping?`}
+      opened
+      onClose={onHide}
+      size="lg"
+    >
+      <Stack gap="md" mt="md">
+        <Text>{subtitle}</Text>
+        <Text fw={500}>{whatShouldHappenText}</Text>
 
-          <Radio
-            className={CS.ml2}
-            vertical
-            value={value as DeleteMappingModalValueType | undefined}
-            options={[
-              {
-                name: t`Nothing, just remove the mapping`,
-                value: "nothing",
-              },
-              {
-                name: t`Also remove all group members (except from Admin)`,
-                value: "clear",
-              },
-              {
-                name:
-                  groupIds.length > 1
-                    ? t`Also delete the groups (except Admin)`
-                    : t`Also delete the group`,
-                value: "delete",
-              },
-            ]}
-            showButtons
-            onChange={handleChange}
-          />
-        </ModalRadioRoot>
-        <ModalFooter fullPageModal={false} formModal={true}>
+        <Radio.Group value={value} onChange={handleChange}>
+          <Stack gap="sm" ml="md">
+            <Radio
+              value="nothing"
+              label={t`Nothing, just remove the mapping`}
+            />
+            <Radio
+              value="clear"
+              label={t`Also remove all group members (except from Admin)`}
+            />
+            <Radio
+              value="delete"
+              label={
+                groupIds.length > 1
+                  ? t`Also delete the groups (except Admin)`
+                  : t`Also delete the group`
+              }
+            />
+          </Stack>
+        </Radio.Group>
+
+        <Flex justify="flex-end" gap="md" mt="md">
           <Button onClick={onHide}>{t`Cancel`}</Button>
-          <Button danger onClick={handleConfirm}>
-            {submitButtonLabels[value as DeleteMappingModalValueType]}
+          <Button color="danger" variant="filled" onClick={handleConfirm}>
+            {submitButtonLabels[value]}
           </Button>
-        </ModalFooter>
-      </div>
+        </Flex>
+      </Stack>
     </Modal>
   );
 };
-
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default DeleteGroupMappingModal;

frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/DeleteGroupMappingModal.unit.spec.tsx

@@ -2,8 +2,10 @@ import userEvent from "@testing-library/user-event";
 
 import { render, screen } from "__support__/ui";
 
-import type { DeleteGroupMappingModalProps } from "./DeleteGroupMappingModal";
-import DeleteGroupMappingModal from "./DeleteGroupMappingModal";
+import {
+  DeleteGroupMappingModal,
+  type DeleteGroupMappingModalProps,
+} from "./DeleteGroupMappingModal";
 
 type SetupOpts = Partial<DeleteGroupMappingModalProps>;
 

frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/DeleteGroupMappingModal/index.tsx

@@ -1,2 +1 @@
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export { default } from "./DeleteGroupMappingModal";
+export * from "./DeleteGroupMappingModal";

frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.module.css

@@ -0,0 +1,26 @@
+.root {
+  border: 1px solid var(--mb-color-border);
+  border-radius: 8px;
+  display: flex;
+  flex-direction: column;
+  max-width: 720px;
+  width: 100%;
+}
+
+.header {
+  background-color: var(--mb-color-bg-light);
+  border-top-left-radius: 8px;
+  border-top-right-radius: 8px;
+  margin-bottom: var(--mantine-spacing-md);
+  padding: var(--mantine-spacing-sm) var(--mantine-spacing-md);
+}
+
+.addMappingButton {
+  float: right;
+  margin-right: var(--mantine-spacing-md);
+  margin-bottom: -40px;
+}
+
+.tableColumn {
+  width: 50%;
+}

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

@@ -1,66 +0,0 @@
-// eslint-disable-next-line no-restricted-imports
-import styled from "@emotion/styled";
-
-import { Button } from "metabase/common/components/Button";
-
-export const GroupMappingsWidgetAndErrorRoot = styled.div`
-  display: flex;
-  flex-direction: column;
-  width: 100%;
-`;
-
-export const GroupMappingsWidgetRoot = styled.div`
-  border: 1px solid var(--mb-color-border);
-  border-radius: 8px;
-  display: flex;
-  flex-direction: column;
-  max-width: 720px;
-  width: 100%;
-`;
-
-export const GroupMappingsWidgetHeader = styled.div`
-  background-color: var(--mb-color-background-secondary);
-  border-top-left-radius: 8px;
-  border-top-right-radius: 8px;
-  display: flex;
-  justify-content: space-between;
-  margin-bottom: var(--mantine-spacing-md);
-  padding: var(--mantine-spacing-sm) var(--mantine-spacing-md);
-
-  span {
-    font-weight: 700;
-  }
-`;
-
-export const GroupMappingsWidgetToggleRoot = styled.div`
-  align-items: center;
-  display: flex;
-
-  > * {
-    color: var(--mb-color-text-primary);
-    padding-right: var(--mantine-spacing-sm);
-    padding-top: 0;
-  }
-`;
-
-export const GroupMappingsWidgetAbout = styled.div`
-  align-items: center;
-  color: var(--mb-color-text-secondary);
-  display: flex;
-  flex-direction: row;
-
-  span {
-    padding-left: var(--mantine-spacing-sm);
-  }
-`;
-
-export const GroupMappingsWidgetAboutContentRoot = styled.div`
-  display: flex;
-  align-items: center;
-`;
-
-export const AddMappingButton = styled(Button)`
-  float: right;
-  margin-right: var(--mantine-spacing-md);
-  margin-bottom: -40px;
-`;

frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidget.unit.spec.jsx

@@ -22,24 +22,23 @@ const setup = ({
   clearGroupMember = jest.fn(),
   deleteGroup = jest.fn(),
   updateSetting = jest.fn(),
-  onSuccess = jest.fn(),
-  setting = { key: "key", value: true },
+  settingKey = "ldap-group-sync",
+  initialSyncEnabled = true,
   groups = defaultGroups,
 } = {}) => {
   render(
     <FormProvider
-      initialValues={{ [setting.key]: setting.value }}
+      initialValues={{ [settingKey]: initialSyncEnabled }}
       onSubmit={() => {}}
     >
       <GroupMappingsWidgetView
         allGroups={groups}
         mappings={mappings}
         mappingSetting={mappingSetting}
-        setting={setting}
+        settingKey={settingKey}
         clearGroupMember={clearGroupMember}
         deleteGroup={deleteGroup}
         updateSetting={updateSetting}
-        onSuccess={onSuccess}
       />
     </FormProvider>,
   );
@@ -66,11 +65,9 @@ describe("GroupMappingsWidgetView", () => {
       const aboutMappingsElement = await screen.findByText("About mappings");
       await userEvent.hover(aboutMappingsElement);
 
-      expect(
-        await screen.findByText(
-          /Mappings allow Metabase to automatically add and remove users from groups based on the membership information provided by the directory server\. If a group isn‘t mapped, its membership won‘t be synced\./,
-        ),
-      ).toBeInTheDocument();
+      expect(await screen.findByRole("tooltip")).toHaveTextContent(
+        /Mappings allow Metabase to automatically add and remove users from groups based on the membership information provided by the directory server/,
+      );
     });
   });
 
@@ -79,7 +76,8 @@ describe("GroupMappingsWidgetView", () => {
       setup({
         mappingSetting: "jwt-group-mappings",
         mappings: {},
-        setting: { key: "jwt-group-sync", value: true },
+        settingKey: "jwt-group-sync",
+        initialSyncEnabled: true,
       });
 
       expect(
@@ -93,7 +91,8 @@ describe("GroupMappingsWidgetView", () => {
       setup({
         mappingSetting: "jwt-group-mappings",
         mappings: {},
-        setting: { key: "jwt-group-sync", value: false },
+        settingKey: "jwt-group-sync",
+        initialSyncEnabled: false,
       });
 
       expect(

frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/GroupMappingsWidgetView.jsx

@@ -1,194 +0,0 @@
-/* eslint-disable react/prop-types */
-import cx from "classnames";
-import { useField } from "formik";
-import { useState } from "react";
-import { t } from "ttag";
-import _ from "underscore";
-
-import { AdminContentTable } from "metabase/common/components/AdminContentTable";
-import CS from "metabase/css/core/index.css";
-import { FormSwitch } from "metabase/forms";
-import { isDefaultGroup } from "metabase/lib/groups";
-import { Icon, Tooltip } from "metabase/ui";
-
-import AddMappingRow from "./AddMappingRow";
-import {
-  GroupMappingsWidgetAbout as About,
-  GroupMappingsWidgetAboutContentRoot as AboutContentRoot,
-  AddMappingButton,
-  GroupMappingsWidgetHeader as Header,
-  GroupMappingsWidgetRoot as Root,
-  GroupMappingsWidgetToggleRoot as ToggleRoot,
-  GroupMappingsWidgetAndErrorRoot as WidgetAndErrorRoot,
-} from "./GroupMappingsWidget.styled";
-import { MappingRow } from "./MappingRow";
-
-const groupIsMappable = (group) => !isDefaultGroup(group);
-
-const helpText = (mappingSetting) => {
-  if (mappingSetting === "jwt-group-mappings") {
-    return t`Mappings allow Metabase to automatically add and remove users from groups based on the membership information provided by the directory server. If no mappings are defined, groups will automatically be assigned based on exactly matching names.`;
-  }
-  return t`Mappings allow Metabase to automatically add and remove users from groups based on the membership information provided by the directory server. If a group isn‘t mapped, its membership won‘t be synced.`;
-};
-
-const noMappingText = (mappingSetting, syncSwitchValue) => {
-  if (!syncSwitchValue) {
-    return `No mappings yet, group sync is not on`;
-  }
-  if (mappingSetting === "jwt-group-mappings") {
-    return t`No mappings yet, groups will be automatically assigned by exactly matching names`;
-  }
-  return t`No mappings yet`;
-};
-
-export function GroupMappingsWidgetView({
-  groupHeading,
-  groupPlaceholder,
-  allGroups = [],
-  mappingSetting,
-  deleteGroup,
-  clearGroupMember,
-  updateSetting,
-  mappings,
-  ...props
-}) {
-  const [showAddRow, setShowAddRow] = useState(false);
-  const [saveError, setSaveError] = useState({});
-
-  const groups = allGroups.filter(groupIsMappable);
-
-  const handleShowAddRow = () => {
-    setShowAddRow(true);
-  };
-
-  const handleHideAddRow = () => {
-    setShowAddRow(false);
-  };
-
-  const handleAddMapping = async (name) => {
-    const mappingsPlusNewMapping = { ...mappings, [name]: [] };
-
-    try {
-      await updateSetting({
-        key: mappingSetting,
-        value: mappingsPlusNewMapping,
-      });
-      setShowAddRow(false);
-      setSaveError(null);
-    } catch (error) {
-      setSaveError(error);
-    }
-  };
-
-  const handleChangeMapping = (name) => async (group, selected) => {
-    const updatedMappings = selected
-      ? { ...mappings, [name]: [...mappings[name], group.id] }
-      : {
-          ...mappings,
-          [name]: mappings[name].filter((id) => id !== group.id),
-        };
-
-    try {
-      await updateSetting({ key: mappingSetting, value: updatedMappings });
-      setSaveError(null);
-    } catch (error) {
-      setSaveError(error);
-    }
-  };
-
-  const handleDeleteMapping = async ({ name, onSuccess }) => {
-    const mappingsMinusDeletedMapping = _.omit(mappings, name);
-
-    try {
-      await updateSetting({
-        key: mappingSetting,
-        value: mappingsMinusDeletedMapping,
-      });
-
-      onSuccess && (await onSuccess());
-      setSaveError(null);
-    } catch (error) {
-      setSaveError(error);
-    }
-  };
-
-  const [{ value: groupSyncSwitchValue }] = useField(props.setting.key);
-
-  return (
-    <WidgetAndErrorRoot>
-      <Root>
-        <Header>
-          <ToggleRoot>
-            <span>{t`Synchronize Group Memberships`}</span>
-            <FormSwitch
-              data-testid="group-sync-switch"
-              name={props.setting.key}
-            />
-          </ToggleRoot>
-          <About>
-            <Tooltip
-              label={helpText(mappingSetting)}
-              position="top"
-              maw="20rem"
-            >
-              <AboutContentRoot>
-                <Icon name="info" />
-                <span>{t`About mappings`}</span>
-              </AboutContentRoot>
-            </Tooltip>
-          </About>
-        </Header>
-
-        <div>
-          <div>
-            {!showAddRow && (
-              <AddMappingButton primary small onClick={handleShowAddRow}>
-                {t`New mapping`}
-              </AddMappingButton>
-            )}
-            <AdminContentTable columnTitles={[groupHeading, t`Groups`, ""]}>
-              {showAddRow && (
-                <AddMappingRow
-                  mappings={mappings}
-                  placeholder={groupPlaceholder}
-                  onCancel={handleHideAddRow}
-                  onAdd={handleAddMapping}
-                />
-              )}
-              {Object.keys(mappings).length === 0 && !showAddRow && (
-                <tr>
-                  <td>&nbsp;</td>
-                  <td>
-                    {" "}
-                    {noMappingText(mappingSetting, groupSyncSwitchValue)}
-                  </td>
-                  <td>&nbsp;</td>
-                </tr>
-              )}
-              {Object.entries(mappings).map(([name, selectedGroupIds]) => {
-                return groups?.length > 0 ? (
-                  <MappingRow
-                    key={name}
-                    name={name}
-                    groups={groups}
-                    selectedGroupIds={selectedGroupIds}
-                    clearGroupMember={clearGroupMember}
-                    deleteGroup={deleteGroup}
-                    onChange={handleChangeMapping(name)}
-                    onDeleteMapping={handleDeleteMapping}
-                  />
-                ) : null;
-              })}
-            </AdminContentTable>
-          </div>
-        </div>
-      </Root>
-      {saveError?.data?.message && (
-        <div className={cx(CS.textError, CS.textBold, CS.m1)}>
-          {saveError.data.message}
-        </div>
-      )}
-    </WidgetAndErrorRoot>
-  );
-}

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

@@ -0,0 +1,214 @@
+import { useDisclosure } from "@mantine/hooks";
+import { useField } from "formik";
+import { useState } from "react";
+import { t } from "ttag";
+import _ from "underscore";
+
+import type { MappingsType } from "metabase/admin/types";
+import { getErrorMessage } from "metabase/api/utils/errors";
+import { AdminContentTable } from "metabase/common/components/AdminContentTable";
+import { FormSwitch } from "metabase/forms";
+import type { GenericErrorResponse } from "metabase/lib/errors/types";
+import { isDefaultGroup } from "metabase/lib/groups";
+import { Box, Button, Flex, Icon, Stack, Text, Tooltip } from "metabase/ui";
+import type { EnterpriseSettingKey, GroupInfo } from "metabase-types/api";
+
+import AddMappingRow from "./AddMappingRow";
+import S from "./GroupMappingsWidget.module.css";
+import { MappingRow } from "./MappingRow";
+
+const groupIsMappable = (group: GroupInfo) => !isDefaultGroup(group);
+
+const helpText = (mappingSetting: EnterpriseSettingKey) => {
+  if (mappingSetting === "jwt-group-mappings") {
+    return t`Mappings allow Metabase to automatically add and remove users from groups based on the membership information provided by the directory server. If no mappings are defined, groups will automatically be assigned based on exactly matching names.`;
+  }
+  return t`Mappings allow Metabase to automatically add and remove users from groups based on the membership information provided by the directory server. If a group isn't mapped, its membership won't be synced.`;
+};
+
+const noMappingText = (
+  mappingSetting: EnterpriseSettingKey,
+  syncSwitchValue: boolean,
+) => {
+  if (!syncSwitchValue) {
+    return t`No mappings yet, group sync is not on`;
+  }
+  if (mappingSetting === "jwt-group-mappings") {
+    return t`No mappings yet, groups will be automatically assigned by exactly matching names`;
+  }
+  return t`No mappings yet`;
+};
+
+type GroupMappingsWidgetViewProps = {
+  groupHeading: string;
+  groupPlaceholder: string;
+  allGroups?: GroupInfo[];
+  mappingSetting: EnterpriseSettingKey;
+  deleteGroup: (args: { id: number }) => Promise<void>;
+  clearGroupMember: (args: { id: number }) => Promise<void>;
+  updateSetting: (args: { key: string; value: MappingsType }) => Promise<void>;
+  mappings: MappingsType;
+  settingKey: EnterpriseSettingKey;
+};
+
+export function GroupMappingsWidgetView({
+  groupHeading,
+  groupPlaceholder,
+  allGroups = [],
+  mappingSetting,
+  deleteGroup,
+  clearGroupMember,
+  updateSetting,
+  mappings,
+  settingKey,
+}: GroupMappingsWidgetViewProps) {
+  const [showAddRow, { open: openAddRow, close: closeAddRow }] =
+    useDisclosure();
+  const [saveError, setSaveError] = useState<GenericErrorResponse | null>(null);
+
+  const groups = allGroups.filter(groupIsMappable);
+
+  const handleAddMapping = async (name: string) => {
+    const mappingsPlusNewMapping = { ...mappings, [name]: [] };
+
+    try {
+      await updateSetting({
+        key: mappingSetting,
+        value: mappingsPlusNewMapping,
+      });
+      closeAddRow();
+      setSaveError(null);
+    } catch (error) {
+      setSaveError(error as GenericErrorResponse);
+    }
+  };
+
+  const handleChangeMapping =
+    (name: string) => async (group: { id: number }, selected: boolean) => {
+      const updatedMappings = selected
+        ? { ...mappings, [name]: [...mappings[name], group.id] }
+        : {
+            ...mappings,
+            [name]: mappings[name].filter((id) => id !== group.id),
+          };
+
+      try {
+        await updateSetting({ key: mappingSetting, value: updatedMappings });
+        setSaveError(null);
+      } catch (error) {
+        setSaveError(error as GenericErrorResponse);
+      }
+    };
+
+  const handleDeleteMapping = async ({
+    name,
+    onSuccess,
+  }: {
+    name: string;
+    onSuccess?: () => void;
+  }) => {
+    const mappingsMinusDeletedMapping = _.omit(mappings, name);
+
+    try {
+      await updateSetting({
+        key: mappingSetting,
+        value: mappingsMinusDeletedMapping,
+      });
+
+      onSuccess?.();
+      setSaveError(null);
+    } catch (error) {
+      setSaveError(error as GenericErrorResponse);
+    }
+  };
+
+  const [{ value: groupSyncSwitchValue }] = useField(settingKey);
+
+  return (
+    <Stack w="100%" gap={0}>
+      <Box className={S.root}>
+        <Flex justify="space-between" align="center" className={S.header}>
+          <Flex align="center" gap="sm">
+            <Text
+              fw={700}
+              c="text-primary"
+              lh={1.2}
+            >{t`Synchronize Group Memberships`}</Text>
+            <FormSwitch
+              data-testid="group-sync-switch"
+              name={settingKey}
+              pr="sm"
+            />
+          </Flex>
+          <Tooltip label={helpText(mappingSetting)} position="top" maw="20rem">
+            <Flex align="center" lh={1.2} gap="sm" c="text-secondary">
+              <Icon name="info" />
+              <Text c="text-secondary" fw={700}>{t`About mappings`}</Text>
+            </Flex>
+          </Tooltip>
+        </Flex>
+
+        <Box>
+          {!showAddRow && (
+            <Button
+              className={S.addMappingButton}
+              variant="filled"
+              size="sm"
+              onClick={openAddRow}
+            >
+              {t`New mapping`}
+            </Button>
+          )}
+          <AdminContentTable
+            columnTitles={[
+              <span key="name" className={S.tableColumn}>
+                {groupHeading}
+              </span>,
+              <span key="groups" className={S.tableColumn}>
+                {t`Groups`}
+              </span>,
+              "",
+            ]}
+          >
+            {showAddRow && (
+              <AddMappingRow
+                mappings={mappings}
+                placeholder={groupPlaceholder}
+                onCancel={closeAddRow}
+                onAdd={handleAddMapping}
+              />
+            )}
+            {Object.keys(mappings).length === 0 && !showAddRow && (
+              <tr>
+                <td>&nbsp;</td>
+                <td className={S.tableColumn}>
+                  {noMappingText(mappingSetting, groupSyncSwitchValue)}
+                </td>
+                <td>&nbsp;</td>
+              </tr>
+            )}
+            {Object.entries(mappings).map(([name, selectedGroupIds]) => {
+              return groups?.length > 0 ? (
+                <MappingRow
+                  key={name}
+                  name={name}
+                  groups={groups}
+                  selectedGroupIds={selectedGroupIds}
+                  clearGroupMember={clearGroupMember}
+                  deleteGroup={deleteGroup}
+                  onChange={handleChangeMapping(name)}
+                  onDeleteMapping={handleDeleteMapping}
+                />
+              ) : null;
+            })}
+          </AdminContentTable>
+        </Box>
+      </Box>
+      {saveError && (
+        <Text c="error" fw={700} m="sm">
+          {getErrorMessage(saveError)}
+        </Text>
+      )}
+    </Stack>
+  );
+}

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

@@ -1,11 +1,8 @@
-import cx from "classnames";
+import { useDisclosure } from "@mantine/hooks";
 import { t } from "ttag";
 
 import { GroupSummary } from "metabase/admin/people/components/GroupSummary";
-import type { GroupIds, UserGroupType } 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 type { GroupIds } from "metabase/admin/types";
 import {
   canEditMembership,
   getGroupColor,
@@ -14,21 +11,26 @@ import {
   isDefaultGroup,
 } from "metabase/lib/groups";
 import { isNotNull } from "metabase/lib/types";
-import { Icon } from "metabase/ui";
+import { Box, Checkbox, Flex, Icon, Popover, Stack, Text } from "metabase/ui";
 import type { GroupInfo } from "metabase-types/api";
 
 type GroupSelectProps = {
   groups: GroupInfo[];
   selectedGroupIds: GroupIds;
-  onGroupChange: (group: UserGroupType, selected: boolean) => void;
+  onGroupChange: (group: GroupInfo, selected: boolean) => void;
   isCurrentUser?: boolean;
   emptyListMessage?: string;
 };
 
-function getSections(groups: GroupInfo[]) {
+type GroupSection = {
+  name?: string;
+  items: GroupInfo[];
+};
+
+function getSections(groups: GroupInfo[]): GroupSection[] {
   const adminGroup = groups.find(isAdminGroup);
   const defaultGroup = groups.find(isDefaultGroup);
-  const topGroups = [defaultGroup, adminGroup].filter((g) => g != null);
+  const topGroups = [defaultGroup, adminGroup].filter(isNotNull);
   const groupsExceptDefaultAndAdmin = groups.filter(
     (g) => !isAdminGroup(g) && !isDefaultGroup(g),
   );
@@ -37,15 +39,16 @@ function getSections(groups: GroupInfo[]) {
     return [{ items: groupsExceptDefaultAndAdmin }];
   }
 
-  return [
-    { items: topGroups },
-    groupsExceptDefaultAndAdmin.length > 0
-      ? {
-          items: groupsExceptDefaultAndAdmin as any,
-          name: t`Groups`,
-        }
-      : null,
-  ].filter(isNotNull);
+  const sections: GroupSection[] = [{ items: topGroups }];
+
+  if (groupsExceptDefaultAndAdmin.length > 0) {
+    sections.push({
+      items: groupsExceptDefaultAndAdmin,
+      name: t`Groups`,
+    });
+  }
+
+  return sections;
 }
 
 export const GroupSelect = ({
@@ -55,54 +58,81 @@ export const GroupSelect = ({
   isCurrentUser = false,
   emptyListMessage = t`No groups`,
 }: GroupSelectProps) => {
+  const [opened, { toggle }] = useDisclosure(false);
+
+  const handleCheckboxChange = (group: GroupInfo, checked: boolean) => {
+    onGroupChange(group, checked);
+  };
+
+  const isOptionDisabled = (group: GroupInfo) =>
+    (isAdminGroup(group) && isCurrentUser) || !canEditMembership(group);
+
   const triggerElement = (
-    <div className={cx(CS.flex, CS.alignCenter)}>
+    <Flex onClick={toggle} align="center" style={{ cursor: "pointer" }}>
       <GroupSummary
         mr="0.5rem"
         groups={groups}
         selectedGroupIds={selectedGroupIds}
       />
-      <Icon className={CS.textLight} name="chevrondown" size={10} />
-    </div>
+      <Icon c="text-tertiary" name="chevrondown" size={10} />
+    </Flex>
   );
 
   if (groups.length === 0) {
     return (
-      <PopoverWithTrigger triggerElement={triggerElement}>
-        <span className={CS.p1}>{emptyListMessage}</span>
-      </PopoverWithTrigger>
+      <Popover opened={opened} onChange={toggle}>
+        <Popover.Target>{triggerElement}</Popover.Target>
+        <Popover.Dropdown>
+          <Text p="sm" c="text-secondary">
+            {emptyListMessage}
+          </Text>
+        </Popover.Dropdown>
+      </Popover>
     );
   }
 
   const sections = getSections(groups);
 
   return (
-    <Select
-      triggerElement={triggerElement}
-      onChange={({ target: { value } }: { target: { value: any } }) => {
-        groups
-          .filter(
-            // find the differing groups between the new `value` on previous `selectedGroupIds`
-            (group) =>
-              (selectedGroupIds.includes(group.id) as any) ^
-              value.includes(group.id),
-          )
-          .forEach((group) => onGroupChange(group, value.includes(group.id)));
-      }}
-      optionDisabledFn={(group: GroupInfo) =>
-        (isAdminGroup(group) && isCurrentUser) || !canEditMembership(group)
-      }
-      optionValueFn={(group: GroupInfo) => group.id}
-      optionNameFn={getGroupNameLocalized}
-      optionStylesFn={(group: GroupInfo) => ({
-        color: getGroupColor(group),
-      })}
-      value={selectedGroupIds}
-      sections={sections}
-      multiple
-    />
+    <Popover opened={opened} onChange={toggle}>
+      <Popover.Target>{triggerElement}</Popover.Target>
+      <Popover.Dropdown p="sm" w={200}>
+        <Stack gap="xs">
+          {sections.map((section, sectionIndex) => (
+            <Box key={sectionIndex}>
+              {section.name && (
+                <Text size="md" fw={700} c="text-secondary" mb="xs">
+                  {section.name}
+                </Text>
+              )}
+              <Stack gap="xs">
+                {section.items.map((group) => {
+                  const isDisabled = isOptionDisabled(group);
+                  const isChecked = selectedGroupIds.includes(group.id);
+                  const color = getGroupColor(group);
+
+                  return (
+                    <Checkbox
+                      key={group.id}
+                      size="md"
+                      label={
+                        <Text c={color} size="md">
+                          {getGroupNameLocalized(group)}
+                        </Text>
+                      }
+                      checked={isChecked}
+                      disabled={isDisabled}
+                      onChange={(event) =>
+                        handleCheckboxChange(group, event.currentTarget.checked)
+                      }
+                    />
+                  );
+                })}
+              </Stack>
+            </Box>
+          ))}
+        </Stack>
+      </Popover.Dropdown>
+    </Popover>
   );
 };
-
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default GroupSelect;

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

@@ -1,2 +1 @@
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export { default } from "./GroupSelect";
+export * from "./GroupSelect";

frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.module.css

@@ -0,0 +1,7 @@
+.deleteMappingButton {
+  color: var(--mb-color-text-dark);
+}
+
+.deleteMappingButton:hover {
+  color: var(--mb-color-danger);
+}

frontend/src/metabase/admin/settings/components/widgets/GroupMappingsWidget/MappingRow/MappingRow.styled.tsx

@@ -1,12 +0,0 @@
-// eslint-disable-next-line no-restricted-imports
-import styled from "@emotion/styled";
-
-import { IconButtonWrapper } from "metabase/common/components/IconButtonWrapper";
-
-export const DeleteMappingButton = styled(IconButtonWrapper)`
-  color: var(--mb-color-text-primary);
-
-  &:hover {
-    color: var(--mb-color-danger);
-  }
-`;

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

@@ -1,6 +1,5 @@
 import { useDisclosure } from "@mantine/hooks";
 import cx from "classnames";
-import type * as React from "react";
 import { t } from "ttag";
 import _ from "underscore";
 
@@ -9,16 +8,18 @@ import type {
   GroupIds,
 } from "metabase/admin/types";
 import { ConfirmModal } from "metabase/common/components/ConfirmModal";
+import { IconButtonWrapper } from "metabase/common/components/IconButtonWrapper";
 import AdminS from "metabase/css/admin.module.css";
 import CS from "metabase/css/core/index.css";
 import { isAdminGroup } from "metabase/lib/groups";
 import { Icon, Tooltip } from "metabase/ui";
 import type { GroupInfo } from "metabase-types/api";
 
-import DeleteGroupMappingModal from "../DeleteGroupMappingModal";
-import Selectbox from "../GroupSelect";
+import { DeleteGroupMappingModal } from "../DeleteGroupMappingModal/DeleteGroupMappingModal";
+import WidgetS from "../GroupMappingsWidget.module.css";
+import { GroupSelect } from "../GroupSelect";
 
-import { DeleteMappingButton } from "./MappingRow.styled";
+import S from "./MappingRow.module.css";
 
 type OnDeleteMappingType = (arg: {
   name: string;
@@ -32,7 +33,7 @@ type MappingRowProps = {
   selectedGroupIds: GroupIds;
   clearGroupMember: ({ id }: { id: number }) => void;
   deleteGroup: ({ id }: { id: number }) => void;
-  onChange: () => void;
+  onChange: (group: { id: number }, selected: boolean) => void;
   onDeleteMapping: OnDeleteMappingType;
 };
 
@@ -79,7 +80,7 @@ export const MappingRow = ({
   const getCallbackForGroupsAfterDeletingMapping = (
     whatToDoAboutGroups: DeleteMappingModalValueType,
     groupIds: GroupIds,
-  ) => {
+  ): (() => void) | undefined => {
     switch (whatToDoAboutGroups) {
       case "clear":
         return () =>
@@ -88,13 +89,14 @@ export const MappingRow = ({
               try {
                 const group = groups.find((group) => group.id === id);
                 if (group && !isAdminGroup(group)) {
-                  await clearGroupMember({ id });
+                  clearGroupMember({ id });
                 }
               } catch (error) {
                 console.error(error);
               }
             }),
           );
+
       case "delete":
         return () =>
           Promise.all(
@@ -102,15 +104,16 @@ export const MappingRow = ({
               try {
                 const group = groups.find((group) => group.id === id);
                 if (group && !isAdminGroup(group)) {
-                  await deleteGroup({ id });
+                  deleteGroup({ id });
                 }
               } catch (error) {
                 console.error(error);
               }
             }),
           );
+
       default:
-        return () => null;
+        return undefined;
     }
   };
 
@@ -131,9 +134,9 @@ export const MappingRow = ({
   return (
     <>
       <tr>
-        <td>{name}</td>
-        <td>
-          <Selectbox
+        <td className={WidgetS.tableColumn}>{name}</td>
+        <td className={WidgetS.tableColumn}>
+          <GroupSelect
             groups={groups}
             selectedGroupIds={selectedGroupIdsFromGroupsThatExist}
             onGroupChange={onChange}
@@ -141,13 +144,18 @@ export const MappingRow = ({
         </td>
         <td className={AdminS.TableActions}>
           <div className={cx(CS.floatRight, CS.mr1)}>
-            <DeleteButton
-              onDelete={() =>
-                shouldUseDeleteGroupMappingModal
-                  ? openDeleteGroupMappingModal()
-                  : openDeleteMappingModal()
-              }
-            />
+            <Tooltip label={t`Remove mapping`} position="top">
+              <IconButtonWrapper
+                className={S.deleteMappingButton}
+                onClick={() =>
+                  shouldUseDeleteGroupMappingModal
+                    ? openDeleteGroupMappingModal()
+                    : openDeleteMappingModal()
+                }
+              >
+                <Icon name="close" />
+              </IconButtonWrapper>
+            </Tooltip>
           </div>
         </td>
       </tr>
@@ -171,15 +179,3 @@ export const MappingRow = ({
     </>
   );
 };
-
-const DeleteButton = ({
-  onDelete,
-}: {
-  onDelete?: React.MouseEventHandler<HTMLButtonElement>;
-}) => (
-  <Tooltip label={t`Remove mapping`} position="top">
-    <DeleteMappingButton onClick={onDelete}>
-      <Icon name="close" />
-    </DeleteMappingButton>
-  </Tooltip>
-);