From 1e3f492ce89c7e31b4502b4a5a75a6ef6ac3a2d1 Mon Sep 17 00:00:00 2001
From: Maina Wycliffe <wmmaina7@gmail.com>
Date: Thu, 2 Nov 2023 16:58:49 +0300
Subject: [PATCH] refactor: improve the canary modal layout

Closes #1288

fix: fix height issues

chore: show labels until full width

fix: remove bg issues

fix: fix height issue in the table
---
 .../CanaryPopup/CanaryCheckDetailsLabel.tsx   | 44 ----------
 .../Canary/CanaryPopup/CheckDetails.tsx       | 51 ++---------
 .../Canary/CanaryPopup/CheckLabels.tsx        | 85 +++++++++++++++++++
 .../StatusHistory/StatusHistory.tsx           |  7 +-
 src/components/Modal/index.tsx                |  4 +-
 src/components/Popover/Popover.tsx            |  4 +-
 src/components/TagList/TagList.tsx            |  2 +-
 7 files changed, 102 insertions(+), 95 deletions(-)
 delete mode 100644 src/components/Canary/CanaryPopup/CanaryCheckDetailsLabel.tsx
 create mode 100644 src/components/Canary/CanaryPopup/CheckLabels.tsx

diff --git a/src/components/Canary/CanaryPopup/CanaryCheckDetailsLabel.tsx b/src/components/Canary/CanaryPopup/CanaryCheckDetailsLabel.tsx
deleted file mode 100644
index e785ef3f9d..0000000000
--- a/src/components/Canary/CanaryPopup/CanaryCheckDetailsLabel.tsx
+++ /dev/null
@@ -1,44 +0,0 @@
-import { useEffect } from "react";
-import ReactTooltip from "react-tooltip";
-import { HealthCheck } from "../../../api/types/health";
-
-type CanaryCheckDetailsLabelProps = {
-  check?: Partial<HealthCheck>;
-};
-
-export function CanaryCheckDetailsLabel({
-  check
-}: CanaryCheckDetailsLabelProps) {
-  useEffect(() => {
-    ReactTooltip.rebuild();
-  });
-
-  if (!check?.labels) {
-    return null;
-  }
-
-  return (
-    <div className="flex flex-wrap ">
-      {Object.entries(check?.labels).map((entry) => {
-        const key = entry[0];
-        const value = entry[1] === "true" || entry[1] === true ? "" : entry[1];
-
-        return (
-          <div
-            data-tip={value === "" ? key : `${key}: ${value}`}
-            className="max-w-full h-auto overflow-hidden text-ellipsis bg-blue-100 border border-gray-300 px-1 py-0.75 m-0.5 rounded-md text-gray-600 font-semibold text-xs"
-            key={key}
-          >
-            {value === "" ? (
-              key
-            ) : (
-              <>
-                {key}: <span className="font-light">{value}</span>
-              </>
-            )}
-          </div>
-        );
-      })}
-    </div>
-  );
-}
diff --git a/src/components/Canary/CanaryPopup/CheckDetails.tsx b/src/components/Canary/CanaryPopup/CheckDetails.tsx
index 7cfe2a8b62..b8b24add3e 100644
--- a/src/components/Canary/CanaryPopup/CheckDetails.tsx
+++ b/src/components/Canary/CanaryPopup/CheckDetails.tsx
@@ -1,4 +1,4 @@
-import React, { Suspense, useMemo, useRef } from "react";
+import React, { Suspense, useRef } from "react";
 import { useCanaryGraphQuery } from "../../../api/query-hooks/health";
 import { HealthCheck } from "../../../api/types/health";
 import {
@@ -7,15 +7,12 @@ import {
 } from "../../../utils/common";
 import { usePrevious } from "../../../utils/hooks";
 import mixins from "../../../utils/mixins.module.css";
-import { AccordionBox } from "../../AccordionBox";
 import { DropdownStandaloneWrapper } from "../../Dropdown/StandaloneWrapper";
 import { TimeRange, timeRanges } from "../../Dropdown/TimeRange";
-import { Age } from "../../../ui/Age";
 import { Duration } from "../renderers";
-import { CanaryCheckDetailsLabel } from "./CanaryCheckDetailsLabel";
 import { CanaryCheckDetailsSpecTab } from "./CanaryCheckDetailsSpec";
+import CheckLabels from "./CheckLabels";
 import { CheckStat } from "./CheckStat";
-import { DetailField } from "./DetailField";
 import { StatusHistory } from "./StatusHistory/StatusHistory";
 import { PopupTabs } from "./tabs";
 import { getUptimePercentage } from "./utils";
@@ -47,23 +44,9 @@ export function CheckDetails({ check, timeRange, ...rest }: CheckDetailsProps) {
   const validUptime =
     !Number.isNaN(validCheck?.uptime?.passed) &&
     !Number.isNaN(validCheck?.uptime?.failed);
-  const severityValue = validCheck?.severity || "-";
 
-  const details: Record<string, React.ReactNode> = useMemo(
-    () => ({
-      Labels: <CanaryCheckDetailsLabel check={validCheck} />,
-      Owner: validCheck?.owner || "-",
-      Interval: validCheck?.interval || "-",
-      Location: validCheck?.location || "-",
-      Schedule: validCheck?.schedule || "-",
-      "Last Runtime": validCheck?.lastRuntime ? (
-        <Age from={validCheck.lastRuntime} />
-      ) : (
-        "-"
-      )
-    }),
-    [validCheck]
-  );
+  const severityValue =
+    validCheck?.severity ?? validCheck?.spec?.severity ?? "-";
 
   if (validCheck == null) {
     return null;
@@ -72,6 +55,7 @@ export function CheckDetails({ check, timeRange, ...rest }: CheckDetailsProps) {
   return (
     <div {...rest} ref={containerRef}>
       <div className="flex flex-col" ref={statsRef}>
+        <CheckLabels check={validCheck} />
         <div className="flex flex-row flex-wrap">
           <CheckStat
             containerClassName="w-52 mb-4"
@@ -129,7 +113,6 @@ export function CheckDetails({ check, timeRange, ...rest }: CheckDetailsProps) {
           </div>
           <div className="w-full h-52 overflow-visible">
             <Suspense fallback={<div>Loading..</div>}>
-              {/*  */}
               <CanaryStatusChart timeRange={timeRange} check={validCheck} />
             </Suspense>
           </div>
@@ -156,7 +139,7 @@ export function CheckDetails({ check, timeRange, ...rest }: CheckDetailsProps) {
             content: (
               <div
                 key="status-history"
-                className={`border border-b-0 border-gray-300 bg-gray-50 overflow-y-auto flex flex-col flex-1 relative -mb-px ${mixins.appleScrollbar}`}
+                className={`border border-b-0 border-gray-300 bg-white overflow-y-auto flex flex-col flex-1 relative -mb-px ${mixins.appleScrollbar}`}
               >
                 <StatusHistory timeRange={timeRange} check={validCheck} />
               </div>
@@ -164,28 +147,6 @@ export function CheckDetails({ check, timeRange, ...rest }: CheckDetailsProps) {
             class:
               "flex-1 flex flex-col overflow-y-hidden border-b h-full border-gray-300"
           },
-          checkDetails: {
-            label: "Check details",
-            content: (
-              <div key="check-details" className="px-6 py-6 h-full">
-                <AccordionBox
-                  content={
-                    <div className="flex flex-row flex-wrap">
-                      {Object.entries(details).map(([label, value]) => (
-                        <DetailField
-                          key={label}
-                          className="w-1/3 mb-3 whitespace-nowrap"
-                          label={label}
-                          value={value as any}
-                        />
-                      ))}
-                    </div>
-                  }
-                />
-              </div>
-            ),
-            class: `flex-1 flex flex-col overflow-y-auto  border border-gray-300 ${mixins.appleScrollbar}`
-          },
           specs: {
             label: "Spec",
             content: <CanaryCheckDetailsSpecTab check={validCheck} />,
diff --git a/src/components/Canary/CanaryPopup/CheckLabels.tsx b/src/components/Canary/CanaryPopup/CheckLabels.tsx
new file mode 100644
index 0000000000..d657045b3a
--- /dev/null
+++ b/src/components/Canary/CanaryPopup/CheckLabels.tsx
@@ -0,0 +1,85 @@
+import { useLayoutEffect, useMemo, useState } from "react";
+import { HealthCheck } from "../../../api/types/health";
+import Popover from "../../Popover/Popover";
+import { TagItem, TagList } from "../../TagList/TagList";
+
+type Props = {
+  check: Partial<Pick<HealthCheck, "labels">>;
+};
+
+export default function CheckLabels({ check }: Props) {
+  const [isOverflowing, setIsOverflowing] = useState(false);
+
+  const checkLabels = useMemo(() => {
+    if (check?.labels) {
+      return Object.entries(check.labels).map(([key, value]) => ({
+        key,
+        value
+      }));
+    }
+    return [];
+  }, [check?.labels]);
+
+  useLayoutEffect(() => {
+    const node = document.getElementById("labels-container");
+    if (node) {
+      const isOverflowing =
+        node.scrollWidth > node.clientWidth ||
+        node.scrollHeight > node.clientHeight;
+
+      if (isOverflowing) {
+        const childNodes = Array.from(node.childNodes) as HTMLDivElement[];
+        let width = 0;
+
+        childNodes.forEach((childNode) => {
+          width += childNode.clientWidth;
+          if (width > node.clientWidth) {
+            childNode.style.display = "none";
+          }
+        });
+      }
+      setIsOverflowing(isOverflowing);
+    }
+  }, [checkLabels]);
+
+  if (checkLabels.length === 0) {
+    return null;
+  }
+
+  return (
+    <div className="flex flex-row gap-2 mb-4 items-center w-full">
+      <span className="text-sm font-medium">Labels:</span>
+      <Popover
+        menuClass="w-auto top-6"
+        toggle={
+          <div className="flex flex-row flex-1 items-center">
+            <div
+              id="labels-container"
+              className="flex w-auto flex-row flex-shrink overflow-hidden h-7 gap-2 flex-wrap cursor-pointer"
+            >
+              {checkLabels.map((item) => (
+                <TagItem key={item.key} tag={item} />
+              ))}
+            </div>
+            {isOverflowing && (
+              <div className="w-auto gap-2 underline decoration-solid justify-left text-xs cursor-pointer">
+                +{checkLabels.length - 1} more
+              </div>
+            )}
+          </div>
+        }
+        placement="left"
+      >
+        <div className="flex flex-col p-1">
+          <div className="flex flex-col items-stretch max-h-96 overflow-y-auto">
+            <TagList
+              className="flex flex-col flex-1"
+              tags={checkLabels}
+              minimumItemsToShow={checkLabels.length}
+            />
+          </div>
+        </div>
+      </Popover>
+    </div>
+  );
+}
diff --git a/src/components/Canary/CanaryPopup/StatusHistory/StatusHistory.tsx b/src/components/Canary/CanaryPopup/StatusHistory/StatusHistory.tsx
index 618cad4499..23cf798933 100644
--- a/src/components/Canary/CanaryPopup/StatusHistory/StatusHistory.tsx
+++ b/src/components/Canary/CanaryPopup/StatusHistory/StatusHistory.tsx
@@ -162,7 +162,10 @@ export function StatusHistory({
 
   return (
     <div
-      className={clsx("w-full flex flex-col flex-1 overflow-y-auto", className)}
+      className={clsx(
+        "w-full flex flex-col flex-1 overflow-y-auto bg-white",
+        className
+      )}
       {...props}
     >
       <StatusHistoryFilters onFiltersChanges={onFiltersChange} />
@@ -173,7 +176,7 @@ export function StatusHistory({
             columns={columns}
             data={statii}
             tableStyle={{ borderSpacing: "0" }}
-            className="flex-1"
+            className=""
             pagination={pagination}
             paginationClassName="px-2 pb-2"
             preferencesKey="health-check-status-list"
diff --git a/src/components/Modal/index.tsx b/src/components/Modal/index.tsx
index 779b52247c..696dd80b32 100644
--- a/src/components/Modal/index.tsx
+++ b/src/components/Modal/index.tsx
@@ -27,6 +27,7 @@ interface IModalProps {
   size: ModalSize;
   children: React.ReactNode;
   containerClassName?: string;
+  dialogClassName?: string;
 }
 
 export function Modal({
@@ -42,6 +43,7 @@ export function Modal({
   size,
   children,
   containerClassName = "overflow-auto max-h-full",
+  dialogClassName = "fixed z-50 inset-0 overflow-y-auto 2xl:my-20",
   ...rest
 }: IModalProps) {
   return (
@@ -50,7 +52,7 @@ export function Modal({
       <Dialog
         as="div"
         auto-reopen="true"
-        className="fixed z-50 inset-0 overflow-y-auto my-20"
+        className={dialogClassName}
         onClose={allowBackgroundClose ? () => onClose() : () => {}}
         {...rest}
       >
diff --git a/src/components/Popover/Popover.tsx b/src/components/Popover/Popover.tsx
index a39931a41e..6842985c9a 100644
--- a/src/components/Popover/Popover.tsx
+++ b/src/components/Popover/Popover.tsx
@@ -19,7 +19,7 @@ export default function Popover({
   children,
   placement = "right",
   className,
-  menuClass = "top-6",
+  menuClass = "top-6 w-56",
   toggle,
   autoCloseTimeInMS = 5000,
   ...props
@@ -99,7 +99,7 @@ export default function Popover({
         aria-orientation="vertical"
         aria-labelledby="menu-button"
         className={clsx(
-          "flex flex-col origin-top-right absolute w-96 z-50 divide-y divide-gray-100 rounded-md drop-shadow-xl bg-slate-50 ring-1 ring-black ring-opacity-5 focus:outline-none",
+          "flex flex-col origin-top-right absolute z-50 divide-y divide-gray-100 rounded-md drop-shadow-xl bg-slate-50 ring-1 ring-black ring-opacity-5 focus:outline-none",
           isPopoverOpen ? "display-block" : "hidden",
           placement === "right" ? "right-0" : "left-0",
           menuClass
diff --git a/src/components/TagList/TagList.tsx b/src/components/TagList/TagList.tsx
index 9a7e176318..2cc4ee2fb5 100644
--- a/src/components/TagList/TagList.tsx
+++ b/src/components/TagList/TagList.tsx
@@ -27,7 +27,7 @@ export function TagItem({ tag: { key, value }, containerWidth }: TagItemProps) {
       data-tip={`${key}:${value}`}
     >
       <div
-        className="flex flex-row space-x-1 font-semibold p-[0.2rem] text-gray-600 text-xs truncate"
+        className="flex flex-row space-x-1 font-semibold p-[0.2rem] text-gray-600 text-xs whitespace-nowrap break-inside-avoid-column"
         style={containerWidth ? { width: containerWidth } : {}}
       >
         <span className="inline">{key}:</span>