fix(ui): apply Slack integration design feedback (#12677)

ui/components/integrations/slack/slack-channel-multi-select.tsx

@@ -1,7 +1,8 @@
 "use client";
 
-import { Lock, RefreshCw } from "lucide-react";
+import { RefreshCw } from "lucide-react";
 
+import { SlackInlineCode } from "@/components/integrations/slack/slack-inline-code";
 import {
   Alert,
   AlertDescription,
@@ -10,6 +11,7 @@ import {
   Button,
   Label,
 } from "@/components/shadcn";
+import { CustomLink } from "@/components/shadcn/custom/custom-link";
 import {
   MultiSelect,
   MultiSelectContent,
@@ -17,10 +19,23 @@ import {
   MultiSelectTrigger,
   MultiSelectValue,
 } from "@/components/shadcn/select/multiselect";
+import { DOCS_URLS } from "@/lib/external-urls";
 import type { SlackChannelOption } from "@/types/integrations";
 
-const INVITE_HINT =
-  "A private channel only appears here after someone invites @Prowler to it in Slack. Invite it, then refresh.";
+const INVITE_HINT = (
+  <>
+    A private channel only appears here after someone invites{" "}
+    <SlackInlineCode>@Prowler Cloud</SlackInlineCode> to it in Slack. Invite it,
+    then refresh.{" "}
+    <CustomLink
+      href={DOCS_URLS.SLACK_INTEGRATION_PRIVATE_CHANNELS}
+      ariaLabel="Learn more about why a private channel is missing from the channel list"
+      size="xs"
+    >
+      Learn more
+    </CustomLink>
+  </>
+);
 
 interface SlackChannelMultiSelectProps {
   options: SlackChannelOption[];
@@ -38,15 +53,15 @@ interface SlackChannelMultiSelectProps {
   describedBy?: string;
 }
 
+/** Marked exactly as the listing marks it, so the chip is the row it came from. */
 const chipLabel = (option: SlackChannelOption) => (
   <span className="flex min-w-0 items-center gap-1">
-    {option.is_private && (
-      <>
-        <Lock size={12} aria-hidden="true" />
-        <span className="sr-only">Private</span>
-      </>
-    )}
     <span className="truncate">#{option.name}</span>
+    {option.is_private && (
+      <Badge variant="tag" size="sm">
+        Private
+      </Badge>
+    )}
   </span>
 );
 
@@ -104,8 +119,10 @@ export const SlackChannelMultiSelect = ({
           <AlertTitle>No channels available yet</AlertTitle>
           <AlertDescription>
             Prowler cannot see a single channel in this workspace. Create a
-            public channel, or invite @Prowler to a private one in Slack with
-            <span className="font-medium"> /invite @Prowler</span>, then
+            public channel, or invite{" "}
+            <SlackInlineCode>@Prowler Cloud</SlackInlineCode> to a private one
+            in Slack with{" "}
+            <SlackInlineCode>/invite @Prowler Cloud</SlackInlineCode>, then
             refresh.
           </AlertDescription>
         </Alert>

ui/components/integrations/slack/slack-connection-check-status.tsx

@@ -0,0 +1,124 @@
+import { Check, CircleAlert, Loader2, type LucideIcon } from "lucide-react";
+import type { ReactNode } from "react";
+
+import { cn } from "@/lib/utils";
+
+// Read by the page harness off `data-connection-check-status`.
+export const CHECK_STATUS = {
+  /** No check has run against the channels currently on record. */
+  IDLE: "idle",
+  RUNNING: "running",
+  PASSED: "passed",
+  FAILED: "failed",
+} as const;
+
+export type CheckStatus = (typeof CHECK_STATUS)[keyof typeof CHECK_STATUS];
+
+interface CheckStatusStyle {
+  /** `null` for the resting state, which shows no outcome to decorate. */
+  icon: LucideIcon | null;
+  className: string;
+}
+
+const CHECK_STATUS_STYLES = {
+  [CHECK_STATUS.IDLE]: {
+    icon: null,
+    className: "",
+  },
+  [CHECK_STATUS.RUNNING]: {
+    icon: Loader2,
+    className:
+      "border-border-neutral-secondary bg-bg-neutral-tertiary text-text-neutral-secondary",
+  },
+  // The dark pass tokens are sized for a whole card, and at this width they
+  // read as an alert rather than a note, so they are taken down a shade there.
+  [CHECK_STATUS.PASSED]: {
+    icon: Check,
+    className:
+      "border-bg-pass bg-bg-pass-secondary text-text-success-primary dark:border-bg-pass/40 dark:bg-bg-pass-secondary/40",
+  },
+  [CHECK_STATUS.FAILED]: {
+    icon: CircleAlert,
+    className:
+      "border-border-error bg-bg-fail-secondary text-text-error-primary dark:border-border-error/50",
+  },
+} as const satisfies Record<CheckStatus, CheckStatusStyle>;
+
+interface SlackConnectionCheckStatusProps {
+  status: CheckStatus;
+  /** What the last check found. Absent while no check covers the record. */
+  outcome?: ReactNode;
+  /**
+   * Id the control points at with `aria-describedby`. The caption alone carries
+   * it: an outcome read out as the control's description would say what the
+   * last check did, where the description has to say what pressing it does.
+   */
+  captionId: string;
+  /** What the check does, or why it cannot run. */
+  children: ReactNode;
+}
+
+/**
+ * The connection check's standing state: what a check last found, over the
+ * caption explaining what running one does.
+ *
+ * The two are kept in separate registers — the outcome boxed, the caption plain
+ * underneath — because they are answers to different questions and can honestly
+ * disagree. A refused channel is still a channel the next check will try, so
+ * reading the caption as a continuation of the outcome ("Slack refused #ops" …
+ * "posts the confirmation to #ops") would turn a true pair of statements into a
+ * contradiction.
+ */
+export const SlackConnectionCheckStatus = ({
+  status,
+  outcome,
+  captionId,
+  children,
+}: SlackConnectionCheckStatusProps) => {
+  const { icon: Icon, className } = CHECK_STATUS_STYLES[status];
+
+  return (
+    <div
+      data-connection-check-status={status}
+      className={cn("flex w-full flex-col", outcome && "gap-2")}
+    >
+      <div
+        className={cn(
+          "flex items-start gap-2 text-xs",
+          outcome && "rounded-lg border px-3 py-2",
+          outcome && className,
+        )}
+      >
+        {Icon && outcome && (
+          <Icon
+            aria-hidden="true"
+            className={cn(
+              "mt-0.5 size-3.5 shrink-0",
+              status === CHECK_STATUS.RUNNING && "animate-spin",
+            )}
+          />
+        )}
+        {/* Always mounted, so a landing outcome is announced as a change to a
+            region that was already there — and polite, since the user asked
+            for the check and is watching it. Empty, it takes no room.
+
+            Keyed, because the check's toast carries this same refusal copy: a
+            reader going by text alone would answer for the toast, and pass
+            with this line never rendered. */}
+        <p
+          data-connection-check-outcome
+          aria-live="polite"
+          className={cn("font-medium", !outcome && "sr-only")}
+        >
+          {outcome}
+        </p>
+      </div>
+      <p
+        id={captionId}
+        className="text-text-neutral-tertiary max-w-prose text-xs"
+      >
+        {children}
+      </p>
+    </div>
+  );
+};

ui/components/integrations/slack/slack-inline-code.tsx

@@ -0,0 +1,15 @@
+import type { ReactNode } from "react";
+
+interface SlackInlineCodeProps {
+  children: ReactNode;
+}
+
+/**
+ * A Slack identifier — a channel name, the bot mention, a slash command — set
+ * apart from the prose around it.
+ */
+export const SlackInlineCode = ({ children }: SlackInlineCodeProps) => (
+  <code className="bg-bg-neutral-tertiary text-text-neutral-secondary rounded px-1 py-0.5 font-mono">
+    {children}
+  </code>
+);

ui/components/integrations/slack/slack-integration-card.tsx

@@ -4,9 +4,7 @@ import Link from "next/link";
 import { SlackIcon } from "@/components/icons/services/IconServices";
 import { Button, Card, CardContent, CardHeader } from "@/components/shadcn";
 import { CustomLink } from "@/components/shadcn/custom/custom-link";
-
-const SLACK_DOCS_URL =
-  "https://docs.prowler.com/user-guide/tutorials/prowler-app-slack-integration";
+import { DOCS_URLS } from "@/lib/external-urls";
 
 export const SlackIntegrationCard = () => {
   return (
@@ -24,7 +22,7 @@ export const SlackIntegrationCard = () => {
                   Send Prowler messages to your Slack workspace.
                 </p>
                 <CustomLink
-                  href={SLACK_DOCS_URL}
+                  href={DOCS_URLS.SLACK_INTEGRATION}
                   aria-label="Learn more about Slack integration"
                   size="xs"
                 >

ui/components/integrations/slack/slack-integration-manager.tsx

@@ -2,7 +2,7 @@
 
 import { format, isValid, parseISO } from "date-fns";
 import { TestTube, Unplug } from "lucide-react";
-import { useEffect, useState } from "react";
+import { type ReactNode, useEffect, useState } from "react";
 
 import { testIntegrationConnection } from "@/actions/integrations/integrations";
 import {
@@ -14,6 +14,12 @@ import {
 import { SlackIcon } from "@/components/icons/services/IconServices";
 import { IntegrationCardHeader } from "@/components/integrations/shared";
 import { SlackChannelMultiSelect } from "@/components/integrations/slack/slack-channel-multi-select";
+import {
+  CHECK_STATUS,
+  type CheckStatus,
+  SlackConnectionCheckStatus,
+} from "@/components/integrations/slack/slack-connection-check-status";
+import { SlackInlineCode } from "@/components/integrations/slack/slack-inline-code";
 import {
   Alert,
   AlertDescription,
@@ -76,6 +82,20 @@ interface UnconfirmedRevocation {
   workspace: string | null;
 }
 
+/**
+ * What the last check found, together with what it was measured against. Both
+ * ride with the finding so it can never outlive them: a green line vouching for
+ * a channel nothing ever reached is worse than no line at all, and so is one
+ * standing over a notice saying the credential is dead.
+ */
+interface ConnectionCheckOutcome {
+  reachable: boolean;
+  summary: string;
+  channelIds: string[];
+  /** The credential verdict the check was run under. */
+  credentialFailure: SlackTokenErrorCode | null;
+}
+
 /** Order-insensitive: the mirror must not re-seed on a mere reordering. */
 const sameChannelIds = (a: string[], b: string[]) =>
   a.length === b.length && new Set([...a, ...b]).size === a.length;
@@ -129,10 +149,39 @@ const recordedFromSelection = (
     ];
   });
 
+const CHANNEL_LIST_FORMAT = new Intl.ListFormat("en", {
+  style: "long",
+  type: "conjunction",
+});
+
+const channelTokens = (names: string[]): string[] =>
+  names.map((name) => `#${name}`);
+
+/** For the copy that travels as plain text: toasts, the deauthorize warning. */
 const channelList = (names: string[]): string =>
-  new Intl.ListFormat("en", { style: "long", type: "conjunction" }).format(
-    names.map((name) => `#${name}`),
-  );
+  CHANNEL_LIST_FORMAT.format(channelTokens(names));
+
+interface StyledChannelListProps {
+  names: string[];
+}
+
+/**
+ * The same list as `channelList()`, each channel set apart. `formatToParts`
+ * emits exactly the literals `format` joins, so the rendered text is unchanged
+ * (design D3) — the copy is read as text content, here and by the tests.
+ */
+const StyledChannelList = ({ names }: StyledChannelListProps) => (
+  <>
+    {CHANNEL_LIST_FORMAT.formatToParts(channelTokens(names)).map(
+      (part, index) =>
+        part.type === "element" ? (
+          <SlackInlineCode key={index}>{part.value}</SlackInlineCode>
+        ) : (
+          part.value
+        ),
+    )}
+  </>
+);
 
 /**
  * Slack's own reason, when the string is one: the connection check reports a
@@ -160,6 +209,10 @@ export const SlackIntegrationManager = ({
   loadError,
 }: SlackIntegrationManagerProps) => {
   const [isTesting, setIsTesting] = useState(false);
+  // A check's finding outlives its toast: the card keeps saying where the
+  // workspace stands until a change to the channels makes the finding moot.
+  const [checkOutcome, setCheckOutcome] =
+    useState<ConnectionCheckOutcome | null>(null);
   const [isDisconnectOpen, setIsDisconnectOpen] = useState(false);
   const [isDisconnecting, setIsDisconnecting] = useState(false);
   // The row is gone the moment the API says so; the server component's
@@ -317,15 +370,55 @@ export const SlackIntegrationManager = ({
     (channel) => !selectedChannelIds.includes(channel.id),
   );
 
-  const checkHint = (): string => {
+  const authorizedChannelIds = authorizedChannels.map((channel) => channel.id);
+
+  /**
+   * Derived, not stored, so a finding retires on its own the moment anything it
+   * was measured against moves: a channel authorized or dropped leaves a set no
+   * check has covered, and a credential verdict that has since changed leaves a
+   * finding taken under conditions that no longer hold.
+   */
+  const coveredOutcome =
+    checkOutcome &&
+    sameChannelIds(checkOutcome.channelIds, authorizedChannelIds) &&
+    checkOutcome.credentialFailure === credentialFailure
+      ? checkOutcome
+      : null;
+
+  const checkStatus = (): CheckStatus => {
+    if (isTesting) return CHECK_STATUS.RUNNING;
+    if (!coveredOutcome) return CHECK_STATUS.IDLE;
+    return coveredOutcome.reachable ? CHECK_STATUS.PASSED : CHECK_STATUS.FAILED;
+  };
+
+  // Deliberately not a second telling of the caption below it: the button
+  // already reads "Testing...", so this only has to say a check is under way.
+  const checkSummary = isTesting
+    ? "Checking the connection..."
+    : coveredOutcome?.summary;
+
+  /** What a passing check proves, said in terms of what was actually reached. */
+  const reachableSummary = (channels: SlackAuthorizedChannel[]): string =>
+    channels.length === 1
+      ? `#${channels[0].name} is reachable.`
+      : `All ${channels.length} channels are reachable.`;
+
+  const checkHint = (): ReactNode => {
     if (authorizedChannels.length === 0) {
       return "Authorize at least one destination channel below to enable this check.";
     }
-    return unconfirmedChannels.length > 0
-      ? `Checks every authorized channel and posts “${CONFIRMATION_MESSAGE}” once to ${channelList(
-          unconfirmedChannels.map((channel) => channel.name),
-        )}.`
-      : "Checks every authorized channel. Each was confirmed once already, so nothing is posted.";
+    return unconfirmedChannels.length > 0 ? (
+      <>
+        Checks every authorized channel and posts “{CONFIRMATION_MESSAGE}” once
+        to{" "}
+        <StyledChannelList
+          names={unconfirmedChannels.map((channel) => channel.name)}
+        />
+        .
+      </>
+    ) : (
+      "Checks every authorized channel. Nothing is posted."
+    );
   };
 
   const handleSaveChannels = async () => {
@@ -395,12 +488,24 @@ export const SlackIntegrationManager = ({
     // Passed in by a chained check: it runs before the save's state lands.
     channels: SlackAuthorizedChannel[] = authorizedChannels,
   ) => {
+    const checkedChannelIds = channels.map((channel) => channel.id);
+
     setIsTesting(true);
     try {
       const result = await testIntegrationConnection(id);
 
       if (result.success) {
         provedCredentialAlive();
+        setCheckOutcome({
+          reachable: true,
+          // Prowler's own count, not the API's prose: the line has to keep
+          // meaning the same thing every time it is read.
+          summary: reachableSummary(channels),
+          channelIds: checkedChannelIds,
+          // An answer at all clears the verdict, so a pass is always taken
+          // under a live credential.
+          credentialFailure: null,
+        });
         toast({
           title: "Connection test successful!",
           description:
@@ -411,8 +516,13 @@ export const SlackIntegrationManager = ({
         // A dead credential named here is not a failure checking again can
         // fix, so the reason is recorded and not only reported.
         const reason = result.error?.trim() || null;
+        const reasonCode = asReasonCode(reason);
 
-        recordRefusal(asReasonCode(reason));
+        recordRefusal(reasonCode);
+
+        // The verdict this check leaves behind: its own reason when it named
+        // one, else the last answer's, which nothing here contradicted.
+        const codeAfterCheck = reasonCode ?? lastRefusalCode;
 
         const explanation = reason
           ? slackErrorMessage({ code: reason, detail: reason })
@@ -424,15 +534,32 @@ export const SlackIntegrationManager = ({
           ? channels.find((channel) => channel.id === result.failedChannelId)
           : undefined;
 
+        const refusal = refusedChannel
+          ? `Slack refused #${refusedChannel.name}: ${explanation}`
+          : explanation;
+
+        setCheckOutcome({
+          reachable: false,
+          summary: refusal,
+          channelIds: checkedChannelIds,
+          credentialFailure: isSlackTokenErrorCode(codeAfterCheck)
+            ? codeAfterCheck
+            : null,
+        });
         toast({
           variant: "destructive",
           title: "Connection test failed",
-          description: refusedChannel
-            ? `Slack refused #${refusedChannel.name}: ${explanation}`
-            : explanation,
+          description: refusal,
         });
       }
     } catch (_error) {
+      setCheckOutcome({
+        reachable: false,
+        summary: "Prowler could not reach Slack to run the check.",
+        channelIds: checkedChannelIds,
+        // Never an answer from Slack, so the standing verdict is untouched.
+        credentialFailure,
+      });
       toast({
         variant: "destructive",
         title: "Error",
@@ -607,11 +734,18 @@ export const SlackIntegrationManager = ({
         </Alert>
       ) : integration && !disconnected ? (
         <Card variant="base">
-          <CardHeader>
+          <CardHeader className="gap-3">
             <IntegrationCardHeader
               icon={<SlackIcon size={32} />}
               title={`Connected to ${workspaceName ?? "your Slack workspace"}`}
               subtitle="Prowler posts to this workspace only."
+              meta={
+                lastCheckedOn && (
+                  <p className="text-text-neutral-tertiary font-mono text-xs">
+                    Last checked {lastCheckedOn}
+                  </p>
+                )
+              }
               connectionStatus={{
                 // A dead token outranks the state the page was loaded with.
                 connected:
@@ -619,21 +753,8 @@ export const SlackIntegrationManager = ({
                     ? integration.attributes.connected
                     : false,
               }}
-            />
-          </CardHeader>
-
-          <CardContent className="pt-0">
-            <div className="flex flex-col gap-3 sm:flex-row sm:items-start sm:justify-between">
-              <div className="text-xs text-gray-500 dark:text-gray-300">
-                {lastCheckedOn && (
-                  <p>
-                    <span className="font-medium">Last checked:</span>{" "}
-                    {lastCheckedOn}
-                  </p>
-                )}
-              </div>
-              <div className="flex flex-col items-start gap-1 sm:items-end">
-                <div className="flex items-center gap-2">
+              actions={
+                <>
                   {/* The check reaches the authorized channels: the API
                       answers 400 while the set is empty. */}
                   <Button
@@ -661,17 +782,23 @@ export const SlackIntegrationManager = ({
                     <Unplug size={14} />
                     Disconnect
                   </Button>
-                </div>
-                <p
-                  id={CHECK_HINT_ID}
-                  className="max-w-prose text-xs text-gray-500 sm:text-right dark:text-gray-300"
-                >
-                  {checkHint()}
-                </p>
-              </div>
-            </div>
+                </>
+              }
+            />
 
-            <div className="border-border-neutral-secondary mt-6 flex flex-col gap-4 border-t pt-6">
+            {/* Below the row, not beside the buttons: the width is what lets
+                the check say what it did without wrapping into a column. */}
+            <SlackConnectionCheckStatus
+              status={checkStatus()}
+              outcome={checkSummary}
+              captionId={CHECK_HINT_ID}
+            >
+              {checkHint()}
+            </SlackConnectionCheckStatus>
+          </CardHeader>
+
+          <CardContent className="pt-0">
+            <div className="border-border-neutral-secondary flex flex-col gap-4 border-t pt-6">
               <SlackChannelMultiSelect
                 options={channelOptions}
                 values={selectedChannelIds}
@@ -714,19 +841,24 @@ export const SlackIntegrationManager = ({
                   className="text-text-neutral-secondary text-xs"
                   data-authorized-channels
                 >
-                  {authorizedChannels.length > 0
-                    ? `Prowler posts to ${channelList(
-                        authorizedChannels.map((channel) => channel.name),
-                      )}.`
-                    : "No destination channels authorized yet."}
+                  {authorizedChannels.length > 0 ? (
+                    <>
+                      Prowler posts to{" "}
+                      <StyledChannelList
+                        names={authorizedChannels.map(
+                          (channel) => channel.name,
+                        )}
+                      />
+                      .
+                    </>
+                  ) : (
+                    "No destination channels authorized yet."
+                  )}
                 </p>
                 <Button
                   size="sm"
                   disabled={
-                    sameChannelIds(
-                      selectedChannelIds,
-                      authorizedChannels.map((channel) => channel.id),
-                    ) ||
+                    sameChannelIds(selectedChannelIds, authorizedChannelIds) ||
                     isSavingChannels ||
                     isTesting
                   }