From 550ad65c5d306df875654ed19043b0fb7dc97a4e Mon Sep 17 00:00:00 2001
From: Kerem Yilmaz <kerem@skyvern.com>
Date: Tue, 23 Apr 2024 13:54:04 -0700
Subject: [PATCH] UI/UX improvements (#218)

---
 skyvern-frontend/src/components/ui/card.tsx   |   6 +-
 .../src/routes/settings/Settings.tsx          |  71 ++++---
 .../src/routes/tasks/detail/TaskDetails.tsx   | 112 +++++-----
 .../src/routes/tasks/list/TaskList.tsx        | 192 ++++++++++--------
 .../routes/tasks/list/TaskListSkeleton.tsx    |   8 +-
 .../src/routes/tasks/running/QueuedTasks.tsx  |   4 +
 6 files changed, 220 insertions(+), 173 deletions(-)

diff --git a/skyvern-frontend/src/components/ui/card.tsx b/skyvern-frontend/src/components/ui/card.tsx
index 84a3b50b6..13793cd7b 100644
--- a/skyvern-frontend/src/components/ui/card.tsx
+++ b/skyvern-frontend/src/components/ui/card.tsx
@@ -23,7 +23,7 @@ const CardHeader = React.forwardRef<
 >(({ className, ...props }, ref) => (
   <div
     ref={ref}
-    className={cn("flex flex-col space-y-1.5 p-6", className)}
+    className={cn("flex flex-col space-y-1.5 p-4", className)}
     {...props}
   />
 ));
@@ -57,7 +57,7 @@ const CardContent = React.forwardRef<
   HTMLDivElement,
   React.HTMLAttributes<HTMLDivElement>
 >(({ className, ...props }, ref) => (
-  <div ref={ref} className={cn("p-6 pt-0", className)} {...props} />
+  <div ref={ref} className={cn("p-4", className)} {...props} />
 ));
 CardContent.displayName = "CardContent";
 
@@ -67,7 +67,7 @@ const CardFooter = React.forwardRef<
 >(({ className, ...props }, ref) => (
   <div
     ref={ref}
-    className={cn("flex items-center p-6 pt-0", className)}
+    className={cn("flex items-center p-4 pt-0", className)}
     {...props}
   />
 ));
diff --git a/skyvern-frontend/src/routes/settings/Settings.tsx b/skyvern-frontend/src/routes/settings/Settings.tsx
index 1248f7c63..f3ea047c2 100644
--- a/skyvern-frontend/src/routes/settings/Settings.tsx
+++ b/skyvern-frontend/src/routes/settings/Settings.tsx
@@ -1,5 +1,4 @@
 import { Label } from "@/components/ui/label";
-import { useId } from "react";
 import {
   Select,
   SelectContent,
@@ -8,40 +7,54 @@ import {
   SelectValue,
 } from "@/components/ui/select";
 import { useSettingsStore } from "@/store/SettingsStore";
+import {
+  Card,
+  CardContent,
+  CardDescription,
+  CardHeader,
+  CardTitle,
+} from "@/components/ui/card";
 
 function Settings() {
   const { environment, organization, setEnvironment, setOrganization } =
     useSettingsStore();
-  const environmentInputId = useId();
-  const organizationInputId = useId();
 
   return (
-    <div className="flex flex-col gap-6">
-      <h1>Settings</h1>
-      <div className="flex flex-col gap-4">
-        <Label htmlFor={environmentInputId}>Environment</Label>
-        <div>
-          <Select value={environment} onValueChange={setEnvironment}>
-            <SelectTrigger>
-              <SelectValue placeholder="Environment" />
-            </SelectTrigger>
-            <SelectContent>
-              <SelectItem value="local">local</SelectItem>
-            </SelectContent>
-          </Select>
-        </div>
-        <Label htmlFor={organizationInputId}>Organization</Label>
-        <div>
-          <Select value={organization} onValueChange={setOrganization}>
-            <SelectTrigger>
-              <SelectValue placeholder="Organization" />
-            </SelectTrigger>
-            <SelectContent>
-              <SelectItem value="skyvern">Skyvern</SelectItem>
-            </SelectContent>
-          </Select>
-        </div>
-      </div>
+    <div className="flex flex-col gap-8 max-w-5xl mx-auto">
+      <Card>
+        <CardHeader className="border-b-2">
+          <CardTitle className="text-lg">Settings</CardTitle>
+          <CardDescription>
+            You can select environment and organization here
+          </CardDescription>
+        </CardHeader>
+        <CardContent className="p-8">
+          <div className="flex flex-col gap-4">
+            <div className="flex gap-4 items-center">
+              <Label className="whitespace-nowrap w-36">Environment</Label>
+              <Select value={environment} onValueChange={setEnvironment}>
+                <SelectTrigger>
+                  <SelectValue placeholder="Environment" />
+                </SelectTrigger>
+                <SelectContent>
+                  <SelectItem value="local">local</SelectItem>
+                </SelectContent>
+              </Select>
+            </div>
+            <div className="flex gap-4 items-center">
+              <Label className="whitespace-nowrap w-36">Organization</Label>
+              <Select value={organization} onValueChange={setOrganization}>
+                <SelectTrigger>
+                  <SelectValue placeholder="Organization" />
+                </SelectTrigger>
+                <SelectContent>
+                  <SelectItem value="skyvern">Skyvern</SelectItem>
+                </SelectContent>
+              </Select>
+            </div>
+          </div>
+        </CardContent>
+      </Card>
     </div>
   );
 }
diff --git a/skyvern-frontend/src/routes/tasks/detail/TaskDetails.tsx b/skyvern-frontend/src/routes/tasks/detail/TaskDetails.tsx
index feb57f6cf..6f9d5d4cb 100644
--- a/skyvern-frontend/src/routes/tasks/detail/TaskDetails.tsx
+++ b/skyvern-frontend/src/routes/tasks/detail/TaskDetails.tsx
@@ -18,6 +18,7 @@ import { StepArtifactsLayout } from "./StepArtifactsLayout";
 import Zoom from "react-medium-image-zoom";
 import { AspectRatio } from "@/components/ui/aspect-ratio";
 import { getRecordingURL, getScreenshotURL } from "./artifactUtils";
+import { Skeleton } from "@/components/ui/skeleton";
 
 function TaskDetails() {
   const { taskId } = useParams();
@@ -39,14 +40,6 @@ function TaskDetails() {
     return <div>Error: {taskError?.message}</div>;
   }
 
-  if (isTaskFetching) {
-    return <div>Loading...</div>; // TODO: skeleton
-  }
-
-  if (!task) {
-    return <div>Task not found</div>;
-  }
-
   return (
     <div>
       <div className="flex flex-col gap-4 relative">
@@ -58,9 +51,9 @@ function TaskDetails() {
             refetch();
           }}
         >
-          <ReloadIcon />
+          <ReloadIcon className="w-4 h-4" />
         </Button>
-        {task.recording_url ? (
+        {task?.recording_url ? (
           <div className="flex">
             <Label className="w-32">Recording</Label>
             <video src={getRecordingURL(task)} controls />
@@ -68,9 +61,13 @@ function TaskDetails() {
         ) : null}
         <div className="flex items-center">
           <Label className="w-32">Status</Label>
-          <StatusBadge status={task.status} />
+          {isTaskFetching ? (
+            <Skeleton className="w-32 h-8" />
+          ) : task ? (
+            <StatusBadge status={task?.status} />
+          ) : null}
         </div>
-        {task.status === Status.Completed ? (
+        {task?.status === Status.Completed ? (
           <div className="flex items-center">
             <Label className="w-32 shrink-0">Extracted Information</Label>
             <Textarea
@@ -80,7 +77,8 @@ function TaskDetails() {
             />
           </div>
         ) : null}
-        {task.status === Status.Failed || task.status === Status.Terminated ? (
+        {task?.status === Status.Failed ||
+        task?.status === Status.Terminated ? (
           <div className="flex items-center">
             <Label className="w-32 shrink-0">Failure Reason</Label>
             <Textarea
@@ -97,55 +95,59 @@ function TaskDetails() {
             <h1>Task Parameters</h1>
           </AccordionTrigger>
           <AccordionContent>
-            <div>
-              <p className="py-2">Task ID: {taskId}</p>
-              <p className="py-2">URL: {task.request.url}</p>
-              <p className="py-2">
-                Created: {basicTimeFormat(task.created_at)}
-              </p>
-              <div className="py-2">
-                <Label>Navigation Goal</Label>
-                <Textarea
-                  rows={5}
-                  value={task.request.navigation_goal}
-                  readOnly
-                />
+            {task ? (
+              <div>
+                <p className="py-2">Task ID: {taskId}</p>
+                <p className="py-2">URL: {task.request.url}</p>
+                <p className="py-2">
+                  Created: {basicTimeFormat(task.created_at)}
+                </p>
+                <div className="py-2">
+                  <Label>Navigation Goal</Label>
+                  <Textarea
+                    rows={5}
+                    value={task.request.navigation_goal}
+                    readOnly
+                  />
+                </div>
+                <div className="py-2">
+                  <Label>Navigation Payload</Label>
+                  <Textarea
+                    rows={5}
+                    value={task.request.navigation_payload}
+                    readOnly
+                  />
+                </div>
+                <div className="py-2">
+                  <Label>Data Extraction Goal</Label>
+                  <Textarea
+                    rows={5}
+                    value={task.request.data_extraction_goal}
+                    readOnly
+                  />
+                </div>
               </div>
-              <div className="py-2">
-                <Label>Navigation Payload</Label>
-                <Textarea
-                  rows={5}
-                  value={task.request.navigation_payload}
-                  readOnly
-                />
-              </div>
-              <div className="py-2">
-                <Label>Data Extraction Goal</Label>
-                <Textarea
-                  rows={5}
-                  value={task.request.data_extraction_goal}
-                  readOnly
-                />
-              </div>
-            </div>
+            ) : null}
           </AccordionContent>
         </AccordionItem>
         <AccordionItem value="task-artifacts">
           <AccordionTrigger>
-            <h1>Screenshot</h1>
+            <h1>Final Screenshot</h1>
           </AccordionTrigger>
           <AccordionContent>
-            <div className="max-w-sm mx-auto">
-              {task.screenshot_url ? (
-                <Zoom zoomMargin={16}>
-                  <AspectRatio ratio={16 / 9}>
-                    <img src={getScreenshotURL(task)} alt="screenshot" />
-                  </AspectRatio>
-                </Zoom>
-              ) : (
-                <p>No screenshot</p>
-              )}
-            </div>
+            {task ? (
+              <div className="max-w-sm mx-auto">
+                {task.screenshot_url ? (
+                  <Zoom zoomMargin={16}>
+                    <AspectRatio ratio={16 / 9}>
+                      <img src={getScreenshotURL(task)} alt="screenshot" />
+                    </AspectRatio>
+                  </Zoom>
+                ) : (
+                  <p>No screenshot</p>
+                )}
+              </div>
+            ) : null}
           </AccordionContent>
         </AccordionItem>
         <AccordionItem value="task-steps">
diff --git a/skyvern-frontend/src/routes/tasks/list/TaskList.tsx b/skyvern-frontend/src/routes/tasks/list/TaskList.tsx
index 24dbcfceb..a38db083c 100644
--- a/skyvern-frontend/src/routes/tasks/list/TaskList.tsx
+++ b/skyvern-frontend/src/routes/tasks/list/TaskList.tsx
@@ -25,6 +25,13 @@ import { PAGE_SIZE } from "../constants";
 import { StatusBadge } from "@/components/StatusBadge";
 import { basicTimeFormat } from "@/util/timeFormat";
 import { QueuedTasks } from "../running/QueuedTasks";
+import {
+  Card,
+  CardContent,
+  CardDescription,
+  CardHeader,
+  CardTitle,
+} from "@/components/ui/card";
 
 function TaskList() {
   const navigate = useNavigate();
@@ -52,19 +59,11 @@ function TaskList() {
     placeholderData: keepPreviousData,
   });
 
-  if (isPending) {
-    return <TaskListSkeleton />;
-  }
-
   if (isError) {
     return <div>Error: {error?.message}</div>;
   }
 
-  if (!tasks) {
-    return null;
-  }
-
-  const resolvedTasks = tasks.filter(
+  const resolvedTasks = tasks?.filter(
     (task) =>
       task.status === "completed" ||
       task.status === "failed" ||
@@ -72,81 +71,110 @@ function TaskList() {
   );
 
   return (
-    <div className="flex flex-col gap-4">
-      <h1 className="text-2xl py-2 border-b-2">Running Tasks</h1>
-      <div className="grid grid-cols-4 gap-4">
-        <RunningTasks />
-      </div>
-      <h1 className="text-2xl py-2 border-b-2">Queued Tasks</h1>
-      <QueuedTasks />
-      <h1 className="text-2xl py-2 border-b-2">Task History</h1>
-      <Table>
-        <TableHeader>
-          <TableRow>
-            <TableHead className="w-1/3">URL</TableHead>
-            <TableHead className="w-1/3">Status</TableHead>
-            <TableHead className="w-1/3">Created At</TableHead>
-          </TableRow>
-        </TableHeader>
-        <TableBody>
-          {tasks.length === 0 ? (
-            <TableRow>
-              <TableCell colSpan={3}>No tasks found</TableCell>
-            </TableRow>
+    <div className="flex flex-col gap-8 max-w-5xl mx-auto">
+      <Card>
+        <CardHeader className="border-b-2">
+          <CardTitle className="text-xl">Running Tasks</CardTitle>
+          <CardDescription>Tasks that are currently running</CardDescription>
+        </CardHeader>
+        <CardContent className="p-4">
+          <div className="grid grid-cols-4 gap-4">
+            <RunningTasks />
+          </div>
+        </CardContent>
+      </Card>
+      <Card>
+        <CardHeader className="border-b-2">
+          <CardTitle className="text-xl">Queued Tasks</CardTitle>
+          <CardDescription>Tasks that are waiting to run</CardDescription>
+        </CardHeader>
+        <CardContent className="p-4">
+          <QueuedTasks />
+        </CardContent>
+      </Card>
+      <Card>
+        <CardHeader className="border-b-2">
+          <CardTitle className="text-xl">Task History</CardTitle>
+          <CardDescription>Tasks you have run previously</CardDescription>
+        </CardHeader>
+        <CardContent className="p-4">
+          {isPending ? (
+            <TaskListSkeleton />
           ) : (
-            resolvedTasks.map((task) => {
-              return (
-                <TableRow
-                  key={task.task_id}
-                  className="cursor-pointer w-4"
-                  onClick={() => {
-                    navigate(task.task_id);
-                  }}
-                >
-                  <TableCell className="w-1/3">{task.request.url}</TableCell>
-                  <TableCell className="w-1/3">
-                    <StatusBadge status={task.status} />
-                  </TableCell>
-                  <TableCell className="w-1/3">
-                    {basicTimeFormat(task.created_at)}
-                  </TableCell>
-                </TableRow>
-              );
-            })
+            <>
+              <Table>
+                <TableHeader>
+                  <TableRow>
+                    <TableHead className="w-1/3">URL</TableHead>
+                    <TableHead className="w-1/3">Status</TableHead>
+                    <TableHead className="w-1/3">Created At</TableHead>
+                  </TableRow>
+                </TableHeader>
+                <TableBody>
+                  {tasks.length === 0 ? (
+                    <TableRow>
+                      <TableCell colSpan={3}>No tasks found</TableCell>
+                    </TableRow>
+                  ) : (
+                    resolvedTasks?.map((task) => {
+                      return (
+                        <TableRow
+                          key={task.task_id}
+                          className="cursor-pointer w-4"
+                          onClick={() => {
+                            navigate(task.task_id);
+                          }}
+                        >
+                          <TableCell className="w-1/3">
+                            {task.request.url}
+                          </TableCell>
+                          <TableCell className="w-1/3">
+                            <StatusBadge status={task.status} />
+                          </TableCell>
+                          <TableCell className="w-1/3">
+                            {basicTimeFormat(task.created_at)}
+                          </TableCell>
+                        </TableRow>
+                      );
+                    })
+                  )}
+                </TableBody>
+              </Table>
+              <Pagination>
+                <PaginationContent>
+                  <PaginationItem>
+                    <PaginationPrevious
+                      href="#"
+                      className={cn({ "cursor-not-allowed": page === 1 })}
+                      onClick={() => {
+                        if (page === 1) {
+                          return;
+                        }
+                        const params = new URLSearchParams();
+                        params.set("page", String(Math.max(1, page - 1)));
+                        setSearchParams(params);
+                      }}
+                    />
+                  </PaginationItem>
+                  <PaginationItem>
+                    <PaginationLink href="#">{page}</PaginationLink>
+                  </PaginationItem>
+                  <PaginationItem>
+                    <PaginationNext
+                      href="#"
+                      onClick={() => {
+                        const params = new URLSearchParams();
+                        params.set("page", String(page + 1));
+                        setSearchParams(params);
+                      }}
+                    />
+                  </PaginationItem>
+                </PaginationContent>
+              </Pagination>
+            </>
           )}
-        </TableBody>
-      </Table>
-      <Pagination>
-        <PaginationContent>
-          <PaginationItem>
-            <PaginationPrevious
-              href="#"
-              className={cn({ "cursor-not-allowed": page === 1 })}
-              onClick={() => {
-                if (page === 1) {
-                  return;
-                }
-                const params = new URLSearchParams();
-                params.set("page", String(Math.max(1, page - 1)));
-                setSearchParams(params);
-              }}
-            />
-          </PaginationItem>
-          <PaginationItem>
-            <PaginationLink href="#">{page}</PaginationLink>
-          </PaginationItem>
-          <PaginationItem>
-            <PaginationNext
-              href="#"
-              onClick={() => {
-                const params = new URLSearchParams();
-                params.set("page", String(page + 1));
-                setSearchParams(params);
-              }}
-            />
-          </PaginationItem>
-        </PaginationContent>
-      </Pagination>
+        </CardContent>
+      </Card>
     </div>
   );
 }
diff --git a/skyvern-frontend/src/routes/tasks/list/TaskListSkeleton.tsx b/skyvern-frontend/src/routes/tasks/list/TaskListSkeleton.tsx
index 60de14fb7..1f33557ba 100644
--- a/skyvern-frontend/src/routes/tasks/list/TaskListSkeleton.tsx
+++ b/skyvern-frontend/src/routes/tasks/list/TaskListSkeleton.tsx
@@ -8,7 +8,7 @@ import {
   TableRow,
 } from "@/components/ui/table";
 
-const pageSizeArray = new Array(15).fill(null); // doesn't matter the value
+const pageSizeArray = new Array(5).fill(null); // doesn't matter the value
 
 function TaskListSkeleton() {
   return (
@@ -26,13 +26,13 @@ function TaskListSkeleton() {
             return (
               <TableRow key={index}>
                 <TableCell className="w-1/3">
-                  <Skeleton className="w-full h-full" />
+                  <Skeleton className="w-full h-4" />
                 </TableCell>
                 <TableCell className="w-1/3">
-                  <Skeleton className="w-full h-full" />
+                  <Skeleton className="w-full h-4" />
                 </TableCell>
                 <TableCell className="w-1/3">
-                  <Skeleton className="w-full h-full" />
+                  <Skeleton className="w-full h-4" />
                 </TableCell>
               </TableRow>
             );
diff --git a/skyvern-frontend/src/routes/tasks/running/QueuedTasks.tsx b/skyvern-frontend/src/routes/tasks/running/QueuedTasks.tsx
index 65713166f..dda37fd56 100644
--- a/skyvern-frontend/src/routes/tasks/running/QueuedTasks.tsx
+++ b/skyvern-frontend/src/routes/tasks/running/QueuedTasks.tsx
@@ -28,6 +28,10 @@ function QueuedTasks() {
     ?.filter((task) => task.status === Status.Queued)
     .slice(0, 10);
 
+  if (queuedTasks?.length === 0) {
+    return <div>No queued tasks</div>;
+  }
+
   return (
     <Table>
       <TableHeader>