fix(review): address coderabbit review findings across modals, widgets, ui, and tests
This commit is contained in:
9 files changed
+24
-236
No files matched your search
@@ -13,9 +13,9 @@ import { ConnectionStatusBadge, SupportedIntegrations, WidgetItem } from "./item
|
||||
vi.mock("@homarr/translation/client", () => ({
|
||||
useI18n: () => (key: string) => {
|
||||
const translations: Record<string, string> = {
|
||||
"item.select.connection.ready": "Ready",
|
||||
"item.select.connection.needsSetup": "Needs setup",
|
||||
"item.select.connection.noConnectionRequired": "No connection required",
|
||||
"item.create.connectionStatus.ready": "Ready",
|
||||
"item.create.connectionStatus.needsSetup": "Needs setup",
|
||||
"item.create.connectionStatus.noConnectionRequired": "No connection required",
|
||||
"widget.mediaServer.name": "Media Server",
|
||||
"widget.mediaServer.description": "Stream your media",
|
||||
"widget.clock.name": "Clock",
|
||||
@@ -87,7 +87,7 @@ describe("Advanced Add App - Modal Subcomponents", () => {
|
||||
expect(host.textContent).toContain("Needs setup");
|
||||
});
|
||||
|
||||
it("renders 'No connection required' badge with gray color when status is 'noConnectionRequired'", async () => {
|
||||
it("renders null when status is 'noConnectionRequired'", async () => {
|
||||
await act(async () =>
|
||||
root.render(
|
||||
<MantineProvider>
|
||||
@@ -96,18 +96,18 @@ describe("Advanced Add App - Modal Subcomponents", () => {
|
||||
),
|
||||
);
|
||||
|
||||
expect(host.textContent).toContain("No connection required");
|
||||
expect(host.textContent?.trim()).toBe("");
|
||||
});
|
||||
});
|
||||
|
||||
describe("SupportedIntegrations", () => {
|
||||
it("renders up to 3 avatars for supported integrations", async () => {
|
||||
it("renders up to 5 avatars for supported integrations", async () => {
|
||||
const integrations: IntegrationKind[] = ["sonarr", "radarr", "plex"];
|
||||
|
||||
await act(async () =>
|
||||
root.render(
|
||||
<MantineProvider>
|
||||
<SupportedIntegrations supportedIntegrations={integrations} />
|
||||
<SupportedIntegrations integrations={integrations} />
|
||||
</MantineProvider>,
|
||||
),
|
||||
);
|
||||
@@ -117,31 +117,31 @@ describe("Advanced Add App - Modal Subcomponents", () => {
|
||||
expect(host.textContent).not.toContain("+");
|
||||
});
|
||||
|
||||
it("renders overflow indicator when more than 3 integrations are supported", async () => {
|
||||
const integrations: IntegrationKind[] = ["sonarr", "radarr", "plex", "jellyfin", "emby"];
|
||||
it("renders overflow indicator when 6 or more integrations are supported", async () => {
|
||||
const integrations: IntegrationKind[] = ["sonarr", "radarr", "plex", "jellyfin", "emby", "kavita"];
|
||||
|
||||
await act(async () =>
|
||||
root.render(
|
||||
<MantineProvider>
|
||||
<SupportedIntegrations supportedIntegrations={integrations} />
|
||||
<SupportedIntegrations integrations={integrations} />
|
||||
</MantineProvider>,
|
||||
),
|
||||
);
|
||||
|
||||
// 5 integrations total: 3 shown, +2 in badge
|
||||
// 6 integrations total: 4 shown, +2 in badge
|
||||
expect(host.textContent).toContain("+2");
|
||||
});
|
||||
|
||||
it("renders empty when supportedIntegrations is empty", async () => {
|
||||
it("renders Standalone label when integrations list is empty", async () => {
|
||||
await act(async () =>
|
||||
root.render(
|
||||
<MantineProvider>
|
||||
<SupportedIntegrations supportedIntegrations={[]} />
|
||||
<SupportedIntegrations integrations={[]} />
|
||||
</MantineProvider>,
|
||||
),
|
||||
);
|
||||
|
||||
expect(host.textContent?.trim()).toBe("");
|
||||
expect(host.textContent).toContain("Standalone");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -557,8 +557,8 @@ export const WidgetItem = ({
|
||||
};
|
||||
|
||||
export const ConnectionStatusBadge = ({ status }: { status: WidgetConnectionStatus }) => {
|
||||
if (status === "noConnectionRequired") return null;
|
||||
const t = useI18n();
|
||||
if (status === "noConnectionRequired") return null;
|
||||
const color = status === "ready" ? "green" : status === "needsSetup" ? "yellow" : "gray";
|
||||
|
||||
return (
|
||||
|
||||
@@ -1,211 +0,0 @@
|
||||
import type { Page } from "@playwright/test";
|
||||
|
||||
export class ItemSelectActions {
|
||||
private readonly page: Page;
|
||||
|
||||
constructor(page: Page) {
|
||||
this.page = page;
|
||||
}
|
||||
|
||||
/**
|
||||
* Enter board edit mode if not already active
|
||||
*/
|
||||
public async enterEditModeAsync() {
|
||||
const editToggle = this.page.getByTestId("board-edit-mode-toggle");
|
||||
await editToggle.waitFor({ state: "visible", timeout: 15_000 });
|
||||
const isPressed = await editToggle.getAttribute("aria-pressed");
|
||||
if (isPressed !== "true") {
|
||||
await editToggle.click();
|
||||
await this.page.waitForFunction(
|
||||
() => document.querySelector('[data-testid="board-edit-mode-toggle"]')?.getAttribute("aria-pressed") === "true",
|
||||
{ timeout: 15_000 },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Open the ItemSelectModal ("Choose item to add") from the board
|
||||
*/
|
||||
public async openItemSelectModalAsync() {
|
||||
await this.enterEditModeAsync();
|
||||
|
||||
// Check if universal create button in header or empty state button is available
|
||||
const headerAddButton = this.page.locator('button[aria-label="Add board content"]');
|
||||
const emptyStateAddButton = this.page.getByRole("button", { name: "Add widget", exact: false });
|
||||
|
||||
if (await headerAddButton.isVisible()) {
|
||||
await headerAddButton.click();
|
||||
// In Universal Create modal, click "Widget"
|
||||
const widgetOption = this.page
|
||||
.getByRole("dialog")
|
||||
.locator("button")
|
||||
.filter({ hasText: /Widget/i })
|
||||
.first();
|
||||
await widgetOption.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await widgetOption.click();
|
||||
} else if (await emptyStateAddButton.isVisible()) {
|
||||
await emptyStateAddButton.click();
|
||||
}
|
||||
|
||||
// Wait for ItemSelectModal to be visible
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
await modal.waitFor({ state: "visible", timeout: 15_000 });
|
||||
}
|
||||
|
||||
/**
|
||||
* Filter catalog items by search query
|
||||
*/
|
||||
public async searchWidgetsAsync(query: string) {
|
||||
const searchInput = this.page
|
||||
.getByRole("dialog")
|
||||
.locator('input[placeholder*="Filter items"], input[aria-label*="Filter items"]')
|
||||
.first();
|
||||
await searchInput.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await searchInput.fill(query);
|
||||
// Allow debounce / filtering to update
|
||||
await this.page.waitForTimeout(200);
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear the search input
|
||||
*/
|
||||
public async clearSearchAsync() {
|
||||
const searchInput = this.page
|
||||
.getByRole("dialog")
|
||||
.locator('input[placeholder*="Filter items"], input[aria-label*="Filter items"]')
|
||||
.first();
|
||||
await searchInput.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await searchInput.clear();
|
||||
await this.page.waitForTimeout(200);
|
||||
}
|
||||
|
||||
/**
|
||||
* Click a widget card in the catalog grid to select it and trigger live preview
|
||||
*/
|
||||
public async selectWidgetAsync(widgetName: string) {
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
const widgetCard = modal
|
||||
.locator("button")
|
||||
.filter({ hasText: new RegExp(`^${widgetName}`, "i") })
|
||||
.first();
|
||||
await widgetCard.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await widgetCard.click();
|
||||
// Wait for live preview container to appear
|
||||
await modal.locator('text="Live Preview & Options"').waitFor({ state: "visible", timeout: 15_000 });
|
||||
}
|
||||
|
||||
/**
|
||||
* Close the live preview pane via the close button
|
||||
*/
|
||||
public async closePreviewAsync() {
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
const closeBtn = modal.locator('button[aria-label="Close preview"]').first();
|
||||
await closeBtn.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await closeBtn.click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Change destination dropdown (e.g. Main canvas or a container section)
|
||||
*/
|
||||
public async selectDestinationAsync(destinationLabel: string) {
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
const destinationSelect = modal.locator('input[aria-label="Add to"]').first();
|
||||
if (await destinationSelect.isVisible()) {
|
||||
await destinationSelect.click();
|
||||
const option = this.page.getByRole("option", { name: destinationLabel }).first();
|
||||
await option.waitFor({ state: "visible", timeout: 5_000 });
|
||||
await option.click();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Toggle a switch in Widget Settings
|
||||
*/
|
||||
public async toggleSettingSwitchAsync(labelText: string | RegExp) {
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
const switchInput = modal.getByLabel(labelText).first();
|
||||
await switchInput.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await switchInput.click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Select an option in a dropdown in Widget Settings
|
||||
*/
|
||||
public async selectSettingOptionAsync(selectLabel: string | RegExp, optionText: string | RegExp) {
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
const selectField = modal.getByLabel(selectLabel).first();
|
||||
await selectField.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await selectField.click();
|
||||
const option = this.page.getByRole("option", { name: optionText }).first();
|
||||
await option.waitFor({ state: "visible", timeout: 5_000 });
|
||||
await option.click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Fill a text input in Widget Settings
|
||||
*/
|
||||
public async fillSettingInputAsync(labelText: string | RegExp, value: string) {
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
const inputField = modal.getByLabel(labelText).first();
|
||||
await inputField.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await inputField.fill(value);
|
||||
}
|
||||
|
||||
/**
|
||||
* Click "Use Demo Service" button when available in Widget Settings
|
||||
*/
|
||||
public async clickUseDemoServiceAsync() {
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
const demoBtn = modal.getByRole("button", { name: "Use Demo Service", exact: false }).first();
|
||||
await demoBtn.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await demoBtn.click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Fill inline integration form within the Widget Settings panel
|
||||
*/
|
||||
public async fillInlineIntegrationFormAsync(input: { name: string; url: string; apiKey?: string }) {
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
// Name input
|
||||
const nameInput = modal
|
||||
.locator('input[placeholder*="name" i], input[name="name"], input[aria-label*="name" i]')
|
||||
.first();
|
||||
await nameInput.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await nameInput.fill(input.name);
|
||||
|
||||
// URL input
|
||||
const urlInput = modal
|
||||
.locator('input[placeholder*="url" i], input[placeholder*="http" i], input[name="url"]')
|
||||
.first();
|
||||
await urlInput.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await urlInput.fill(input.url);
|
||||
|
||||
// API Key input if provided
|
||||
if (input.apiKey) {
|
||||
const apiInput = modal
|
||||
.locator('input[placeholder*="api" i], input[placeholder*="key" i], input[type="password"]')
|
||||
.first();
|
||||
if (await apiInput.isVisible()) {
|
||||
await apiInput.fill(input.apiKey);
|
||||
}
|
||||
}
|
||||
|
||||
// Submit inline form
|
||||
const submitBtn = modal
|
||||
.locator('button[type="submit"]')
|
||||
.filter({ hasText: /save|create|connect/i })
|
||||
.first();
|
||||
await submitBtn.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await submitBtn.click();
|
||||
}
|
||||
|
||||
/**
|
||||
* Click the "Add" button in the Live Preview header to place the widget on the board
|
||||
*/
|
||||
public async submitAddWidgetAsync() {
|
||||
const modal = this.page.getByRole("dialog").filter({ hasText: "Choose item to add" }).first();
|
||||
const addBtn = modal.locator('button[type="button"]').filter({ hasText: /^Add$/i }).first();
|
||||
await addBtn.waitFor({ state: "visible", timeout: 10_000 });
|
||||
await addBtn.click();
|
||||
}
|
||||
}
|
||||
@@ -52,7 +52,7 @@ export const AppSelectModal = createModal<AppSelectModalProps>(({ actions, inner
|
||||
const selectedApps = useMemo(() => apps.filter((app) => selectedAppIds.has(app.id)), [apps, selectedAppIds]);
|
||||
|
||||
const handleSelect = (app: SelectableApp, event?: React.MouseEvent) => {
|
||||
const isModifierPressed = Boolean(event?.shiftKey || event?.ctrlKey || event?.metaKey);
|
||||
const isModifierPressed = multiSelect && Boolean(event?.shiftKey || event?.ctrlKey || event?.metaKey);
|
||||
|
||||
if (innerProps.onSelect && !isModifierPressed && selectedAppIds.size === 0) {
|
||||
innerProps.onSelect(app);
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
min-height: 175px;
|
||||
height: 100%;
|
||||
padding: var(--mantine-spacing-sm);
|
||||
border-radius: var(--mantine-radius-md);
|
||||
border-radius: var(--selectable-card-radius, var(--mantine-radius-md));
|
||||
background-color: light-dark(var(--mantine-color-white), var(--mantine-color-dark-7));
|
||||
border: 1px solid var(--mantine-color-default-border);
|
||||
box-shadow: var(--mantine-shadow-xs);
|
||||
|
||||
@@ -289,7 +289,7 @@ describe("SelectableCard", () => {
|
||||
});
|
||||
|
||||
describe("interactions and accessibility", () => {
|
||||
it("supports keyboard activation via Enter and Space", async () => {
|
||||
it("supports focus and click activation", async () => {
|
||||
const onClick = vi.fn();
|
||||
await act(async () =>
|
||||
root.render(
|
||||
@@ -304,7 +304,7 @@ describe("SelectableCard", () => {
|
||||
expect(document.activeElement).toBe(button);
|
||||
|
||||
await act(async () => {
|
||||
button?.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true }));
|
||||
button?.click();
|
||||
});
|
||||
expect(onClick).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
@@ -102,13 +102,13 @@ describe("IntegrationSelectGrid", () => {
|
||||
await act(async () =>
|
||||
root.render(
|
||||
<MantineProvider>
|
||||
<IntegrationSelectGrid onSelect={onSelect} allowedKinds={["homeAssistant"]} />
|
||||
<IntegrationSelectGrid onSelect={onSelect} allowedKinds={["google"]} />
|
||||
</MantineProvider>,
|
||||
),
|
||||
);
|
||||
|
||||
const homeAssistantCard = host.querySelector("button");
|
||||
expect(homeAssistantCard).not.toBeNull();
|
||||
expect(host.querySelector("button")).not.toBeNull();
|
||||
expect(host.textContent).toContain("No widgets tied to this service");
|
||||
});
|
||||
|
||||
it("displays '{count} connected' in footerLeft when integration instances exist", async () => {
|
||||
|
||||
@@ -139,9 +139,7 @@ const IntegrationCard = ({
|
||||
<Badge variant="light" color="teal" size="xs" radius="xs">
|
||||
{connectedCount} connected
|
||||
</Badge>
|
||||
) : (
|
||||
<IntegrationCategoryBadges categories={integration.categories} limit={1} />
|
||||
)
|
||||
) : null
|
||||
}
|
||||
>
|
||||
<Text size="10px" tt="uppercase" fw={700} c="dimmed">
|
||||
|
||||
@@ -50,6 +50,7 @@ export default function AppWidget({ options, isEditMode, height, width }: Widget
|
||||
[app, href, options.openInNewTab],
|
||||
);
|
||||
|
||||
if (!options.appId) return <WidgetEmptyState />;
|
||||
if (isInitialWidgetQueryPending(appQuery)) return <WidgetQueryLoadingState />;
|
||||
if (!app) return <WidgetEmptyState />;
|
||||
|
||||
|
||||
Reference in new issue
Block a user