Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions apps/app/src/components/plugin/PluginsOverview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -552,8 +552,12 @@ describe("PluginsOverview", () => {
expect(officialPills[0]?.parentElement?.className).toContain("px-2");
expect(officialPills[0]?.parentElement?.className).toContain("py-1");

fireEvent.pointerDown(screen.getByRole("button", { name: "Sort" }));
fireEvent.click(screen.getByRole("menuitem", { name: "Plugin name" }));
const sortTrigger = screen.getByRole("button", {
name: "Sort: Plugin name, ascending",
});
expect(sortTrigger.querySelector('[data-icon="ArrowUpDown"]')).toBeTruthy();
fireEvent.pointerDown(sortTrigger);
fireEvent.click(screen.getByRole("menuitemradio", { name: "Plugin name" }));
expect(
[...document.querySelectorAll('[data-testid^="plugin-row-"]')].map(
(row) => row.getAttribute("data-testid"),
Expand All @@ -566,9 +570,12 @@ describe("PluginsOverview", () => {
"plugin-row-inactive-local",
]);

fireEvent.keyDown(screen.getByRole("menu", { name: "Sort" }), {
key: "Escape",
});
fireEvent.keyDown(
screen.getByRole("menu", {
name: "Sort: Plugin name, descending",
}),
{ key: "Escape" },
);
fireEvent.click(screen.getByRole("tab", { name: "Browse" }));
await screen.findByText("GitHub");
fireEvent.click(screen.getByRole("tab", { name: "Installed, 5 plugins" }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,12 @@ describe("BrowsePluginsTab", () => {
"Open Zulu details",
]);

fireEvent.pointerDown(screen.getByRole("button", { name: "Sort" }));
fireEvent.click(screen.getByRole("menuitem", { name: "Plugin name" }));
const sortTrigger = screen.getByRole("button", {
name: "Sort: Plugin name, ascending",
});
expect(sortTrigger.querySelector('[data-icon="ArrowUpDown"]')).toBeTruthy();
fireEvent.pointerDown(sortTrigger);
fireEvent.click(screen.getByRole("menuitemradio", { name: "Plugin name" }));
expect(cardOrder()).toEqual([
"Open Zulu details",
"Open Middle details",
Expand Down Expand Up @@ -269,6 +273,7 @@ describe("BrowsePluginsTab", () => {

const install = screen.getByRole("button", { name: "Install Memory" });
expect(install.className).toContain("w-7");
expect(install.querySelector('[data-icon="Download"]')).not.toBeNull();
fireEvent.pointerMove(install);
expect((await screen.findByRole("tooltip")).textContent).toBe(
"Install Memory",
Expand Down Expand Up @@ -357,8 +362,28 @@ describe("BrowsePluginsTab", () => {
expect((await screen.findByRole("tooltip")).textContent).toBe(
"Uninstall Memory",
);
expect(document.querySelector('[data-icon="Check"]')).not.toBeNull();
expect(installed.querySelector('[data-icon="Download"]')).not.toBeNull();
expect(installed.querySelector('[data-icon="Check"]')).toBeNull();
expect(installed.className).toContain("border-success/40");
expect(installed.className).toContain("bg-success/15");
expect(installed.className).toContain(
"text-[color:color-mix(in_oklab,var(--success)_72%,var(--ink))]",
);
expect(installed.className).not.toContain("text-success-foreground");
expect(installed.className).toContain(
"hover:text-[color:color-mix(in_oklab,var(--success)_72%,var(--ink))]",
);
expect(installed.className).toContain(
"focus-visible:text-[color:color-mix(in_oklab,var(--success)_72%,var(--ink))]",
);
expect(installed.className).not.toContain("hover:text-foreground");
expect(installed.className).toContain("hover:bg-success/25");
expect(screen.queryByRole("button", { name: "Install" })).toBeNull();
fireEvent.click(installed);
expect(
screen.getByRole("heading", { name: "Uninstall Memory?" }),
).toBeTruthy();
fireEvent.click(screen.getByRole("button", { name: "Cancel" }));

fireEvent.click(
screen.getByRole("button", { name: "Open Memory details" }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
ResourceBrowseGrid,
ResourceCollectionViewport,
ResourceInstallControl,
ResourceInstalledControl,
ResourceListState,
ResourceSortMenu,
ResourceToolbar,
Expand Down Expand Up @@ -193,11 +192,12 @@ function BrowseCard({
) : undefined;
const headerAction =
installedPluginId !== null ? (
<ResourceInstalledControl
<ResourceInstallControl

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 slopcop/review — The installed state now depends on color alone.

Both branches show the same Download icon. Only the green color identifies the installed state. This design also presents an uninstall action as a download action.

Use a distinct installed icon. Preserve a destructive hover or focus signal before the confirmation dialog.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 slopcop/review — The button shows a Download icon, but it opens an uninstall confirmation.

This visual signal conflicts with the accessible label, tooltip, and action. The shared ResourceInstalledControl already provides the installed Check state and the Trash uninstall state on hover or focus. Please reuse that control or show an uninstall icon.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 slopcop/review — The Download icon now starts an Uninstall action.

The browser test confirmed that each installed card shows Download, but the button opens the uninstall dialog.

ResourceInstalledControl already shows Check and reveals Trash2 with destructive hover and focus styles. Use that control or an explicit uninstall state.

accessibleLabel={`Uninstall ${entry.displayName}`}
pending={uninstall.isPending}
presentation="icon"
tooltip={`Uninstall ${entry.displayName}`}
className="border-success/40 bg-success/15 text-[color:color-mix(in_oklab,var(--success)_72%,var(--ink))] hover:border-success/55 hover:bg-success/25 hover:text-[color:color-mix(in_oklab,var(--success)_72%,var(--ink))] focus-visible:border-success/55 focus-visible:bg-success/25 focus-visible:text-[color:color-mix(in_oklab,var(--success)_72%,var(--ink))]"
onAction={() => setConfirmingUninstall(true)}
/>
) : (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,29 +1,90 @@
// @vitest-environment jsdom

import { act, cleanup, render } from "@testing-library/react";
import { createElement } from "react";
import { afterEach, vi } from "vitest";
import { describe, expect, it } from "vitest";
import {
getTabStripChevronEdgeClass,
getTabStripChevronVisibilityClass,
SecondaryPanelTabStrip,
SECONDARY_PANEL_TAB_STRIP_FADE_TONE,
} from "./SecondaryPanelTabStrip";

afterEach(() => {
cleanup();
vi.unstubAllGlobals();
});

describe("secondary panel tab-strip edge fades", () => {
it("uses one opaque themed edge fade and no second caret gradient", () => {
it("uses the themed edge fade without overlay scroll controls", () => {
expect(SECONDARY_PANEL_TAB_STRIP_FADE_TONE).toBe("sidebar");
expect(getTabStripChevronEdgeClass("left")).toBe(
"left-0 justify-start",
);
expect(getTabStripChevronEdgeClass("right")).toBe(
"right-0 justify-end",
);
});

it("keeps an available scroll control visible without requiring hover", () => {
const visibleClass = getTabStripChevronVisibilityClass(true);
it("observes the intrinsic tab row so async title changes refresh overflow", () => {
const observed: Element[] = [];
let resizeCallback: ResizeObserverCallback | undefined;
vi.stubGlobal(
"ResizeObserver",
class {
constructor(callback: ResizeObserverCallback) {
resizeCallback = callback;
}
observe(element: Element) {
observed.push(element);
}
disconnect() {}
},
);

const { container } = render(
createElement(SecondaryPanelTabStrip, {
fileTabs: [
{
id: "browser",
filename: "Browser",
isActive: true,
isPinned: false,
leadingVisual: null,
statusLabel: null,
onSelect: vi.fn(),
onClose: vi.fn(),
},
],
onReorderTab: vi.fn(),
usesDesktopChrome: false,
}),
);

expect(visibleClass).toContain("pointer-events-auto");
expect(visibleClass).toContain("opacity-100");
expect(visibleClass).not.toContain("hover:");
expect(getTabStripChevronVisibilityClass(false)).toBe(
"pointer-events-none opacity-0",
const viewport = container.querySelector(".no-scrollbar");
const content = container.querySelector(
"[data-secondary-panel-tab-content]",
);
expect(content).not.toBeNull();
expect(observed).toContain(viewport);
expect(observed).toContain(content);
expect(resizeCallback).toBeDefined();
expect(container.querySelectorAll("[data-overflow-fade]")).toHaveLength(2);
expect(
container
.querySelector("[data-overflow-fade='left']")
?.classList.contains("w-6"),
).toBe(true);
expect(
container.querySelector('[aria-label="Scroll tabs left"]'),
).toBeNull();
expect(
container.querySelector('[aria-label="Scroll tabs right"]'),
).toBeNull();

const rightFade = container.querySelector("[data-overflow-fade='right']");
expect(rightFade?.classList.contains("opacity-0")).toBe(true);
Object.defineProperties(viewport!, {
clientWidth: { configurable: true, value: 120 },
scrollWidth: { configurable: true, value: 240 },
scrollLeft: { configurable: true, value: 0, writable: true },
});
act(() => {
resizeCallback?.([], {} as ResizeObserver);
});
expect(rightFade?.classList.contains("opacity-100")).toBe(true);
});
});
Loading
Loading