From f6ca7d1a95da517d28fb916969ce0a401b64cb69 Mon Sep 17 00:00:00 2001
From: Kirill Chernakov <yakiryous@gmail.com>
Date: Tue, 27 Aug 2024 16:29:15 +0400
Subject: [PATCH] feat: slightly different UX for provider tiles without hiding
 content on hover (#1736)

---
 keep-ui/app/providers/provider-tile.tsx       | 166 +++++++++---------
 keep-ui/app/providers/providers-tiles.tsx     |  52 +++---
 tests/e2e_tests/test_end_to_end.py            |  18 +-
 .../test_pushing_prometheus_alerts.py         |  18 +-
 4 files changed, 130 insertions(+), 124 deletions(-)

diff --git a/keep-ui/app/providers/provider-tile.tsx b/keep-ui/app/providers/provider-tile.tsx
index 3bfa29f477..51d0a4865a 100644
--- a/keep-ui/app/providers/provider-tile.tsx
+++ b/keep-ui/app/providers/provider-tile.tsx
@@ -1,13 +1,12 @@
 import {
   Badge,
-  Button,
   Icon,
   SparkAreaChart,
   Subtitle,
   Text,
   Title,
 } from "@tremor/react";
-import {Provider, TProviderLabels} from "./providers";
+import { Provider, TProviderLabels } from "./providers";
 import {
   BellAlertIcon,
   ChatBubbleBottomCenterIcon,
@@ -15,6 +14,7 @@ import {
   QueueListIcon,
   TicketIcon,
   MapIcon,
+  Cog6ToothIcon,
 } from "@heroicons/react/20/solid";
 import "./provider-tile.css";
 import moment from "moment";
@@ -88,8 +88,7 @@ const getEmptyDistribution = () => {
   return emptyDistribution;
 };
 
-
-function getIconForTag(tag:TProviderLabels) {
+function getIconForTag(tag: TProviderLabels) {
   switch (tag) {
     case "alert":
       return BellAlertIcon;
@@ -111,25 +110,59 @@ export default function ProviderTile({ provider, onClick }: Props) {
     if (provider.installed || provider.linked) {
       return null;
     }
-    return (<div className="labels flex flex-wrap group-hover:hidden gap-1">
-              {provider.tags.map((tag) => {
-                  return (
-                    <Badge
-                      key={tag}
-                      icon={getIconForTag(tag)}
-                      size="xs"
-                      color="slate"
-                    >
-                      <p>{tag}</p>
-                    </Badge>
-                  );
-                })}
-            </div>)
-  }
+    return (
+      <div className="labels flex flex-wrap gap-1">
+        {provider.tags.map((tag) => {
+          return (
+            <Badge key={tag} icon={getIconForTag(tag)} size="xs" color="slate">
+              <p>{tag}</p>
+            </Badge>
+          );
+        })}
+      </div>
+    );
+  };
+
+  const renderChart = () => {
+    const className = "mt-2 h-8 w-20 sm:h-10 sm:w-full";
+    if (!provider.installed && !provider.linked) {
+      return null;
+    }
+
+    if (provider.alertsDistribution && provider.alertsDistribution.length > 0) {
+      return (
+        <SparkAreaChart
+          data={addOneToDistribution(provider.alertsDistribution)}
+          categories={["number"]}
+          index={"hour"}
+          colors={["orange"]}
+          showGradient={true}
+          autoMinValue={true}
+          className={className}
+        />
+      );
+    }
+
+    return (
+      <SparkAreaChart
+        data={getEmptyDistribution()}
+        categories={["number"]}
+        index={"hour"}
+        colors={["orange"]}
+        className={className}
+        autoMinValue={true}
+        maxValue={1}
+      />
+    );
+  };
 
   return (
-    <div
-      className="tile-basis py-2 px-4 relative group flex justify-around items-center bg-white rounded-lg shadow h-44 hover:shadow-lg hover:grayscale-0 cursor-pointer gap-2"
+    <button
+      className={
+        "tile-basis text-left min-w-0 py-4 px-4 relative group flex justify-around items-center bg-white rounded-lg shadow hover:shadow-lg hover:grayscale-0 cursor-pointer gap-3" +
+        // Add fixed height only if provider card doesn't have much content
+        (!provider.installed && !provider.linked ? " h-32" : "")
+      }
       onClick={onClick}
     >
       <div className="flex-1 min-w-0">
@@ -158,7 +191,7 @@ export default function ProviderTile({ provider, onClick }: Props) {
           />
         )}
         {provider.installed ? (
-          <Text color={"green"} className="flex text-xs group-hover:hidden">
+          <Text color={"green"} className="flex text-xs">
             Connected
           </Text>
         ) : null}
@@ -169,24 +202,17 @@ export default function ProviderTile({ provider, onClick }: Props) {
         ) : null}
         <div className="flex flex-col gap-2">
           <div>
-            <Title
-              className={`${
-                !provider.linked ? "group-hover:hidden" : ""
-              } capitalize`}
-              title={provider.details?.name}
-            >
+            <Title className="capitalize" title={provider.details?.name}>
               {provider.display_name}{" "}
             </Title>
 
             {provider.details && provider.details.name && (
-              <Subtitle className="group-hover:hidden">
+              <Subtitle className="truncate">
                 id: {provider.details.name}
               </Subtitle>
             )}
             {provider.last_alert_received ? (
-              <Text
-                className={`${!provider.linked ? "group-hover:hidden" : ""}`}
-              >
+              <Text>
                 Last alert: {moment(provider.last_alert_received).fromNow()}
               </Text>
             ) : (
@@ -195,59 +221,35 @@ export default function ProviderTile({ provider, onClick }: Props) {
             {provider.linked && provider.id ? (
               <Text className="truncate">Id: {provider.id}</Text>
             ) : null}
-            {(provider.installed || provider.linked) &&
-            provider.alertsDistribution &&
-            provider.alertsDistribution.length > 0 ? (
-              <SparkAreaChart
-                data={addOneToDistribution(provider.alertsDistribution)}
-                categories={["number"]}
-                index={"hour"}
-                colors={["orange"]}
-                showGradient={true}
-                autoMinValue={true}
-                className={`${
-                  !provider.linked ? "group-hover:hidden" : ""
-                } mt-2 h-8 w-20 sm:h-10 sm:w-36`}
-              />
-            ) : provider.installed || provider.linked ? (
-              <SparkAreaChart
-                data={getEmptyDistribution()}
-                categories={["number"]}
-                index={"hour"}
-                colors={["orange"]}
-                className={`${
-                  !provider.linked ? "group-hover:hidden" : ""
-                } mt-2 h-8 w-20 sm:h-10 sm:w-36`}
-                autoMinValue={true}
-                maxValue={1}
-              />
-            ) : null}
+            {renderChart()}
           </div>
           {renderTags()}
-          {!provider.linked && (
-            <Button
-              variant="secondary"
-              size="xs"
-              color={provider.installed ? "orange" : "green"}
-              className="hidden group-hover:block pd-2"
-            >
-              {provider.installed ? "Modify" : "Connect"}
-            </Button>
-          )}
         </div>
       </div>
-      <ImageWithFallback
-        src={`/icons/${provider.type}-icon.png`}
-        fallbackSrc={`/icons/keep-icon.png`}
-        width={48}
-        height={48}
-        alt={provider.type}
-        className={`${
-          provider.installed || provider.linked
-            ? ""
-            : "grayscale group-hover:grayscale-0"
-        }`}
-      />
-    </div>
+      <div className="flex flex-col justify-center h-full">
+        <div className="flex-grow flex items-center">
+          <ImageWithFallback
+            src={`/icons/${provider.type}-icon.png`}
+            fallbackSrc={`/icons/keep-icon.png`}
+            width={48}
+            height={48}
+            alt={provider.type}
+            className={`${
+              provider.installed || provider.linked
+                ? ""
+                : "grayscale group-hover:grayscale-0"
+            }`}
+          />
+        </div>
+        {provider.installed ? (
+          <Icon
+            icon={Cog6ToothIcon}
+            color="gray"
+            className="w-6 h-6 self-end place-self-end"
+            tooltip="Modify"
+          />
+        ) : null}
+      </div>
+    </button>
   );
 }
diff --git a/keep-ui/app/providers/providers-tiles.tsx b/keep-ui/app/providers/providers-tiles.tsx
index 78a3411176..71b5e3af19 100644
--- a/keep-ui/app/providers/providers-tiles.tsx
+++ b/keep-ui/app/providers/providers-tiles.tsx
@@ -98,18 +98,31 @@ const ProvidersTiles = ({
     if (isConnected) handleCloseModal();
   };
 
+  const getSectionTitle = () => {
+    if (installedProvidersMode) {
+      return "Installed Providers";
+    }
+
+    if (linkedProvidersMode) {
+      return "Linked Providers";
+    }
+
+    return "Connect Provider";
+  };
+
+  const sortedProviders = providers.sort(
+    (a, b) =>
+      Number(b.can_setup_webhook) - Number(a.can_setup_webhook) ||
+      Number(b.supports_webhook) - Number(a.supports_webhook) ||
+      Number(b.oauth2_url ? true : false) - Number(a.oauth2_url ? true : false)
+  );
+
   return (
     <div>
       <div className="flex items-center mb-2.5">
-        <Title>
-          {installedProvidersMode
-            ? "Installed Providers"
-            : linkedProvidersMode
-            ? "Linked Providers"
-            : "Available Providers"}
-        </Title>
+        <Title>{getSectionTitle()}</Title>
         {linkedProvidersMode && (
-          <div className="ml-2 relative">
+          <div className="relative">
             <Icon
               icon={QuestionMarkCircleIcon} // Use the appropriate icon for your use case
               className="text-gray-400 hover:text-gray-600"
@@ -121,22 +134,13 @@ const ProvidersTiles = ({
       </div>
 
       <div className="flex flex-wrap mb-5 gap-5">
-        {providers
-          .filter(provider => Object.keys(provider.config || {}).length > 0 || (provider.tags && provider.tags.includes('alert')))
-          .sort(
-            (a, b) =>
-              Number(b.can_setup_webhook) - Number(a.can_setup_webhook) ||
-              Number(b.supports_webhook) - Number(a.supports_webhook) ||
-              Number(b.oauth2_url ? true : false) -
-                Number(a.oauth2_url ? true : false)
-          )
-          .map((provider) => (
-            <ProviderTile
-              key={provider.id}
-              provider={provider}
-              onClick={() => handleConnectProvider(provider)}
-            ></ProviderTile>
-          ))}
+        {sortedProviders.map((provider) => (
+          <ProviderTile
+            key={provider.id}
+            provider={provider}
+            onClick={() => handleConnectProvider(provider)}
+          ></ProviderTile>
+        ))}
       </div>
 
       <SlidingPanel
diff --git a/tests/e2e_tests/test_end_to_end.py b/tests/e2e_tests/test_end_to_end.py
index e9e3a0fac1..d6a09eb315 100644
--- a/tests/e2e_tests/test_end_to_end.py
+++ b/tests/e2e_tests/test_end_to_end.py
@@ -91,13 +91,13 @@ def test_providers_page_is_accessible(browser):
         )
         browser.goto("http://localhost:3000/providers")
         # get the GCP Monitoring provider
-        browser.locator("div").filter(
-            has_text=re.compile(r"^GCP Monitoring alertConnect$")
+        browser.locator("button").filter(
+            has_text=re.compile(r"^GCP Monitoring alert$")
         ).first.click()
         browser.get_by_role("button", name="Cancel").click()
         # connect resend provider
-        browser.locator("div").filter(
-            has_text=re.compile(r"^resend messagingConnect$")
+        browser.locator("button").filter(
+            has_text=re.compile(r"^resend messaging$")
         ).first.click()
         browser.get_by_placeholder("Enter provider name").click()
         random_provider_name = "".join(
@@ -106,14 +106,14 @@ def test_providers_page_is_accessible(browser):
         browser.get_by_placeholder("Enter provider name").fill(random_provider_name)
         browser.get_by_placeholder("Enter provider name").press("Tab")
         browser.get_by_placeholder("Enter api_key").fill("bla")
-        browser.get_by_role("button", name="Connect").click()
+        browser.get_by_role("button", name="Connect", exact=True).click()
         # wait a bit
         browser.wait_for_selector("text=Connected", timeout=15000)
         # make sure the provider is connected:
-        # find connected provider id label
-        id_label = browser.get_by_text(f"resend id: {random_provider_name}")
-        # click on parent div, the tile
-        id_label.locator('..').click()
+        # find and click the button containing the provider id in its nested elements
+        provider_button = browser.locator(f"button:has-text('{random_provider_name}')")
+        print(provider_button)
+        provider_button.click()
     except Exception:
         # Current file + test name for unique html and png dump.
         current_test_name = (
diff --git a/tests/e2e_tests/test_pushing_prometheus_alerts.py b/tests/e2e_tests/test_pushing_prometheus_alerts.py
index 1812d2c08c..8bf39f0cc5 100644
--- a/tests/e2e_tests/test_pushing_prometheus_alerts.py
+++ b/tests/e2e_tests/test_pushing_prometheus_alerts.py
@@ -34,12 +34,12 @@ def test_pulling_prometheus_alerts_to_provider(browser):
         browser.get_by_placeholder("Filter providers...").click()
         browser.get_by_placeholder("Filter providers...").fill("prometheus")
         browser.get_by_placeholder("Filter providers...").press("Enter")
-        browser.get_by_text("Available Providers").hover()
-        browser.locator("div").filter(
-            has_text=re.compile(r"^prometheus dataalertConnect$")
-        ).nth(1).hover()
+        browser.get_by_text("Connect Provider").hover()
+        browser.locator("button").filter(
+            has_text=re.compile(r"^prometheus dataalert$")
+        ).first.hover()
 
-        browser.get_by_role("button", name="Connect").click()
+        browser.locator("button:has-text('prometheus')").click()
         browser.get_by_placeholder("Enter provider name").click()
         browser.get_by_placeholder("Enter provider name").fill(provider_name)
         browser.get_by_placeholder("Enter url").click()
@@ -55,11 +55,11 @@ def test_pulling_prometheus_alerts_to_provider(browser):
         )
 
         browser.mouse.wheel(1000, 10000)  # Scroll down.
-        browser.get_by_role("button", name="Connect").click()
+        browser.get_by_role("button", name="Connect", exact=True).click()
 
         # Validate provider is created
         expect(
-            browser.locator("div")
+            browser.locator("button")
             .filter(has_text=re.compile(re.escape(provider_name)))
             .first
         ).to_be_visible()
@@ -92,7 +92,7 @@ def test_pulling_prometheus_alerts_to_provider(browser):
 
         # Delete provider
         browser.get_by_role("link", name="Providers").click()
-        browser.locator("div").filter(
+        browser.locator("button").filter(
             has_text=re.compile(re.escape(provider_name))
         ).first.hover()
         browser.locator(".tile-basis").first.click()
@@ -101,7 +101,7 @@ def test_pulling_prometheus_alerts_to_provider(browser):
 
         # Assert provider was deleted
         expect(
-            browser.locator("div")
+            browser.locator("button")
             .filter(has_text=re.compile(re.escape(provider_name)))
             .first
         ).not_to_be_visible()