improvements: revise approval sequence table display and access request modal

fix(checkbox): Remove manual css overrides of checkbox checked state

frontend/src/components/v2/GenericFieldLabel/GenericFieldLabel.tsx

@@ -1,4 +1,6 @@
 import { ReactNode } from "react";
+import { IconDefinition } from "@fortawesome/free-solid-svg-icons";
+import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
 import { twMerge } from "tailwind-merge";
 
 type Props = {
@@ -7,6 +9,7 @@ type Props = {
   className?: string;
   labelClassName?: string;
   truncate?: boolean;
+  icon?: IconDefinition;
 };
 
 export const GenericFieldLabel = ({
@@ -14,11 +17,15 @@ export const GenericFieldLabel = ({
   children,
   className,
   labelClassName,
-  truncate
+  truncate,
+  icon
 }: Props) => {
   return (
     <div className={twMerge("min-w-0", className)}>
-      <p className={twMerge("text-xs font-medium text-mineshaft-400", labelClassName)}>{label}</p>
+      <div className="flex items-center gap-1.5">
+        {icon && <FontAwesomeIcon icon={icon} className="text-mineshaft-400" size="sm" />}
+        <p className={twMerge("text-xs font-medium text-mineshaft-400", labelClassName)}>{label}</p>
+      </div>
       {children ? (
         <p className={twMerge("text-sm text-mineshaft-100", truncate && "truncate")}>{children}</p>
       ) : (

frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/AccessApprovalRequest.tsx

@@ -247,9 +247,7 @@ export const AccessApprovalRequest = ({
         };
       else if (userReviewStatus === ApprovalStatus.APPROVED) {
         displayData = {
-          label: `Pending ${request.policy.approvals - request.reviewers.length} review${
-            request.policy.approvals - request.reviewers.length > 1 ? "s" : ""
-          }`,
+          label: "Pending Additional Reviews",
           type: "primary",
           icon: faClipboardCheck
         };

frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/ReviewAccessModal.tsx

@@ -1,10 +1,9 @@
-import { useCallback, useMemo, useState } from "react";
+import { ReactNode, useCallback, useMemo, useState } from "react";
 import {
-  faCheckCircle,
-  faCircle,
-  faTriangleExclamation,
-  faUsers,
-  faXmarkCircle
+  faBan,
+  faCheck,
+  faHourglass,
+  faTriangleExclamation
 } from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
 import ms from "ms";
@@ -15,12 +14,10 @@ import {
   Button,
   Checkbox,
   FormControl,
+  GenericFieldLabel,
   Input,
   Modal,
   ModalContent,
-  Popover,
-  PopoverContent,
-  PopoverTrigger,
   Tooltip
 } from "@app/components/v2";
 import { Badge } from "@app/components/v2/Badge";
@@ -38,10 +35,22 @@ import { groupBy } from "@app/lib/fn/array";
 
 const getReviewedStatusSymbol = (status?: ApprovalStatus) => {
   if (status === ApprovalStatus.APPROVED)
-    return <FontAwesomeIcon icon={faCheckCircle} size="xs" style={{ color: "#15803d" }} />;
+    return (
+      <Badge variant="success" className="flex h-4 items-center justify-center">
+        <FontAwesomeIcon icon={faCheck} size="xs" />
+      </Badge>
+    );
   if (status === ApprovalStatus.REJECTED)
-    return <FontAwesomeIcon icon={faXmarkCircle} size="xs" style={{ color: "#b91c1c" }} />;
-  return <FontAwesomeIcon icon={faCircle} size="xs" style={{ color: "#c2410c" }} />;
+    return (
+      <Badge variant="danger" className="flex h-4 items-center justify-center">
+        <FontAwesomeIcon icon={faBan} size="xs" />
+      </Badge>
+    );
+  return (
+    <Badge variant="primary" className="flex h-4 items-center justify-center">
+      <FontAwesomeIcon icon={faHourglass} size="xs" />
+    </Badge>
+  );
 };
 
 export const ReviewAccessRequestModal = ({
@@ -267,139 +276,160 @@ export const ReviewAccessRequestModal = ({
         </div>
         <div className="">
           <div className="mb-2 mt-4 text-mineshaft-200">
-            <div className="grid grid-cols-2 gap-4">
-              <div>
-                <div className="mb-1 text-xs font-semibold uppercase">Environment</div>
-                <div>{accessDetails.env || "-"}</div>
-              </div>
-              <div>
-                <div className="mb-1 text-xs font-semibold uppercase">Secret Path</div>
-                <div>{accessDetails.secretPath || "-"}</div>
-              </div>
-              <div>
-                <div className="mb-1 text-xs font-semibold uppercase">Access Type</div>
-                <div>{getAccessLabel()}</div>
-              </div>
-              <div>
-                <div className="mb-1 text-xs font-semibold uppercase">Permission</div>
-                <div>{requestedAccess}</div>
-              </div>
-              <div className="col-span-2">
-                <div className="mb-1 text-xs font-semibold uppercase">Note</div>
-                <div>{request.note || "-"}</div>
-              </div>
+            <div className="flex flex-wrap gap-8">
+              <GenericFieldLabel label="Environment">{accessDetails.env}</GenericFieldLabel>
+              <GenericFieldLabel truncate label="Secret Path">
+                {accessDetails.secretPath}
+              </GenericFieldLabel>
+              <GenericFieldLabel label="Access Type">{getAccessLabel()}</GenericFieldLabel>
+              <GenericFieldLabel label="Permission">{requestedAccess}</GenericFieldLabel>
+              {request.note && (
+                <GenericFieldLabel className="col-span-full" label="Note">
+                  {request.note}
+                </GenericFieldLabel>
+              )}
             </div>
           </div>
-          <div className="mb-4 border-b-2 border-mineshaft-500 py-2 text-lg">Approvers</div>
-          <div className="thin-scrollbar max-h-64 overflow-y-auto rounded p-2">
-            {approverSequence?.approvers?.map((approver, index) => (
-              <div
-                key={`approval-list-${index + 1}`}
-                className={twMerge(
-                  "relative mb-2 flex items-center rounded border border-mineshaft-500 bg-mineshaft-700 p-4",
-                  approverSequence?.currentSequence !== approver.sequence &&
-                    !hasApproved &&
-                    "text-mineshaft-400"
-                )}
-              >
-                <div>
-                  <div
-                    className={twMerge(
-                      "mr-8 flex h-8 w-8 items-center justify-center text-3xl font-medium",
-                      approver.hasApproved && "border-green-400 text-green-400",
-                      approver.hasRejected && "border-red-500 text-red-500"
-                    )}
-                  >
-                    {index + 1}
-                  </div>
-                  {index !== (approverSequence?.approvers?.length || 0) - 1 && (
-                    <div
-                      className={twMerge(
-                        "absolute bottom-0 left-8 h-5 border-r-2 border-gray-400",
-                        approver.hasApproved && "border-green-400",
-                        approver.hasRejected && "border-red-500"
-                      )}
-                    />
-                  )}
-                  {index !== 0 && (
-                    <div
-                      className={twMerge(
-                        "absolute left-8 top-0 h-5 border-r-2 border-gray-400",
-                        approver.hasApproved && "border-green-400",
-                        approver.hasRejected && "border-red-500"
-                      )}
-                    />
-                  )}
-                </div>
-                <div className="grid flex-grow grid-cols-3">
-                  <div>
-                    <div className="mb-1 text-xs font-semibold uppercase">Users</div>
-                    <div>
-                      {approver?.user
-                        ?.map(
-                          (el) => approverSequence?.membersGroupById?.[el.id]?.[0]?.user?.username
-                        )
-                        .join(",") || "-"}
-                    </div>
-                  </div>
-                  <div>
-                    <div className="mb-1 text-xs font-semibold uppercase">Groups</div>
-                    <div>
-                      {approver?.group
-                        ?.map(
-                          (el) =>
-                            approverSequence?.projectGroupsGroupById?.[el.id]?.[0]?.group?.name
-                        )
-                        .join(",") || "-"}
-                    </div>
-                  </div>
-                  <div className="flex items-center">
-                    <div>
-                      <div className="mb-1 text-xs font-semibold uppercase">Approvals Required</div>
-                      <div>{approver.approvals || "-"}</div>
-                    </div>
-                    <div className="ml-16">
-                      <Popover>
-                        <PopoverTrigger>
-                          <FontAwesomeIcon icon={faUsers} />
-                        </PopoverTrigger>
-                        <PopoverContent hideCloseBtn className="pt-3">
-                          <div>
-                            <div className="mb-1 text-sm text-bunker-300">Reviewers</div>
-                            <div className="thin-scrollbar flex max-h-64 flex-col gap-1 overflow-y-auto rounded">
-                              {approver.reviewers.map((el, idx) => (
-                                <div
-                                  key={`reviewer-${idx + 1}`}
-                                  className="flex items-center gap-2 bg-mineshaft-700 p-1 text-sm"
-                                >
-                                  <div className="flex-grow">{el.username}</div>
-                                  <Tooltip
-                                    content={`Status: ${el?.status || ApprovalStatus.PENDING}`}
-                                  >
-                                    {getReviewedStatusSymbol(el?.status as ApprovalStatus)}
-                                  </Tooltip>
-                                </div>
-                              ))}
-                            </div>
-                          </div>
-                        </PopoverContent>
-                      </Popover>
-                    </div>
-                  </div>
-                </div>
-              </div>
-            ))}
+
+          <div className="mt-4 flex items-center justify-between border-b-2 border-mineshaft-500 py-2">
+            <span>Approvers</span>
+            {approverSequence.isMyReviewInThisSequence &&
+              request.status === ApprovalStatus.PENDING && (
+                <Badge variant="primary" className="h-min">
+                  Awaiting Your Review
+                </Badge>
+              )}
+          </div>
+          <div className="thin-scrollbar max-h-[40vh] overflow-y-auto rounded py-2">
+            {approverSequence?.approvers &&
+              approverSequence.approvers.map((approver, index) => {
+                const isInactive =
+                  approverSequence?.currentSequence <
+                  (approver.sequence ?? approverSequence.approvers!.length);
+
+                const isPending = approverSequence?.currentSequence === approver.sequence;
+
+                let StepComponent: ReactNode;
+                let BadgeComponent: ReactNode = null;
+                if (approver.hasRejected) {
+                  StepComponent = (
+                    <Badge
+                      variant="danger"
+                      className="flex h-6 min-w-6 items-center justify-center"
+                    >
+                      <FontAwesomeIcon icon={faBan} />
+                    </Badge>
+                  );
+                  BadgeComponent = <Badge variant="danger">Rejected</Badge>;
+                } else if (approver.hasApproved) {
+                  StepComponent = (
+                    <Badge
+                      variant="success"
+                      className="flex h-6 min-w-6 items-center justify-center"
+                    >
+                      <FontAwesomeIcon icon={faCheck} />
+                    </Badge>
+                  );
+                  BadgeComponent = <Badge variant="success">Approved</Badge>;
+                } else if (isPending) {
+                  StepComponent = (
+                    <Badge
+                      variant="primary"
+                      className="flex h-6 min-w-6 items-center justify-center"
+                    >
+                      <FontAwesomeIcon icon={faHourglass} />
+                    </Badge>
+                  );
+                  BadgeComponent = <Badge variant="primary">Pending</Badge>;
+                } else {
+                  StepComponent = (
+                    <Badge
+                      className={
+                        isInactive
+                          ? "py-auto my-auto flex h-6 min-w-6 items-center justify-center gap-1.5 whitespace-nowrap bg-mineshaft-400/50 text-center text-bunker-200"
+                          : ""
+                      }
+                    >
+                      <span>{index + 1}</span>
+                    </Badge>
+                  );
+                }
+
+                return (
+                  <div
+                    key={`approval-list-${index + 1}`}
+                    className={twMerge("flex", isInactive && "opacity-50")}
+                  >
+                    {approverSequence.approvers!.length > 1 && (
+                      <div className="flex w-12 flex-col items-center gap-2 pr-4">
+                        <div
+                          className={twMerge(
+                            "flex-grow border-mineshaft-600",
+                            index !== 0 && "border-r"
+                          )}
+                        />
+                        {StepComponent}
+                        <div
+                          className={twMerge(
+                            "flex-grow border-mineshaft-600",
+                            index < approverSequence.approvers!.length - 1 && "border-r"
+                          )}
+                        />
+                      </div>
+                    )}
+                    <div className="grid flex-1 grid-cols-5 border-b border-mineshaft-600 p-4">
+                      <GenericFieldLabel className="col-span-2" label="Users">
+                        {approver?.user
+                          ?.map(
+                            (el) => approverSequence?.membersGroupById?.[el.id]?.[0]?.user?.username
+                          )
+                          .join(", ")}
+                      </GenericFieldLabel>
+                      <GenericFieldLabel className="col-span-2" label="Groups">
+                        {approver?.group
+                          ?.map(
+                            (el) =>
+                              approverSequence?.projectGroupsGroupById?.[el.id]?.[0]?.group?.name
+                          )
+                          .join(", ")}
+                      </GenericFieldLabel>
+                      <GenericFieldLabel label="Approvals Required">
+                        <div className="flex items-center">
+                          <span className="mr-2">{approver.approvals}</span>
+                          {BadgeComponent && (
+                            <Tooltip
+                              className="max-w-lg"
+                              content={
+                                <div>
+                                  <div className="mb-1 text-sm text-bunker-300">Reviewers</div>
+                                  <div className="thin-scrollbar flex max-h-64 flex-col divide-y divide-mineshaft-500 overflow-y-auto rounded">
+                                    {approver.reviewers.map((el, idx) => (
+                                      <div
+                                        key={`reviewer-${idx + 1}`}
+                                        className="flex items-center gap-2 px-2 py-2 text-sm"
+                                      >
+                                        <div className="flex-1">{el.username}</div>
+                                        {getReviewedStatusSymbol(el?.status as ApprovalStatus)}
+                                      </div>
+                                    ))}
+                                  </div>
+                                </div>
+                              }
+                            >
+                              <div>{BadgeComponent}</div>
+                            </Tooltip>
+                          )}
+                        </div>
+                      </GenericFieldLabel>
+                    </div>
+                  </div>
+                );
+              })}
           </div>
-          {approverSequence.isMyReviewInThisSequence &&
-            request.status === ApprovalStatus.PENDING && (
-              <div className="mb-4 rounded-r border-l-2 border-l-primary-400 bg-mineshaft-300/5 px-4 py-2.5 text-sm">
-                Awaiting review from you.
-              </div>
-            )}
           {shouldBlockRequestActions ? (
             <div
               className={twMerge(
-                "mb-4 rounded-r border-l-2 border-l-red-500 bg-mineshaft-300/5 px-4 py-2.5 text-sm",
+                "mt-2 rounded-r border-l-2 border-l-red-500 bg-mineshaft-300/5 px-4 py-2.5 text-sm",
                 isReviewedByMe && "border-l-green-400",
                 !approverSequence.isMyReviewInThisSequence && "border-l-primary-400",
                 hasRejected && "border-l-red-500"
@@ -409,41 +439,6 @@ export const ReviewAccessRequestModal = ({
             </div>
           ) : (
             <>
-              <div className="space-x-2">
-                <Button
-                  isLoading={isLoading === "approved"}
-                  isDisabled={
-                    Boolean(isLoading) ||
-                    (!(
-                      request.isApprover &&
-                      (!request.isRequestedByCurrentUser || request.isSelfApproveAllowed)
-                    ) &&
-                      !bypassApproval)
-                  }
-                  onClick={() => handleReview("approved")}
-                  className="mt-4"
-                  size="sm"
-                  colorSchema={!request.isApprover && isSoftEnforcement ? "danger" : "primary"}
-                >
-                  Approve Request
-                </Button>
-                <Button
-                  isLoading={isLoading === "rejected"}
-                  isDisabled={
-                    !!isLoading ||
-                    (!(
-                      request.isApprover &&
-                      (!request.isRequestedByCurrentUser || request.isSelfApproveAllowed)
-                    ) &&
-                      !bypassApproval)
-                  }
-                  onClick={() => handleReview("rejected")}
-                  className="mt-4 border-transparent bg-transparent text-mineshaft-200 hover:border-red hover:bg-red/20 hover:text-mineshaft-200"
-                  size="sm"
-                >
-                  Reject Request
-                </Button>
-              </div>
               {isSoftEnforcement &&
                 request.isRequestedByCurrentUser &&
                 !(request.isApprover && request.isSelfApproveAllowed) &&
@@ -453,11 +448,7 @@ export const ReviewAccessRequestModal = ({
                       onCheckedChange={(checked) => setBypassApproval(checked === true)}
                       isChecked={bypassApproval}
                       id="byPassApproval"
-                      checkIndicatorBg="text-white"
-                      className={twMerge(
-                        "mr-2",
-                        bypassApproval ? "border-red bg-red hover:bg-red-600" : ""
-                      )}
+                      className={twMerge("mr-2", bypassApproval ? "border-red/30 bg-red/10" : "")}
                     >
                       <span className="text-xs text-red">
                         Approve without waiting for requirements to be met (bypass policy
@@ -481,6 +472,42 @@ export const ReviewAccessRequestModal = ({
                     )}
                   </div>
                 )}
+              <div className="space-x-2">
+                <Button
+                  isLoading={isLoading === "approved"}
+                  isDisabled={
+                    Boolean(isLoading) ||
+                    (!(
+                      request.isApprover &&
+                      (!request.isRequestedByCurrentUser || request.isSelfApproveAllowed)
+                    ) &&
+                      !bypassApproval)
+                  }
+                  onClick={() => handleReview("approved")}
+                  className="mt-4"
+                  size="sm"
+                  variant="outline_bg"
+                  colorSchema={!request.isApprover && isSoftEnforcement ? "danger" : "primary"}
+                >
+                  Approve Request
+                </Button>
+                <Button
+                  isLoading={isLoading === "rejected"}
+                  isDisabled={
+                    !!isLoading ||
+                    (!(
+                      request.isApprover &&
+                      (!request.isRequestedByCurrentUser || request.isSelfApproveAllowed)
+                    ) &&
+                      !bypassApproval)
+                  }
+                  onClick={() => handleReview("rejected")}
+                  className="mt-4 border-transparent bg-transparent text-mineshaft-200 hover:border-red hover:bg-red/20 hover:text-mineshaft-200"
+                  size="sm"
+                >
+                  Reject Request
+                </Button>
+              </div>
             </>
           )}
         </div>

frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx

@@ -1,5 +1,12 @@
 import { useMemo } from "react";
-import { faEdit, faEllipsisV, faTrash } from "@fortawesome/free-solid-svg-icons";
+import {
+  faClipboardCheck,
+  faEdit,
+  faEllipsisV,
+  faTrash,
+  faUser,
+  faUserGroup
+} from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
 import { twMerge } from "tailwind-merge";
 
@@ -179,26 +186,40 @@ export const ApprovalPolicyRow = ({
             }`}
           >
             <div className="p-4">
-              <div className="mb-4 border-b-2 border-mineshaft-500 pb-2">Approvers</div>
+              <div className="border-b-2 border-mineshaft-500 pb-2">Approvers</div>
               {labels?.map((el, index) => (
-                <div
-                  key={`approval-list-${index + 1}`}
-                  className="relative mb-2 flex rounded border border-mineshaft-500 bg-mineshaft-800 p-4"
-                >
-                  <div className="my-auto mr-8 flex h-8 w-8 items-center justify-center rounded border border-mineshaft-400 bg-bunker-500/50 text-white">
-                    <div>{index + 1}</div>
-                  </div>
-                  {index !== labels.length - 1 && (
-                    <div className="absolute bottom-0 left-8 h-[1.25rem] border-r border-mineshaft-400" />
+                <div key={`approval-list-${index + 1}`} className="flex">
+                  {labels.length > 1 && (
+                    <div className="flex w-12 flex-col items-center gap-2 pr-4">
+                      <div
+                        className={twMerge(
+                          "flex-grow border-mineshaft-600",
+                          index !== 0 && "border-r"
+                        )}
+                      />
+                      {labels.length > 1 && (
+                        <Badge className="my-auto flex h-5 w-min min-w-5 items-center justify-center gap-1.5 whitespace-nowrap bg-mineshaft-400/50 text-center text-bunker-200">
+                          <span>{index + 1}</span>
+                        </Badge>
+                      )}
+                      <div
+                        className={twMerge(
+                          "flex-grow border-mineshaft-600",
+                          index < labels.length - 1 && "border-r"
+                        )}
+                      />
+                    </div>
                   )}
-                  {index !== 0 && (
-                    <div className="absolute left-8 top-0 h-[1.25rem] border-r border-mineshaft-400" />
-                  )}
-
-                  <div className="grid flex-grow grid-cols-3">
-                    <GenericFieldLabel label="Users">{el.userLabels}</GenericFieldLabel>
-                    <GenericFieldLabel label="Groups">{el.groupLabels}</GenericFieldLabel>
-                    <GenericFieldLabel label="Approvals Required">{el.approvals}</GenericFieldLabel>
+                  <div className="grid flex-1 grid-cols-5 border-b border-mineshaft-600 p-4">
+                    <GenericFieldLabel className="col-span-2" icon={faUser} label="Users">
+                      {el.userLabels}
+                    </GenericFieldLabel>
+                    <GenericFieldLabel className="col-span-2" icon={faUserGroup} label="Groups">
+                      {el.groupLabels}
+                    </GenericFieldLabel>
+                    <GenericFieldLabel icon={faClipboardCheck} label="Approvals Required">
+                      {el.approvals}
+                    </GenericFieldLabel>
                   </div>
                 </div>
               ))}