From 04bb57d43d53c778ad5026c77a38354eb6096390 Mon Sep 17 00:00:00 2001
From: David Mears <clarinet.david@gmail.com>
Date: Tue, 19 Nov 2024 17:16:27 +0000
Subject: [PATCH] Organise the presentation of metadata page

---
 .../components/contents/metadata/Metadata.tsx | 48 ++++++++++++-------
 app/src/styles/globals.css                    |  5 ++
 .../contents/metadata/Metadata.test.tsx       | 28 ++++++-----
 .../components/main/PacketLayout.test.tsx     |  2 +-
 4 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/app/src/app/components/contents/metadata/Metadata.tsx b/app/src/app/components/contents/metadata/Metadata.tsx
index 3bb121cb..c051cfe3 100644
--- a/app/src/app/components/contents/metadata/Metadata.tsx
+++ b/app/src/app/components/contents/metadata/Metadata.tsx
@@ -3,6 +3,7 @@ import { getDateUTCString, getElapsedTime } from "../../../../helpers";
 import { usePacketOutletContext } from "../../main/PacketOutlet";
 import { PacketHeader } from "../packets";
 import { MetadataListItem } from "./MetadataListItem";
+import { Github, Timer } from "lucide-react";
 
 export default function Metadata() {
   const { packetId, packetName } = useParams();
@@ -15,26 +16,41 @@ export default function Metadata() {
     <>
       <PacketHeader packetName={packetName ?? ""} packetId={packetId ?? ""} />
       {packet && (
-        <ul className="flex flex-col space-y-3">
-          {startedTime && <MetadataListItem label="Started" value={startedTime} />}
-          {elapsedTime && <MetadataListItem label="Elapsed" value={elapsedTime} />}
+        <>
+          {packet.git &&
+            <span className="flex gap-1 items-center text-muted-foreground">
+              <Timer className="small-icon"/>
+              <h3 className="text-lg font-bold tracking-tight">Timings</h3>
+            </span>
+          }
+          <ul className="ps-1 flex gap-10 !mt-3">
+            {startedTime && <MetadataListItem label="Started" value={startedTime} />}
+            {elapsedTime && <MetadataListItem label="Elapsed" value={elapsedTime} />}
+          </ul>
           {packet.git && (
             <>
-              <MetadataListItem label="Git Branch" value={packet.git.branch} />
-              <MetadataListItem label="Git Commit" value={packet.git.sha} />
-              <li className="flex flex-col">
-                <span className="font-semibold mr-2">Git Remotes</span>
-                <ul className="list-disc list-inside">
-                  {packet.git.url?.map((url, index) => (
-                    <li key={index} className="text-muted-foreground">
-                      {url}
-                    </li>
-                  ))}
-                </ul>
-              </li>
+              <hr className="h-px my-8 bg-gray-200 border-0 dark:bg-gray-700"/>
+              <span className="flex gap-1 items-center text-muted-foreground">
+                <Github className="small-icon"/>
+                <h3 className="text-lg font-bold tracking-tight" data-testid="gitHeading">Git</h3>
+              </span>
+              <ul className="ps-1 flex flex-col space-y-3 !mt-3">
+                <MetadataListItem label="Branch" value={packet.git.branch}/>
+                <MetadataListItem label="Commit" value={packet.git.sha}/>
+                <li className="flex flex-col">
+                  <span className="font-semibold mr-2">Remotes</span>
+                    <ul className="ps-1 list-disc list-inside">
+                      {packet.git.url?.map((url, index) => (
+                        <li key={index} className="text-muted-foreground">
+                          {url}
+                        </li>
+                      ))}
+                    </ul>
+                </li>
+              </ul>
             </>
           )}
-        </ul>
+        </>
       )}
     </>
   );
diff --git a/app/src/styles/globals.css b/app/src/styles/globals.css
index 58926d69..b3666459 100644
--- a/app/src/styles/globals.css
+++ b/app/src/styles/globals.css
@@ -102,4 +102,9 @@
 
 iframe {
   background: #fff;
+}
+
+.small-icon {
+  width: 1em;
+  height: 1em;
 }
\ No newline at end of file
diff --git a/app/src/tests/components/contents/metadata/Metadata.test.tsx b/app/src/tests/components/contents/metadata/Metadata.test.tsx
index 8d264820..8bb614f7 100644
--- a/app/src/tests/components/contents/metadata/Metadata.test.tsx
+++ b/app/src/tests/components/contents/metadata/Metadata.test.tsx
@@ -23,15 +23,17 @@ describe("Metadata component", () => {
     );
   };
 
-  it("renders all metadata if present", async () => {
+  it("renders all metadata and sub-headings if metadata is present", async () => {
     renderComponent();
 
     expect(await screen.findByText(mockPacket.id)).toBeVisible();
+    expect(screen.getByText(/timings/i)).toBeInTheDocument();
     expect(screen.getByText(/started/i)).toBeInTheDocument();
     expect(screen.getByText(/elapsed/i)).toBeInTheDocument();
-    expect(screen.getByText(/git branch/i)).toBeInTheDocument();
-    expect(screen.getByText(/git commit/i)).toBeInTheDocument();
-    expect(screen.getByText(/git remotes/i)).toBeInTheDocument();
+    expect(screen.getByTestId("gitHeading")).toBeInTheDocument();
+    expect(screen.getByText(/branch/i)).toBeInTheDocument();
+    expect(screen.getByText(/commit/i)).toBeInTheDocument();
+    expect(screen.getByText(/remotes/i)).toBeInTheDocument();
   });
 
   it("does not render elapsed time when datetime has no difference", async () => {
@@ -55,7 +57,7 @@ describe("Metadata component", () => {
     expect(screen.queryByText("elapsed")).not.toBeInTheDocument();
   });
 
-  it("should not render git metadata when git is not present", async () => {
+  it("should not render git metadata or any sub-headings when git is not present", async () => {
     server.use(
       rest.get("*", (req, res, ctx) => {
         return res(
@@ -70,9 +72,11 @@ describe("Metadata component", () => {
 
     await screen.findByText(/started/i);
 
-    expect(screen.queryByText(/git branch/i)).not.toBeInTheDocument();
-    expect(screen.queryByText(/git commit/i)).not.toBeInTheDocument();
-    expect(screen.queryByText(/git remotes/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/timings/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/git/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/branch/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/commit/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/remotes/i)).not.toBeInTheDocument();
   });
 
   it("should not render any fields if packet is null", async () => {
@@ -85,10 +89,12 @@ describe("Metadata component", () => {
 
     await screen.findByText(/metadata/i);
 
+    expect(screen.queryByText(/timings/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/git/i)).not.toBeInTheDocument();
     expect(screen.queryByText(/started/i)).not.toBeInTheDocument();
     expect(screen.queryByText(/elapsed/i)).not.toBeInTheDocument();
-    expect(screen.queryByText(/git branch/i)).not.toBeInTheDocument();
-    expect(screen.queryByText(/git commit/i)).not.toBeInTheDocument();
-    expect(screen.queryByText(/git remotes/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/branch/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/commit/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/remotes/i)).not.toBeInTheDocument();
   });
 });
diff --git a/app/src/tests/components/main/PacketLayout.test.tsx b/app/src/tests/components/main/PacketLayout.test.tsx
index 93455759..d55d6f54 100644
--- a/app/src/tests/components/main/PacketLayout.test.tsx
+++ b/app/src/tests/components/main/PacketLayout.test.tsx
@@ -39,7 +39,7 @@ describe("Packet Layout test", () => {
 
     userEvent.click(screen.getByRole("link", { name: /metadata/i }));
 
-    expect(await screen.findByText(/git branch/i)).toBeVisible();
+    expect(await screen.findByText(/branch/i)).toBeVisible();
 
     userEvent.click(screen.getByRole("link", { name: /downloads/i }));