From a4522812fea5a02a04fda1099b003e8d50ee3e58 Mon Sep 17 00:00:00 2001
From: Alex Khomenko <Clarity-89@users.noreply.github.com>
Date: Tue, 17 Oct 2023 13:06:28 +0200
Subject: [PATCH] Admin: Use primitive components for table views (#76512)

* Remove HorizontalGroup and VerticalGroup from OrgUserstable

* Refactor OrgUnits

* Refactor UsersTable

* Add TableWrapper

* Use Stack and Flex for TeamList

* Revert pagination changes

* Update betterer

* Remove div wrapper

* Codeformat
---
 .betterer.results                             |  5 -
 public/app/features/admin/Users/OrgUnits.tsx  | 54 ++++-------
 .../features/admin/Users/OrgUsersTable.tsx    | 72 +++++---------
 .../app/features/admin/Users/TableWrapper.tsx | 30 ++++++
 .../app/features/admin/Users/UsersTable.tsx   | 94 +++++++------------
 public/app/features/teams/TeamList.tsx        | 38 ++------
 6 files changed, 112 insertions(+), 181 deletions(-)
 create mode 100644 public/app/features/admin/Users/TableWrapper.tsx

diff --git a/.betterer.results b/.betterer.results
index ebc179e8f69..051b248f929 100644
--- a/.betterer.results
+++ b/.betterer.results
@@ -1761,11 +1761,6 @@ exports[`better eslint`] = {
     "public/app/features/admin/UserSessions.tsx:5381": [
       [0, 0, 0, "Styles should be written using objects.", "0"]
     ],
-    "public/app/features/admin/Users/OrgUnits.tsx:5381": [
-      [0, 0, 0, "Styles should be written using objects.", "0"],
-      [0, 0, 0, "Styles should be written using objects.", "1"],
-      [0, 0, 0, "Styles should be written using objects.", "2"]
-    ],
     "public/app/features/alerting/AlertTab.tsx:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"],
diff --git a/public/app/features/admin/Users/OrgUnits.tsx b/public/app/features/admin/Users/OrgUnits.tsx
index d3ed21c87c4..ae84b3e5d8b 100644
--- a/public/app/features/admin/Users/OrgUnits.tsx
+++ b/public/app/features/admin/Users/OrgUnits.tsx
@@ -1,15 +1,13 @@
-import { css } from '@emotion/css';
-import React from 'react';
+import React, { forwardRef, PropsWithChildren } from 'react';
 
-import { GrafanaTheme2, IconName } from '@grafana/data';
-import { Icon, Tooltip, useStyles2 } from '@grafana/ui';
+import { IconName } from '@grafana/data';
+import { Icon, Tooltip } from '@grafana/ui';
+import { Box, Flex } from '@grafana/ui/src/unstable';
 import { Unit } from 'app/types';
 
 type OrgUnitProps = { units?: Unit[]; icon: IconName };
 
 export const OrgUnits = ({ units, icon }: OrgUnitProps) => {
-  const styles = useStyles2(getStyles);
-
   if (!units?.length) {
     return null;
   }
@@ -17,39 +15,25 @@ export const OrgUnits = ({ units, icon }: OrgUnitProps) => {
   return units.length > 1 ? (
     <Tooltip
       placement={'top'}
-      content={
-        <div className={styles.unitTooltip}>{units?.map((unit) => <span key={unit.name}>{unit.name}</span>)}</div>
-      }
+      content={<Flex direction={'column'}>{units?.map((unit) => <span key={unit.name}>{unit.name}</span>)}</Flex>}
     >
-      <div className={styles.unitItem}>
-        <Icon name={icon} /> <span>{units.length}</span>
-      </div>
+      <Content icon={icon}>{units.length}</Content>
     </Tooltip>
   ) : (
-    <span className={styles.unitItem}>
-      <Icon name={icon} /> {units[0].name}
-    </span>
+    <Content icon={icon}>{units[0].name}</Content>
   );
 };
 
-const getStyles = (theme: GrafanaTheme2) => {
-  return {
-    unitTooltip: css`
-      display: flex;
-      flex-direction: column;
-    `,
-    unitItem: css`
-      padding: ${theme.spacing(0.5)} 0;
-      margin-right: ${theme.spacing(1)};
+interface ContentProps extends PropsWithChildren {
+  icon: IconName;
+}
 
-      svg {
-        margin-bottom: ${theme.spacing(0.25)};
-      }
-    `,
-    link: css`
-      color: inherit;
-      cursor: pointer;
-      text-decoration: underline;
-    `,
-  };
-};
+export const Content = forwardRef<HTMLElement, ContentProps>(({ children, icon }, ref) => {
+  return (
+    <Box ref={ref} display={'flex'} alignItems={'center'} marginRight={1}>
+      <Icon name={icon} /> <Box marginLeft={1}>{children}</Box>
+    </Box>
+  );
+});
+
+Content.displayName = 'TooltipContent';
diff --git a/public/app/features/admin/Users/OrgUsersTable.tsx b/public/app/features/admin/Users/OrgUsersTable.tsx
index 84a30b5ceb9..bc4aceb65f3 100644
--- a/public/app/features/admin/Users/OrgUsersTable.tsx
+++ b/public/app/features/admin/Users/OrgUsersTable.tsx
@@ -1,7 +1,6 @@
-import { css } from '@emotion/css';
 import React, { useEffect, useMemo, useState } from 'react';
 
-import { GrafanaTheme2, OrgRole } from '@grafana/data';
+import { OrgRole } from '@grafana/data';
 import { selectors as e2eSelectors } from '@grafana/e2e-selectors';
 import {
   Button,
@@ -9,16 +8,14 @@ import {
   Icon,
   Tooltip,
   CellProps,
-  useStyles2,
   Tag,
   InteractiveTable,
   Column,
   FetchDataFunc,
   Pagination,
-  HorizontalGroup,
-  VerticalGroup,
   Avatar,
 } from '@grafana/ui';
+import { Flex, Stack, Box } from '@grafana/ui/src/unstable';
 import { UserRolePicker } from 'app/core/components/RolePicker/UserRolePicker';
 import { fetchRoleOptions } from 'app/core/components/RolePicker/api';
 import { TagBadge } from 'app/core/components/TagFilter/TagBadge';
@@ -28,6 +25,8 @@ import { AccessControlAction, OrgUser, Role } from 'app/types';
 
 import { OrgRolePicker } from '../OrgRolePicker';
 
+import { TableWrapper } from './TableWrapper';
+
 type Cell<T extends keyof OrgUser = keyof OrgUser> = CellProps<OrgUser, OrgUser[T]>;
 
 const disabledRoleMessage = `This user's role is not editable because it is synchronized from your auth provider.
@@ -71,7 +70,6 @@ export const OrgUsersTable = ({
 }: Props) => {
   const [userToRemove, setUserToRemove] = useState<OrgUser | null>(null);
   const [roleOptions, setRoleOptions] = useState<Role[]>([]);
-  const styles = useStyles2(getStyles);
 
   useEffect(() => {
     async function fetchOptions() {
@@ -148,7 +146,18 @@ export const OrgUsersTable = ({
       {
         id: 'info',
         header: '',
-        cell: InfoCell,
+        cell: ({ row: { original } }: Cell) => {
+          const basicRoleDisabled = getBasicRoleDisabled(original);
+          return (
+            basicRoleDisabled && (
+              <Box display={'flex'} alignItems={'center'} marginLeft={1}>
+                <Tooltip content={disabledRoleMessage}>
+                  <Icon name="question-circle" />
+                </Tooltip>
+              </Box>
+            )
+          );
+        },
       },
       {
         id: 'authLabels',
@@ -186,18 +195,18 @@ export const OrgUsersTable = ({
   );
 
   return (
-    <VerticalGroup spacing="md" data-testid={selectors.container}>
-      <div className={styles.wrapper}>
+    <Stack gap={2} data-testid={selectors.container}>
+      <TableWrapper>
         <InteractiveTable
           columns={columns}
           data={users}
           getRowId={(user) => String(user.userId)}
           fetchData={fetchData}
         />
-        <HorizontalGroup justify="flex-end">
+        <Flex justifyContent="flex-end">
           <Pagination onNavigate={changePage} currentPage={page} numberOfPages={totalPages} hideWhenSinglePage={true} />
-        </HorizontalGroup>
-      </div>
+        </Flex>
+      </TableWrapper>
       {Boolean(userToRemove) && (
         <ConfirmModal
           body={`Are you sure you want to delete user ${userToRemove?.login}?`}
@@ -216,43 +225,6 @@ export const OrgUsersTable = ({
           }}
         />
       )}
-    </VerticalGroup>
+    </Stack>
   );
 };
-
-const InfoCell = ({ row: { original } }: Cell) => {
-  const styles = useStyles2(getStyles);
-  const basicRoleDisabled = getBasicRoleDisabled(original);
-  return (
-    basicRoleDisabled && (
-      <div className={styles.row}>
-        <Tooltip content={disabledRoleMessage}>
-          <Icon name="question-circle" className={styles.icon} />
-        </Tooltip>
-      </div>
-    )
-  );
-};
-
-const getStyles = (theme: GrafanaTheme2) => ({
-  row: css({
-    display: 'flex',
-    alignItems: 'center',
-  }),
-  icon: css({
-    marginLeft: theme.spacing(1),
-  }),
-  // Enable RolePicker overflow
-  wrapper: css({
-    display: 'flex',
-    flexDirection: 'column',
-    overflowX: 'auto',
-    overflowY: 'hidden',
-    minHeight: '100vh',
-    width: '100%',
-    '& > div': {
-      overflowX: 'unset',
-      marginBottom: theme.spacing(2),
-    },
-  }),
-});
diff --git a/public/app/features/admin/Users/TableWrapper.tsx b/public/app/features/admin/Users/TableWrapper.tsx
new file mode 100644
index 00000000000..ea3fd28809d
--- /dev/null
+++ b/public/app/features/admin/Users/TableWrapper.tsx
@@ -0,0 +1,30 @@
+import { css } from '@emotion/css';
+import React, { PropsWithChildren } from 'react';
+
+import { GrafanaTheme2 } from '@grafana/data';
+import { useStyles2 } from '@grafana/ui';
+
+/**
+ * A wrapper component for interactive tables using RolePicker to enable overflow.
+ * Should be removed when the RolePicker component uses portals to render its menu
+ */
+export const TableWrapper = ({ children }: PropsWithChildren) => {
+  const styles = useStyles2(getStyles);
+  return <div className={styles.wrapper}>{children}</div>;
+};
+
+const getStyles = (theme: GrafanaTheme2) => ({
+  // Enable RolePicker overflow
+  wrapper: css({
+    display: 'flex',
+    flexDirection: 'column',
+    overflowX: 'auto',
+    overflowY: 'hidden',
+    minHeight: '100vh',
+    width: '100%',
+    '& > div': {
+      overflowX: 'unset',
+      marginBottom: theme.spacing(2),
+    },
+  }),
+});
diff --git a/public/app/features/admin/Users/UsersTable.tsx b/public/app/features/admin/Users/UsersTable.tsx
index 1f9ec1b7e4c..209625698cd 100644
--- a/public/app/features/admin/Users/UsersTable.tsx
+++ b/public/app/features/admin/Users/UsersTable.tsx
@@ -1,21 +1,18 @@
-import { css } from '@emotion/css';
 import React, { useMemo } from 'react';
 
-import { GrafanaTheme2 } from '@grafana/data';
 import {
   InteractiveTable,
   CellProps,
   Tooltip,
   Icon,
-  useStyles2,
   Tag,
   Pagination,
   Column,
-  VerticalGroup,
-  HorizontalGroup,
   FetchDataFunc,
+  Text,
   Avatar,
 } from '@grafana/ui';
+import { Flex, Stack } from '@grafana/ui/src/unstable';
 import { TagBadge } from 'app/core/components/TagFilter/TagBadge';
 import { UserDTO } from 'app/types';
 
@@ -69,7 +66,18 @@ export const UsersTable = ({
       {
         id: 'orgs',
         header: 'Belongs to',
-        cell: OrgUnitsCell,
+        cell: ({ cell: { value, row } }: Cell<'orgs'>) => {
+          return (
+            <Flex alignItems={'center'}>
+              <OrgUnits units={value} icon={'building'} />
+              {row.original.isAdmin && (
+                <Tooltip placement="top" content="Grafana Admin">
+                  <Icon name="shield" />
+                </Tooltip>
+              )}
+            </Flex>
+          );
+        },
         sortType: (a, b) => (a.original.orgs?.length || 0) - (b.original.orgs?.length || 0),
       },
       ...(showLicensedRole
@@ -77,7 +85,18 @@ export const UsersTable = ({
             {
               id: 'licensedRole',
               header: 'Licensed role',
-              cell: LicensedRoleCell,
+              cell: ({ cell: { value } }: Cell<'licensedRole'>) => {
+                return value === 'None' ? (
+                  <Text color={'disabled'}>
+                    Not assigned{' '}
+                    <Tooltip placement="top" content="A licensed role will be assigned when this user signs in">
+                      <Icon name="question-circle" />
+                    </Tooltip>
+                  </Text>
+                ) : (
+                  value
+                );
+              },
               // Needs the assertion here, the types are not inferred correctly due to the  conditional assignment
               sortType: 'string' as const,
             },
@@ -90,7 +109,9 @@ export const UsersTable = ({
           content: 'Time since user was seen using Grafana',
           iconName: 'question-circle',
         },
-        cell: LastSeenAtCell,
+        cell: ({ cell: { value } }: Cell<'lastSeenAtAge'>) => {
+          return <>{value && <>{value === '10 years' ? <Text color={'disabled'}>Never</Text> : value}</>}</>;
+        },
         sortType: (a, b) => new Date(a.original.lastSeenAt!).getTime() - new Date(b.original.lastSeenAt!).getTime(),
       },
       {
@@ -122,62 +143,13 @@ export const UsersTable = ({
     [showLicensedRole]
   );
   return (
-    <VerticalGroup spacing={'md'}>
+    <Stack gap={2}>
       <InteractiveTable columns={columns} data={users} getRowId={(user) => String(user.id)} fetchData={fetchData} />
       {showPaging && (
-        <HorizontalGroup justify={'flex-end'}>
+        <Flex justifyContent={'flex-end'}>
           <Pagination numberOfPages={totalPages} currentPage={currentPage} onNavigate={onChangePage} />
-        </HorizontalGroup>
+        </Flex>
       )}
-    </VerticalGroup>
+    </Stack>
   );
 };
-
-const OrgUnitsCell = ({ cell: { value, row } }: Cell<'orgs'>) => {
-  const styles = useStyles2(getStyles);
-  return (
-    <div className={styles.row}>
-      <OrgUnits units={value} icon={'building'} />
-      {row.original.isAdmin && (
-        <Tooltip placement="top" content="Grafana Admin">
-          <Icon name="shield" />
-        </Tooltip>
-      )}
-    </div>
-  );
-};
-
-const LicensedRoleCell = ({ cell: { value } }: Cell<'licensedRole'>) => {
-  const styles = useStyles2(getStyles);
-
-  return (
-    <>
-      {value === 'None' ? (
-        <span className={styles.disabled}>
-          Not assigned{' '}
-          <Tooltip placement="top" content="A licensed role will be assigned when this user signs in">
-            <Icon name="question-circle" />
-          </Tooltip>
-        </span>
-      ) : (
-        value
-      )}
-    </>
-  );
-};
-
-const LastSeenAtCell = ({ cell: { value } }: Cell<'lastSeenAtAge'>) => {
-  const styles = useStyles2(getStyles);
-
-  return <>{value && <>{value === '10 years' ? <span className={styles.disabled}>Never</span> : value}</>}</>;
-};
-
-const getStyles = (theme: GrafanaTheme2) => {
-  return {
-    disabled: css({ color: theme.colors.text.disabled }),
-    row: css({
-      display: 'flex',
-      alignItems: 'center',
-    }),
-  };
-};
diff --git a/public/app/features/teams/TeamList.tsx b/public/app/features/teams/TeamList.tsx
index 04a269e23bb..435ab51fa02 100644
--- a/public/app/features/teams/TeamList.tsx
+++ b/public/app/features/teams/TeamList.tsx
@@ -1,8 +1,6 @@
-import { css } from '@emotion/css';
 import React, { useEffect, useMemo, useState } from 'react';
 import { connect, ConnectedProps } from 'react-redux';
 
-import { GrafanaTheme2 } from '@grafana/data';
 import {
   LinkButton,
   FilterInput,
@@ -13,12 +11,10 @@ import {
   Icon,
   Tooltip,
   Column,
-  HorizontalGroup,
   Pagination,
-  VerticalGroup,
-  useStyles2,
   Avatar,
 } from '@grafana/ui';
+import { Stack, Flex } from '@grafana/ui/src/unstable';
 import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA';
 import { Page } from 'app/core/components/Page/Page';
 import { fetchRoleOptions } from 'app/core/components/RolePicker/api';
@@ -26,6 +22,7 @@ import { contextSrv } from 'app/core/services/context_srv';
 import { AccessControlAction, Role, StoreState, Team } from 'app/types';
 
 import { TeamRolePicker } from '../../core/components/RolePicker/TeamRolePicker';
+import { TableWrapper } from '../admin/Users/TableWrapper';
 
 import { deleteTeam, loadTeams, changePage, changeQuery, changeSort } from './state/actions';
 
@@ -50,7 +47,6 @@ export const TeamList = ({
   changeSort,
 }: Props) => {
   const [roleOptions, setRoleOptions] = useState<Role[]>([]);
-  const styles = useStyles2(getStyles);
 
   useEffect(() => {
     loadTeams(true);
@@ -165,24 +161,24 @@ export const TeamList = ({
                 New Team
               </LinkButton>
             </div>
-            <VerticalGroup spacing={'md'}>
-              <div className={styles.wrapper}>
+            <Stack gap={2}>
+              <TableWrapper>
                 <InteractiveTable
                   columns={columns}
                   data={teams}
                   getRowId={(team) => String(team.id)}
                   fetchData={changeSort}
                 />
-                <HorizontalGroup justify="flex-end">
+                <Flex justifyContent="flex-end">
                   <Pagination
                     hideWhenSinglePage
                     currentPage={page}
                     numberOfPages={totalPages}
                     onNavigate={changePage}
                   />
-                </HorizontalGroup>
-              </div>
-            </VerticalGroup>
+                </Flex>
+              </TableWrapper>
+            </Stack>
           </>
         )}
       </Page.Contents>
@@ -190,24 +186,6 @@ export const TeamList = ({
   );
 };
 
-const getStyles = (theme: GrafanaTheme2) => {
-  return {
-    // Enable RolePicker overflow
-    wrapper: css({
-      display: 'flex',
-      flexDirection: 'column',
-      overflowX: 'auto',
-      overflowY: 'hidden',
-      minHeight: '100vh',
-      width: '100%',
-      '& > div': {
-        overflowX: 'unset',
-        marginBottom: theme.spacing(2),
-      },
-    }),
-  };
-};
-
 function shouldDisplayRolePicker(): boolean {
   return (
     contextSrv.licensedAccessControlEnabled() &&