fix: make integration cards clickable (#2905)

2026-08-22 15:32:07 +01:00
parent 9ef80367f8
commit f284985d10
9 changed files with 455 additions and 53 deletions
@@ -1,4 +1,4 @@
-import { act, cleanup, fireEvent, render, screen } from "@testing-library/react";
+import { act, cleanup, fireEvent, render, screen, within } from "@testing-library/react";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import type { Component, IntegrationSettingsActionProps } from "@kandev/plugin-sdk";
 import { SettingsSaveProvider } from "@/components/settings/settings-save-provider";
@@ -138,10 +138,17 @@ describe("IntegrationsIndexPage", () => {
    expect(window.localStorage.getItem("kandev:integrations:hideDisabledInNav:v1")).toBeNull();
  });

-  it("navigates when the integration label is clicked", () => {
+  it("exposes one full-card navigation target for a native integration", () => {
    renderPage();

-    fireEvent.click(screen.getByRole("link", { name: "Azure DevOps" }));
+    const card = screen.getByTestId("integration-card-azure-devops");
+    const links = within(card).getAllByRole("link");
+
+    expect(links).toHaveLength(1);
+    expect(links[0].getAttribute("aria-label")).toBe("Azure DevOps");
+    expect(links[0].getAttribute("href")).toBe("/settings/integrations/azure-devops");
+
+    fireEvent.click(links[0]);

    expect(pushNavigationStateSpy).toHaveBeenCalledWith(
      {},
@@ -179,7 +186,12 @@ describe("IntegrationsIndexPage plugin contributions", () => {

    renderPage();

-    const link = screen.getByRole("link", { name: /source control/i });
+    const card = screen.getByTestId(`integration-card-${PLUGIN_ID}-${SOURCE_CONTROL_ID}`);
+    const links = within(card).getAllByRole("link");
+
+    expect(links).toHaveLength(1);
+    const link = links[0];
+    expect(link.getAttribute("aria-label")).toBe("Source Control");
    expect(link.getAttribute("href")).toBe(`/settings/integrations/${SOURCE_CONTROL_ID}`);
    expect(screen.getByText(SOURCE_CONTROL_DESCRIPTION)).not.toBeNull();
  });
@@ -1,6 +1,6 @@
 "use client";

-import type { ComponentType } from "react";
+import type { ComponentType, ReactNode } from "react";
 import Link from "@/components/routing/app-link";
 import {
  IconBrandGithub,
@@ -95,6 +95,47 @@ type IntegrationsIndexPageProps = {
  workspaceId?: string;
 };

+type IntegrationCardProps = {
+  href: string;
+  label: string;
+  description: ReactNode;
+  Icon: ComponentType<{ className?: string }>;
+  control?: ReactNode;
+  testId: string;
+};
+
+function IntegrationCard({
+  href,
+  label,
+  description,
+  Icon,
+  control,
+  testId,
+}: IntegrationCardProps) {
+  return (
+    <Card
+      data-testid={testId}
+      className="relative h-full w-full transition-colors hover:border-primary/40"
+    >
+      <Link
+        href={href}
+        aria-label={label}
+        className="absolute inset-0 rounded-[inherit] cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring"
+      />
+      <CardContent className="space-y-2">
+        <div className="flex items-center justify-between gap-2">
+          <div className="flex min-w-0 items-center gap-2 text-base font-semibold">
+            <Icon className="h-5 w-5 shrink-0" />
+            <span className="truncate">{label}</span>
+          </div>
+          {control ? <div className="relative z-10 shrink-0">{control}</div> : null}
+        </div>
+        <p className="text-sm text-muted-foreground">{description}</p>
+      </CardContent>
+    </Card>
+  );
+}
+
 /** Row for the "Hide disabled integrations from left panel navigation" setting, drafted/saved like the per-integration toggles. */
 function HideDisabledIntegrationsSetting() {
  const { t } = useTranslation();
@@ -153,62 +194,36 @@ export function IntegrationsIndexPage({ workspaceId }: IntegrationsIndexPageProp
          const href = `${rootHref}/${slug}`;
          const EnabledControl = ENABLED_CONTROL_BY_SLUG[slug];
          return (
-            <Card
+            <IntegrationCard
              key={slug}
-              data-testid={`integration-card-${slug}`}
-              className="h-full w-full transition-colors hover:border-primary/40"
-            >
-              <CardContent className="space-y-2">
-                <div className="flex items-center justify-between gap-2">
-                  <Link
-                    href={href}
-                    className="flex min-w-0 items-center gap-2 text-base font-semibold hover:underline cursor-pointer"
-                  >
-                    <Icon className="h-5 w-5 shrink-0" />
-                    <span className="truncate">{label}</span>
-                  </Link>
-                  <div className="shrink-0">
-                    <EnabledControl workspaceId={workspaceId} />
-                  </div>
-                </div>
-                <Link href={href} className="text-sm text-muted-foreground cursor-pointer">
-                  {t(descriptionKey)}
-                </Link>
-              </CardContent>
-            </Card>
+              testId={`integration-card-${slug}`}
+              href={href}
+              label={label}
+              Icon={Icon}
+              description={t(descriptionKey)}
+              control={<EnabledControl workspaceId={workspaceId} />}
+            />
          );
        })}
        {pluginIntegrations.map(({ pluginId, id, label, description, icon, action: Action }) => {
          const href = `${rootHref}/${id}`;
          const Icon = resolvePluginIcon(icon);
          return (
-            <Card
+            <IntegrationCard
              key={`${pluginId}:${id}`}
-              data-testid={`integration-card-${pluginId}-${id}`}
-              className="h-full w-full transition-colors hover:border-primary/40"
-            >
-              <CardContent className="space-y-2">
-                <div className="flex items-center justify-between gap-2">
-                  <Link
-                    href={href}
-                    className="flex min-w-0 items-center gap-2 text-base font-semibold hover:underline cursor-pointer"
-                  >
-                    <Icon className="h-5 w-5 shrink-0" />
-                    <span className="truncate">{label}</span>
-                  </Link>
-                  <div className="shrink-0">
-                    {Action ? (
-                      <PluginErrorBoundary context={`integration card action "${pluginId}:${id}"`}>
-                        <Action workspaceId={workspaceId} surface="index" />
-                      </PluginErrorBoundary>
-                    ) : null}
-                  </div>
-                </div>
-                <Link href={href} className="text-sm text-muted-foreground cursor-pointer">
-                  {description}
-                </Link>
-              </CardContent>
-            </Card>
+              testId={`integration-card-${pluginId}-${id}`}
+              href={href}
+              label={label}
+              Icon={Icon}
+              description={description}
+              control={
+                Action ? (
+                  <PluginErrorBoundary context={`integration card action "${pluginId}:${id}"`}>
+                    <Action workspaceId={workspaceId} surface="index" />
+                  </PluginErrorBoundary>
+                ) : null
+              }
+            />
          );
        })}
      </div>
@@ -0,0 +1,33 @@
+import { expect, test } from "../../fixtures/test-base";
+
+test.describe("integrations index card navigation", () => {
+  test("clicking a card body opens the workspace-scoped integration settings page", async ({
+    testPage,
+    seedData,
+  }) => {
+    const integrationsPath = `/settings/workspaces/${encodeURIComponent(seedData.workspaceId)}/integrations`;
+    await testPage.goto(integrationsPath);
+
+    const card = testPage.getByTestId("integration-card-github");
+    await expect(card).toBeVisible();
+
+    const cardLink = card.getByRole("link", { name: "GitHub" });
+    await cardLink.focus();
+    await expect(cardLink).toBeFocused();
+    await expect(cardLink).toHaveClass(/focus-visible:ring-inset/);
+
+    const cardBox = await card.boundingBox();
+    const descriptionBox = await card.locator("p").boundingBox();
+    expect(cardBox).not.toBeNull();
+    expect(descriptionBox).not.toBeNull();
+    await card.click({
+      position: {
+        x: descriptionBox!.x - cardBox!.x + descriptionBox!.width / 2,
+        y: descriptionBox!.y - cardBox!.y + descriptionBox!.height / 2,
+      },
+    });
+
+    await expect(testPage).toHaveURL(`${integrationsPath}/github`);
+    await expect(testPage.getByTestId("github-integration-heading")).toBeVisible();
+  });
+});
@@ -0,0 +1,28 @@
+import { expect, test } from "../../fixtures/test-base";
+
+test.describe("integrations index card navigation on mobile", () => {
+  test("tapping a card body opens the workspace-scoped integration settings page", async ({
+    testPage,
+    seedData,
+  }) => {
+    const integrationsPath = `/settings/workspaces/${encodeURIComponent(seedData.workspaceId)}/integrations`;
+    await testPage.goto(integrationsPath);
+
+    const card = testPage.getByTestId("integration-card-github");
+    await expect(card).toBeVisible();
+
+    const cardBox = await card.boundingBox();
+    const descriptionBox = await card.locator("p").boundingBox();
+    expect(cardBox).not.toBeNull();
+    expect(descriptionBox).not.toBeNull();
+    await card.tap({
+      position: {
+        x: descriptionBox!.x - cardBox!.x + descriptionBox!.width / 2,
+        y: descriptionBox!.y - cardBox!.y + descriptionBox!.height / 2,
+      },
+    });
+
+    await expect(testPage).toHaveURL(`${integrationsPath}/github`);
+    await expect(testPage.getByTestId("github-integration-heading")).toBeVisible();
+  });
+});
@@ -0,0 +1,108 @@
+---
+spec: docs/specs/integrations/clickable-integration-cards.md
+created: 2026-08-21
+status: complete
+---
+
+# Implementation Plan: Clickable integration cards
+
+## Overview
+
+Update the shared integrations index card composition so each card has one
+accessible, full-card `AppLink` to the route it already exposes. Keep the
+native switch and plugin action above that navigation layer as independent
+interactive controls. Validate the shared component with its unit test and
+prove native navigation on both desktop and mobile Playwright projects.
+
+This is a frontend-only change. It does not add an API, state shape, route, or
+persistence boundary.
+
+## Frontend
+
+### Native and plugin integration cards
+
+Update `apps/web/components/integrations/integrations-index-page.tsx`:
+
+- Keep the existing `rootHref` and per-card `href` derivation for global and
+  workspace-scoped routes.
+- Replace the separate title and description links with one full-card `AppLink`
+  overlay that has the integration label as its accessible name.
+- Keep the visible icon, label, and description as card content while allowing
+  pointer events to reach the overlay for all non-control card areas.
+- Place the native enable control and plugin `action` in an interactive layer
+  above the overlay so toggling or activating them cannot navigate.
+- Preserve existing `data-testid` card identifiers, grid geometry, hover state,
+  plugin error boundary, and translated description content.
+
+### Unit coverage
+
+Update `apps/web/app/settings/integrations/page.test.tsx` to assert that native
+and plugin cards expose the expected accessible link and route, and that the
+native switch still drafts its state without calling navigation.
+
+## Backend
+
+None.
+
+## Tests
+
+- **What:** Native and plugin cards expose the existing global and
+  workspace-scoped routes through a full-card navigation target, while the
+  switch remains non-navigating.
+  **File:** `apps/web/app/settings/integrations/page.test.tsx`
+  **How:** Render the index with the existing navigation spy, inspect the card
+  link `href`, activate it, and separately activate the switch.
+- **What:** A pointer click on the native card body navigates from the desktop
+  integrations index.
+  **File:** `apps/web/e2e/tests/integrations/integrations-index-card-navigation.spec.ts`
+  **How:** Open the index, click a card body away from the switch, and assert the
+  workspace-scoped integration URL and heading.
+- **What:** A touch tap on the native card body navigates from the mobile
+  integrations index.
+  **File:** `apps/web/e2e/tests/integrations/mobile-integrations-index-card-navigation.spec.ts`
+  **How:** Use the configured `mobile-chrome` project, tap a card body, and
+  assert the same workspace-scoped route and integration page content.
+
+## E2E Tests
+
+- **Scenario:** Global or workspace index, body click on a native card, route
+  changes to that integration settings page.
+  **File:** `apps/web/e2e/tests/integrations/integrations-index-card-navigation.spec.ts`
+  **What to verify:** The URL includes the active workspace and integration
+  slug, and the destination heading is visible.
+- **Scenario:** Phone-sized workspace index, body tap on a native card, route
+  changes without a title-only interaction.
+  **File:** `apps/web/e2e/tests/integrations/mobile-integrations-index-card-navigation.spec.ts`
+  **What to verify:** The mobile project reaches the same integration settings
+  route and visible page heading.
+
+## Verification Results
+
+Implementation and verification completed.
+
+- Focused integration index unit suite: 12 passed.
+- Targeted ESLint for changed frontend and E2E files: passed.
+- Web TypeScript check with `NODE_OPTIONS=--max-old-space-size=8192`: passed.
+- Desktop blank-body navigation E2E: 1 passed.
+- Mobile blank-body navigation E2E: 1 passed.
+- Existing desktop enable/disable toggle E2E: 1 passed.
+
+The managed E2E runner rebuilt the backend and production Vite assets for each
+project run and completed its fixture setup and teardown successfully.
+
+## Implementation Waves And Parallel Candidates
+
+Wave 1:
+
+- [x] [Task 01: Card navigation](task-01-card-navigation.md)
+
+Wave 2:
+
+- [x] [Task 02: Navigation E2E coverage](task-02-navigation-e2e.md)
+
+Tasks are sequential because the E2E selectors and route assertions depend on
+the final card markup.
+
+## Open Questions
+
+None.
@@ -0,0 +1,71 @@
+---
+id: "01-card-navigation"
+title: "Integration card navigation"
+status: complete
+wave: 1
+depends_on: []
+plan: "plan.md"
+spec: "../../specs/integrations/clickable-integration-cards.md"
+---
+
+# Task 01: Integration card navigation
+
+## Acceptance
+
+- Native and plugin integration cards expose one accessible full-card link that
+  uses their existing global or workspace-scoped `href`.
+- The native switch and plugin action remain clickable without triggering card
+  navigation.
+- The focused component test covers the link route and switch isolation.
+
+## Verification
+
+From the repository root, after the fresh-worktree install if dependencies are
+missing:
+
+```bash
+cd apps && pnpm install --frozen-lockfile
+cd apps && pnpm --filter @kandev/web test -- app/settings/integrations/page.test.tsx
+```
+
+## Files likely touched
+
+- `apps/web/components/integrations/integrations-index-page.tsx`
+- `apps/web/app/settings/integrations/page.test.tsx`
+
+## Dependencies
+
+None.
+
+## Parallelism
+
+`sequential`.
+
+## Inputs
+
+- `docs/specs/integrations/clickable-integration-cards.md`, What and Scenarios.
+- `docs/plans/clickable-integration-cards/plan.md`, Frontend and Tests.
+- Existing `AppLink` behavior in `apps/web/components/routing/app-link.tsx`.
+
+## Output contract
+
+Report the component and unit-test files changed, the focused test command and
+result, any accessibility or pointer-event risk, and the updated task and plan
+status before handing off to Task 02.
+
+## Results
+
+Updated the shared integration card composition for native and plugin cards.
+Each card now exposes one accessible overlay `AppLink`, while the native
+switch and plugin action remain above that link as independent controls.
+
+Focused verification passed:
+
+```text
+pnpm --filter @kandev/web test -- app/settings/integrations/page.test.tsx
+12 tests passed
+```
+
+Targeted ESLint and the web TypeScript check also passed. The default Node heap
+size was insufficient for the repository-wide typecheck, so it was rerun with
+`NODE_OPTIONS=--max-old-space-size=8192`.
@@ -0,0 +1,75 @@
+---
+id: "02-navigation-e2e"
+title: "Integration card navigation E2E coverage"
+status: complete
+wave: 2
+depends_on: ["01-card-navigation"]
+plan: "plan.md"
+spec: "../../specs/integrations/clickable-integration-cards.md"
+---
+
+# Task 02: Integration card navigation E2E coverage
+
+## Acceptance
+
+- Desktop Playwright coverage clicks a native integration card body and reaches
+  the workspace-scoped integration settings page.
+- Mobile Playwright coverage taps the same card body with the configured
+  `mobile-chrome` project and reaches the same destination.
+- The tests use stable card or route selectors and do not regress the existing
+  switch-isolation coverage.
+
+## Verification
+
+From the repository root, the managed runner must build the production
+frontend before each project run:
+
+```bash
+cd apps && pnpm install --frozen-lockfile
+cd apps/web && pnpm e2e:run --project chromium tests/integrations/integrations-index-card-navigation.spec.ts
+cd apps/web && pnpm e2e:run --project mobile-chrome tests/integrations/mobile-integrations-index-card-navigation.spec.ts
+```
+
+## Files likely touched
+
+- `apps/web/e2e/tests/integrations/integrations-index-card-navigation.spec.ts`
+- `apps/web/e2e/tests/integrations/mobile-integrations-index-card-navigation.spec.ts`
+
+## Dependencies
+
+Task 01.
+
+## Parallelism
+
+`sequential`.
+
+## Inputs
+
+- `docs/specs/integrations/clickable-integration-cards.md`, Scenarios.
+- `docs/plans/clickable-integration-cards/plan.md`, E2E Tests.
+- Existing fixtures from `apps/web/e2e/fixtures/test-base.ts`.
+- Existing layout and toggle tests in `apps/web/e2e/tests/integrations/`.
+
+## Output contract
+
+Report both exact managed-runner commands, discovered test counts, pass/fail
+results, build and teardown evidence, and synchronized task and plan status.
+
+## Results
+
+Added desktop and mobile card-body navigation coverage. Both tests click or tap
+the lower card body away from the title, description, and native switch, then
+assert the workspace-scoped integration URL and destination heading.
+
+Managed verification passed:
+
+```text
+pnpm e2e:run --project chromium tests/integrations/integrations-index-card-navigation.spec.ts
+1 passed
+
+pnpm e2e:run --project mobile-chrome tests/integrations/mobile-integrations-index-card-navigation.spec.ts
+1 passed
+
+pnpm e2e:run --project chromium tests/integrations/integrations-index-enabled-toggle.spec.ts
+1 passed
+```
@@ -159,6 +159,7 @@ Per-workspace credentials and triage triggers for external services.
 | [jira-status-filter](jira-status-filter/spec.md) | shipped |
 | [pr-outcome-attribution](pr-outcome-attribution/spec.md) | shipped |
 | [enable-disable-toggle](integrations/enable-disable-toggle.md) | shipped |
+| [clickable-integration-cards](integrations/clickable-integration-cards.md) | shipped |

 ## workspaces/ — workspace lifecycle

@@ -0,0 +1,59 @@
+---
+status: shipped
+created: 2026-08-21
+owner: Codex
+---
+
+# Clickable integration cards
+
+## Why
+
+The integrations index currently makes only the title and description links
+navigation targets. Users expect the card itself to be the obvious entry point,
+especially on a touch viewport, so requiring a precise tap on text makes the
+settings page harder to use.
+
+## What
+
+- Every native integration card on the global and workspace-scoped integrations
+  index exposes the existing integration route as a full-card navigation target.
+- Every plugin integration card uses the same full-card navigation behavior and
+  preserves its existing workspace-scoped route when one is present.
+- Clicking or tapping any card area that is not an interactive secondary control
+  opens that integration's settings page through the existing in-app navigation.
+- The native enable/disable switch and any plugin card action remain independent
+  controls. Activating either control does not navigate to the integration page.
+- The card navigation target has an accessible integration name and remains
+  usable with keyboard focus and activation.
+- Desktop and mobile retain the same route and control semantics. Mobile uses
+  the existing single-column integrations index and does not add an intermediate
+  drawer or another navigation step.
+
+## Scenarios
+
+- **GIVEN** the global integrations index, **WHEN** the user clicks the body of
+  a native Azure DevOps card outside its switch, **THEN** the app navigates to
+  `/settings/integrations/azure-devops` through the normal SPA route transition.
+- **GIVEN** a workspace-scoped integrations index, **WHEN** the user clicks the
+  body of a native card outside its switch, **THEN** the app navigates to the
+  matching `/settings/workspaces/<encoded-workspace-id>/integrations/<slug>`
+  route.
+- **GIVEN** a plugin integration card, **WHEN** the user clicks its body outside
+  the plugin action, **THEN** the app navigates to that plugin's existing
+  integration settings route.
+- **GIVEN** an integration card with an enabled or disabled switch, **WHEN** the
+  user activates the switch, **THEN** the switch changes or drafts its state and
+  the browser remains on the integrations index.
+- **GIVEN** the integrations index on a phone-sized viewport, **WHEN** the user
+  taps the body of a card outside its switch, **THEN** the app navigates to the
+  same integration settings route without requiring a title or description tap.
+- **GIVEN** a focused card navigation target, **WHEN** the user presses Enter,
+  **THEN** the app opens the same integration settings route as a pointer or
+  touch activation.
+
+## Out of scope
+
+- Changing integration routes, workspace selection, enable/disable persistence,
+  plugin registration, or integration page content.
+- Making the switch or plugin action navigate when its own control is activated.
+- Adding a new mobile navigation surface or changing the integrations grid.