From c062d5e48df72df8c19db962dadbc5bf868e3fc8 Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@gmail.com>
Date: Wed, 6 Nov 2024 21:53:37 +1100
Subject: [PATCH] Cleanup modals (#8432)

* Cleanup modals

* Fix for playwright test

* Playwright test updates
---
 .../components/modals/AboutInvenTreeModal.tsx | 55 ++++++++++---------
 .../src/components/modals/LicenseModal.tsx    | 24 ++++++--
 .../src/components/modals/ServerInfoModal.tsx | 41 ++++++--------
 src/frontend/tests/modals.spec.ts             |  2 +-
 src/frontend/tests/pages/pui_scan.spec.ts     | 37 ++++++++++---
 src/frontend/tests/pui_command.spec.ts        |  5 +-
 src/frontend/tests/pui_general.spec.ts        | 21 -------
 7 files changed, 99 insertions(+), 86 deletions(-)

diff --git a/src/frontend/src/components/modals/AboutInvenTreeModal.tsx b/src/frontend/src/components/modals/AboutInvenTreeModal.tsx
index 8581506fac1c..dc8e7300fd4f 100644
--- a/src/frontend/src/components/modals/AboutInvenTreeModal.tsx
+++ b/src/frontend/src/components/modals/AboutInvenTreeModal.tsx
@@ -20,6 +20,7 @@ import { apiUrl, useServerApiState } from '../../states/ApiState';
 import { useLocalState } from '../../states/LocalState';
 import { useUserState } from '../../states/UserState';
 import { CopyButton } from '../buttons/CopyButton';
+import { StylishText } from '../items/StylishText';
 
 type AboutLookupRef = {
   ref: string;
@@ -56,9 +57,9 @@ export function AboutInvenTreeModal({
     alwaysLink: boolean = false
   ) {
     return lookup.map((map: AboutLookupRef, idx) => (
-      <tr key={idx}>
-        <td>{map.title}</td>
-        <td>
+      <Table.Tr key={idx}>
+        <Table.Td>{map.title}</Table.Td>
+        <Table.Td>
           <Group justify="space-between" gap="xs">
             {alwaysLink ? (
               <Anchor href={data[map.ref]} target="_blank">
@@ -73,8 +74,8 @@ export function AboutInvenTreeModal({
             )}
             {map.copy && <CopyButton value={data[map.ref]} />}
           </Group>
-        </td>
-      </tr>
+        </Table.Td>
+      </Table.Tr>
     ));
   }
   /* renderer */
@@ -95,13 +96,10 @@ export function AboutInvenTreeModal({
   return (
     <Stack>
       <Divider />
-      <Title order={5}>
-        <Trans>Version Information</Trans>
-      </Title>
-      <Group>
-        <Text>
-          <Trans>Your InvenTree version status is</Trans>
-        </Text>
+      <Group justify="space-between" wrap="nowrap">
+        <StylishText size="lg">
+          <Trans>Version Information</Trans>
+        </StylishText>
         {data.dev ? (
           <Badge color="blue">
             <Trans>Development Version</Trans>
@@ -116,8 +114,8 @@ export function AboutInvenTreeModal({
           </Badge>
         )}
       </Group>
-      <Table>
-        <tbody>
+      <Table striped>
+        <Table.Tbody>
           {fillTable(
             [
               {
@@ -144,9 +142,14 @@ export function AboutInvenTreeModal({
               {
                 ref: 'api',
                 title: <Trans>API Version</Trans>,
-                link: `${host}api-doc/`
+                link: `${host}api-doc/`,
+                copy: true
+              },
+              {
+                ref: 'python',
+                title: <Trans>Python Version</Trans>,
+                copy: true
               },
-              { ref: 'python', title: <Trans>Python Version</Trans> },
               {
                 ref: 'django',
                 title: <Trans>Django Version</Trans>,
@@ -156,17 +159,18 @@ export function AboutInvenTreeModal({
             ],
             data.version
           )}
-        </tbody>
+        </Table.Tbody>
       </Table>
-      <Title order={5}>
+      <Divider />
+      <StylishText size="lg">
         <Trans>Links</Trans>
-      </Title>
-      <Table>
-        <tbody>
+      </StylishText>
+      <Table striped>
+        <Table.Tbody>
           {fillTable(
             [
-              { ref: 'doc', title: <Trans>InvenTree Documentation</Trans> },
-              { ref: 'code', title: <Trans>View Code on GitHub</Trans> },
+              { ref: 'doc', title: <Trans>Documentation</Trans> },
+              { ref: 'code', title: <Trans>Source Code</Trans> },
               { ref: 'credit', title: <Trans>Credits</Trans> },
               { ref: 'app', title: <Trans>Mobile App</Trans> },
               { ref: 'bug', title: <Trans>Submit Bug Report</Trans> }
@@ -174,19 +178,18 @@ export function AboutInvenTreeModal({
             data.links,
             true
           )}
-        </tbody>
+        </Table.Tbody>
       </Table>
       <Divider />
       <Group justify="space-between">
         <CopyButton value={copyval} label={t`Copy version information`} />
         <Space />
         <Button
-          color="red"
           onClick={() => {
             context.closeModal(id);
           }}
         >
-          <Trans>Dismiss</Trans>
+          <Trans>Close</Trans>
         </Button>
       </Group>
     </Stack>
diff --git a/src/frontend/src/components/modals/LicenseModal.tsx b/src/frontend/src/components/modals/LicenseModal.tsx
index f3d85d95881d..7a9e8c8527ca 100644
--- a/src/frontend/src/components/modals/LicenseModal.tsx
+++ b/src/frontend/src/components/modals/LicenseModal.tsx
@@ -11,6 +11,7 @@ import {
   Text
 } from '@mantine/core';
 import { useQuery } from '@tanstack/react-query';
+import { useEffect, useMemo, useState } from 'react';
 
 import { api } from '../../App';
 import { ApiEndpoints } from '../../enums/ApiEndpoints';
@@ -53,6 +54,7 @@ export function LicenceView(entries: any[]) {
 export function LicenseModal() {
   const { data, isFetching, isError } = useQuery({
     queryKey: ['license'],
+    refetchOnMount: true,
     queryFn: () =>
       api
         .get(apiUrl(ApiEndpoints.license))
@@ -60,7 +62,17 @@ export function LicenseModal() {
         .catch(() => {})
   });
 
-  const rspdata = !data ? [] : Object.keys(data ?? {});
+  const packageKeys = useMemo(() => {
+    return !!data ? Object.keys(data ?? {}) : [];
+  }, [data]);
+
+  const [selectedKey, setSelectedKey] = useState<string | null>('');
+
+  useEffect(() => {
+    if (packageKeys.length > 0) {
+      setSelectedKey(packageKeys[0]);
+    }
+  }, [packageKeys]);
 
   return (
     <Stack gap="xs">
@@ -78,16 +90,20 @@ export function LicenseModal() {
           </Text>
         </Alert>
       ) : (
-        <Tabs defaultValue={rspdata[0] ?? ''}>
+        <Tabs
+          defaultValue={packageKeys[0] ?? ''}
+          value={selectedKey}
+          onChange={setSelectedKey}
+        >
           <Tabs.List>
-            {rspdata.map((key) => (
+            {packageKeys.map((key) => (
               <Tabs.Tab key={key} value={key}>
                 <Trans>{key} Packages</Trans>
               </Tabs.Tab>
             ))}
           </Tabs.List>
 
-          {rspdata.map((key) => (
+          {packageKeys.map((key) => (
             <Tabs.Panel key={key} value={key}>
               {LicenceView(data[key] ?? [])}
             </Tabs.Panel>
diff --git a/src/frontend/src/components/modals/ServerInfoModal.tsx b/src/frontend/src/components/modals/ServerInfoModal.tsx
index 84dd8cf17cd1..528c6f2844d2 100644
--- a/src/frontend/src/components/modals/ServerInfoModal.tsx
+++ b/src/frontend/src/components/modals/ServerInfoModal.tsx
@@ -12,6 +12,7 @@ import { ContextModalProps } from '@mantine/modals';
 
 import { useServerApiState } from '../../states/ApiState';
 import { OnlyStaff } from '../items/OnlyStaff';
+import { StylishText } from '../items/StylishText';
 
 export function ServerInfoModal({
   context,
@@ -22,10 +23,10 @@ export function ServerInfoModal({
   return (
     <Stack>
       <Divider />
-      <Title order={5}>
+      <StylishText size="lg">
         <Trans>Server</Trans>
-      </Title>
-      <Table>
+      </StylishText>
+      <Table striped>
         <Table.Tbody>
           <Table.Tr>
             <Table.Td>
@@ -33,6 +34,18 @@ export function ServerInfoModal({
             </Table.Td>
             <Table.Td>{server.instance}</Table.Td>
           </Table.Tr>
+          <Table.Tr>
+            <Table.Td>
+              <Trans>Server Version</Trans>
+            </Table.Td>
+            <Table.Td>{server.version}</Table.Td>
+          </Table.Tr>
+          <Table.Tr>
+            <Table.Td>
+              <Trans>API Version</Trans>
+            </Table.Td>
+            <Table.Td>{server.apiVersion}</Table.Td>
+          </Table.Tr>
           <Table.Tr>
             <Table.Td>
               <Trans>Database</Trans>
@@ -117,34 +130,14 @@ export function ServerInfoModal({
           )}
         </Table.Tbody>
       </Table>
-      <Title order={5}>
-        <Trans>Version</Trans>
-      </Title>
-      <Table>
-        <Table.Tbody>
-          <Table.Tr>
-            <Table.Td>
-              <Trans>Server Version</Trans>
-            </Table.Td>
-            <Table.Td>{server.version}</Table.Td>
-          </Table.Tr>
-          <Table.Tr>
-            <Table.Td>
-              <Trans>API Version</Trans>
-            </Table.Td>
-            <Table.Td>{server.apiVersion}</Table.Td>
-          </Table.Tr>
-        </Table.Tbody>
-      </Table>
       <Divider />
       <Group justify="right">
         <Button
-          color="red"
           onClick={() => {
             context.closeModal(id);
           }}
         >
-          <Trans>Dismiss</Trans>
+          <Trans>Close</Trans>
         </Button>
       </Group>
     </Stack>
diff --git a/src/frontend/tests/modals.spec.ts b/src/frontend/tests/modals.spec.ts
index 5977fd20f2e4..b9809ce53895 100644
--- a/src/frontend/tests/modals.spec.ts
+++ b/src/frontend/tests/modals.spec.ts
@@ -12,7 +12,7 @@ test('Modals as admin', async ({ page }) => {
     })
     .click();
   await page.getByRole('cell', { name: 'Instance Name' }).waitFor();
-  await page.getByRole('button', { name: 'Dismiss' }).click();
+  await page.getByRole('button', { name: 'Close' }).click();
 
   await page.waitForURL('**/platform/home');
 
diff --git a/src/frontend/tests/pages/pui_scan.spec.ts b/src/frontend/tests/pages/pui_scan.spec.ts
index 85d2a6c4a574..7459660da51c 100644
--- a/src/frontend/tests/pages/pui_scan.spec.ts
+++ b/src/frontend/tests/pages/pui_scan.spec.ts
@@ -24,7 +24,28 @@ async function defaultScanTest(page, search_text) {
   await page.getByRole('button', { name: 'Lookup part' }).click();
 }
 
-test('Pages - Index - Scan (Part)', async ({ page }) => {
+test('Scanning', async ({ page }) => {
+  await doQuickLogin(page);
+
+  await page.getByLabel('navigation-menu').click();
+  await page.getByRole('button', { name: 'System Information' }).click();
+  await page.locator('button').filter({ hasText: 'Close' }).click();
+
+  await page.getByLabel('navigation-menu').click();
+  await page.getByRole('button', { name: 'Scan Barcode' }).click();
+
+  await page.getByPlaceholder('Select input method').click();
+  await page.getByRole('option', { name: 'Manual input' }).click();
+  await page.getByPlaceholder('Enter item serial or data').click();
+  await page.getByPlaceholder('Enter item serial or data').fill('123');
+  await page.getByPlaceholder('Enter item serial or data').press('Enter');
+  await page.getByRole('cell', { name: 'manually' }).click();
+  await page.getByRole('button', { name: 'Lookup part' }).click();
+  await page.getByPlaceholder('Select input method').click();
+  await page.getByRole('option', { name: 'Manual input' }).click();
+});
+
+test('Scanning (Part)', async ({ page }) => {
   await defaultScanTest(page, '{"part": 1}');
 
   // part: 1
@@ -33,7 +54,7 @@ test('Pages - Index - Scan (Part)', async ({ page }) => {
   await page.getByRole('cell', { name: 'part' }).waitFor();
 });
 
-test('Pages - Index - Scan (Stockitem)', async ({ page }) => {
+test('Scanning (Stockitem)', async ({ page }) => {
   await defaultScanTest(page, '{"stockitem": 408}');
 
   // stockitem: 408
@@ -42,7 +63,7 @@ test('Pages - Index - Scan (Stockitem)', async ({ page }) => {
   await page.getByRole('cell', { name: 'Quantity: 100' }).waitFor();
 });
 
-test('Pages - Index - Scan (StockLocation)', async ({ page }) => {
+test('Scanning (StockLocation)', async ({ page }) => {
   await defaultScanTest(page, '{"stocklocation": 3}');
 
   // stocklocation: 3
@@ -51,7 +72,7 @@ test('Pages - Index - Scan (StockLocation)', async ({ page }) => {
   await page.getByRole('cell', { name: 'stocklocation' }).waitFor();
 });
 
-test('Pages - Index - Scan (SupplierPart)', async ({ page }) => {
+test('Scanning (SupplierPart)', async ({ page }) => {
   await defaultScanTest(page, '{"supplierpart": 204}');
 
   // supplierpart: 204
@@ -60,7 +81,7 @@ test('Pages - Index - Scan (SupplierPart)', async ({ page }) => {
   await page.getByRole('cell', { name: 'supplierpart' }).waitFor();
 });
 
-test('Pages - Index - Scan (PurchaseOrder)', async ({ page }) => {
+test('Scanning (PurchaseOrder)', async ({ page }) => {
   await defaultScanTest(page, '{"purchaseorder": 12}');
 
   // purchaseorder: 12
@@ -69,7 +90,7 @@ test('Pages - Index - Scan (PurchaseOrder)', async ({ page }) => {
   await page.getByRole('cell', { name: 'purchaseorder' }).waitFor();
 });
 
-test('Pages - Index - Scan (SalesOrder)', async ({ page }) => {
+test('Scanning (SalesOrder)', async ({ page }) => {
   await defaultScanTest(page, '{"salesorder": 6}');
 
   // salesorder: 6
@@ -78,7 +99,7 @@ test('Pages - Index - Scan (SalesOrder)', async ({ page }) => {
   await page.getByRole('cell', { name: 'salesorder' }).waitFor();
 });
 
-test('Pages - Index - Scan (Build)', async ({ page }) => {
+test('Scanning (Build)', async ({ page }) => {
   await defaultScanTest(page, '{"build": 8}');
 
   // build: 8
@@ -87,7 +108,7 @@ test('Pages - Index - Scan (Build)', async ({ page }) => {
   await page.getByRole('cell', { name: 'build', exact: true }).waitFor();
 });
 
-test('Pages - Index - Scan (General)', async ({ page }) => {
+test('Scanning (General)', async ({ page }) => {
   await defaultScanTest(page, '{"unknown": 312}');
   await page.getByText('"unknown": 312').waitFor();
 
diff --git a/src/frontend/tests/pui_command.spec.ts b/src/frontend/tests/pui_command.spec.ts
index 8f5d8d15e57b..3b53c7bffa1e 100644
--- a/src/frontend/tests/pui_command.spec.ts
+++ b/src/frontend/tests/pui_command.spec.ts
@@ -53,7 +53,7 @@ test('Quick Command - No Keys', async ({ page }) => {
     })
     .click();
   await page.getByRole('cell', { name: 'Instance Name' }).waitFor();
-  await page.getByRole('button', { name: 'Dismiss' }).click();
+  await page.getByRole('button', { name: 'Close' }).click();
 
   await page.waitForURL('**/platform/home');
 
@@ -66,8 +66,9 @@ test('Quick Command - No Keys', async ({ page }) => {
     .click();
   await page.getByText('License Information').first().waitFor();
   await page.getByRole('tab', { name: 'backend Packages' }).waitFor();
+  await page.getByRole('button', { name: 'Django BSD License' }).click();
 
-  await page.getByLabel('License Information').getByRole('button').click();
+  await page.keyboard.press('Escape');
 
   // use about
   await page.getByLabel('open-spotlight').click();
diff --git a/src/frontend/tests/pui_general.spec.ts b/src/frontend/tests/pui_general.spec.ts
index 370947010201..2575e714c75c 100644
--- a/src/frontend/tests/pui_general.spec.ts
+++ b/src/frontend/tests/pui_general.spec.ts
@@ -53,27 +53,6 @@ test('Sales', async ({ page }) => {
   await page.getByRole('tab', { name: 'Notes' }).click();
 });
 
-test('Scanning', async ({ page }) => {
-  await doQuickLogin(page);
-
-  await page.getByLabel('navigation-menu').click();
-  await page.getByRole('button', { name: 'System Information' }).click();
-  await page.locator('button').filter({ hasText: 'Dismiss' }).click();
-
-  await page.getByLabel('navigation-menu').click();
-  await page.getByRole('button', { name: 'Scan Barcode' }).click();
-
-  await page.getByPlaceholder('Select input method').click();
-  await page.getByRole('option', { name: 'Manual input' }).click();
-  await page.getByPlaceholder('Enter item serial or data').click();
-  await page.getByPlaceholder('Enter item serial or data').fill('123');
-  await page.getByPlaceholder('Enter item serial or data').press('Enter');
-  await page.getByRole('cell', { name: 'manually' }).click();
-  await page.getByRole('button', { name: 'Lookup part' }).click();
-  await page.getByPlaceholder('Select input method').click();
-  await page.getByRole('option', { name: 'Manual input' }).click();
-});
-
 test('Company', async ({ page }) => {
   await doQuickLogin(page);