fix custom widget review findings
This commit is contained in:
1 parent
0482c040ec
commit
afa54755fd
17 files changed
+207
-26
No files matched your search
@@ -145,6 +145,7 @@ export function CustomWidgetForm({ mode, initialValues, definitionId }: CustomWi
|
||||
key={id}
|
||||
component="a"
|
||||
href={`#${id}`}
|
||||
onClick={() => setMobilePane(id === "preview" ? "preview" : "configure")}
|
||||
size="compact-sm"
|
||||
variant="subtle"
|
||||
color={invalidSections.has(id) ? "red" : undefined}
|
||||
|
||||
+8
-1
@@ -22,6 +22,7 @@ vi.mock("@homarr/api/client", () => ({
|
||||
vi.mock("@homarr/translation/client", () => ({ useScopedI18n: () => (key: string) => key }));
|
||||
vi.mock("@homarr/notifications", () => ({ showErrorNotification: vi.fn() }));
|
||||
const rendererAttempt = vi.hoisted(() => vi.fn());
|
||||
const codeEditorProps = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@homarr/widgets/custom-api/custom-jsx-display", () => ({
|
||||
default: ({ data }: { data: Record<string, unknown> }) => {
|
||||
@@ -32,7 +33,12 @@ vi.mock("@homarr/widgets/custom-api/custom-jsx-display", () => ({
|
||||
return <div>widget-renderer</div>;
|
||||
},
|
||||
}));
|
||||
vi.mock("./_code-editor", () => ({ CodeEditor: ({ label }: { label: string }) => <div>{label}</div> }));
|
||||
vi.mock("./_code-editor", () => ({
|
||||
CodeEditor: (props: { label: string; readOnly?: boolean }) => {
|
||||
codeEditorProps(props);
|
||||
return <div>{props.label}</div>;
|
||||
},
|
||||
}));
|
||||
vi.mock("./_custom-widget-preview-action", () => ({ PreviewActionControl: () => <div>action-control</div> }));
|
||||
|
||||
let root: Root;
|
||||
@@ -109,6 +115,7 @@ describe("Custom Widget preview panel", () => {
|
||||
);
|
||||
await act(async () => dataTab?.click());
|
||||
expect(host.textContent).toContain("requestData");
|
||||
expect(codeEditorProps).toHaveBeenCalledWith(expect.objectContaining({ label: "requestData", readOnly: true }));
|
||||
expect(host.textContent).not.toContain("invalid");
|
||||
|
||||
const optionsTab = [...host.querySelectorAll<HTMLElement>('[role="tab"]')].find((element) =>
|
||||
|
||||
@@ -172,6 +172,7 @@ export function CustomWidgetPreviewPanel(props: PreviewPanelProps) {
|
||||
label={t("requestData")}
|
||||
language="json"
|
||||
value={JSON.stringify({ data: props.preview.data, status: props.preview.status }, null, 2)}
|
||||
readOnly
|
||||
onChange={() => undefined}
|
||||
/>
|
||||
</Tabs.Panel>
|
||||
@@ -251,6 +252,7 @@ export function CustomWidgetPreviewPanel(props: PreviewPanelProps) {
|
||||
label={t("journal")}
|
||||
language="json"
|
||||
value={JSON.stringify(journalQuery.data ?? [], null, 2)}
|
||||
readOnly
|
||||
onChange={() => undefined}
|
||||
/>
|
||||
)}
|
||||
|
||||
+3
-1
@@ -87,7 +87,9 @@ export function CustomWidgetSourcesEditor({
|
||||
form.setFieldValue("secrets", [...current, { sourceId, kind, value, hasValue: existing?.hasValue }]);
|
||||
};
|
||||
const addSource = () => {
|
||||
const id = sources.length === 0 ? "default" : `source-${sources.length + 1}`;
|
||||
let suffix = sources.length + 1;
|
||||
while (sources.some(({ id }) => id === `source-${suffix}`)) suffix += 1;
|
||||
const id = sources.length === 0 ? "default" : `source-${suffix}`;
|
||||
form.setFieldValue(
|
||||
"sources",
|
||||
JSON.stringify(
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
// @vitest-environment jsdom
|
||||
|
||||
import { act } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import type { Root } from "react-dom/client";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { JsonPreviewEditor } from "./_json-preview-editor";
|
||||
|
||||
const editorProps = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@homarr/translation/client", () => ({ useScopedI18n: () => (key: string) => key }));
|
||||
vi.mock("./_code-editor", () => ({
|
||||
CodeEditor: (props: { error?: string; onChange(value: string): void }) => {
|
||||
editorProps(props);
|
||||
return <button onClick={() => props.onChange("{invalid")}>edit</button>;
|
||||
},
|
||||
}));
|
||||
|
||||
let root: Root;
|
||||
let host: HTMLDivElement;
|
||||
|
||||
beforeEach(() => {
|
||||
(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
host = document.createElement("div");
|
||||
document.body.append(host);
|
||||
root = createRoot(host);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await act(() => root.unmount());
|
||||
host.remove();
|
||||
});
|
||||
|
||||
describe("preview JSON editor", () => {
|
||||
it("surfaces invalid JSON and clears the previously rendered options", async () => {
|
||||
const onChange = vi.fn();
|
||||
await act(async () => {
|
||||
root.render(<JsonPreviewEditor id="options" label="Options" value={{ stale: true }} onChange={onChange} />);
|
||||
});
|
||||
await act(async () => (host.querySelector("button") as HTMLButtonElement).click());
|
||||
|
||||
expect(onChange).toHaveBeenCalledWith({});
|
||||
expect(editorProps).toHaveBeenLastCalledWith(expect.objectContaining({ error: "invalidJson" }));
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
import { useEffect, useState } from "react";
|
||||
|
||||
import { useScopedI18n } from "@homarr/translation/client";
|
||||
|
||||
import { CodeEditor } from "./_code-editor";
|
||||
import { isRecord, parseJson } from "./_custom-widget-form-utils";
|
||||
|
||||
@@ -16,19 +18,26 @@ export function JsonPreviewEditor({
|
||||
value: Record<string, unknown>;
|
||||
onChange(value: Record<string, unknown>): void;
|
||||
}) {
|
||||
const t = useScopedI18n("customWidget.workbench.builder");
|
||||
const serialized = JSON.stringify(value, null, 2);
|
||||
const [draft, setDraft] = useState(serialized);
|
||||
useEffect(() => setDraft(serialized), [serialized]);
|
||||
const [invalid, setInvalid] = useState(false);
|
||||
useEffect(() => {
|
||||
if (!invalid) setDraft(serialized);
|
||||
}, [invalid, serialized]);
|
||||
return (
|
||||
<CodeEditor
|
||||
id={id}
|
||||
label={label}
|
||||
language="json"
|
||||
value={draft}
|
||||
error={invalid ? t("invalidJson") : undefined}
|
||||
onChange={(next) => {
|
||||
setDraft(next);
|
||||
const parsed = parseJson(next);
|
||||
if (isRecord(parsed)) onChange(parsed);
|
||||
const valid = isRecord(parsed);
|
||||
setInvalid(!valid);
|
||||
onChange(valid ? parsed : {});
|
||||
}}
|
||||
/>
|
||||
);
|
||||
|
||||
@@ -48,4 +48,17 @@ describe("custom widget unsaved changes guard", () => {
|
||||
expect(listLink.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true, button: 0 }))).toBe(true);
|
||||
expect(confirmNavigation).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("restores the current page and confirms browser history navigation", () => {
|
||||
window.history.replaceState(null, "", "/en/manage/custom-widgets");
|
||||
window.history.pushState(null, "", "/en/manage/custom-widgets/edit/widget-1");
|
||||
const confirmNavigation = vi.fn();
|
||||
cleanups.push(registerUnsavedChangesGuard({ isDirty: () => true, confirmNavigation }));
|
||||
|
||||
window.history.replaceState(null, "", "/en/manage/custom-widgets");
|
||||
window.dispatchEvent(new PopStateEvent("popstate"));
|
||||
|
||||
expect(window.location.pathname).toBe("/en/manage/custom-widgets/edit/widget-1");
|
||||
expect(confirmNavigation).toHaveBeenCalledWith("/en/manage/custom-widgets");
|
||||
});
|
||||
});
|
||||
@@ -4,6 +4,7 @@ interface UnsavedChangesGuardOptions {
|
||||
}
|
||||
|
||||
export function registerUnsavedChangesGuard({ isDirty, confirmNavigation }: UnsavedChangesGuardOptions) {
|
||||
let currentHref = `${window.location.pathname}${window.location.search}${window.location.hash}`;
|
||||
const handleClick = (event: MouseEvent) => {
|
||||
if (!isDirty() || event.defaultPrevented || event.button !== 0) return;
|
||||
if (event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) return;
|
||||
@@ -32,11 +33,22 @@ export function registerUnsavedChangesGuard({ isDirty, confirmNavigation }: Unsa
|
||||
event.preventDefault();
|
||||
event.returnValue = true;
|
||||
};
|
||||
const handlePopState = () => {
|
||||
const destination = `${window.location.pathname}${window.location.search}${window.location.hash}`;
|
||||
if (!isDirty()) {
|
||||
currentHref = destination;
|
||||
return;
|
||||
}
|
||||
window.history.pushState(window.history.state, "", currentHref);
|
||||
confirmNavigation(destination);
|
||||
};
|
||||
|
||||
document.addEventListener("click", handleClick, true);
|
||||
window.addEventListener("beforeunload", handleBeforeUnload);
|
||||
window.addEventListener("popstate", handlePopState);
|
||||
return () => {
|
||||
document.removeEventListener("click", handleClick, true);
|
||||
window.removeEventListener("beforeunload", handleBeforeUnload);
|
||||
window.removeEventListener("popstate", handlePopState);
|
||||
};
|
||||
}
|
||||
@@ -295,19 +295,15 @@ export function WorkshopBrowser({ type = "customWidget", onInstall, onUseCss, mo
|
||||
<Stack gap="xs" mt="md">
|
||||
<Group justify="space-between">
|
||||
<Text size="xs" c="dimmed">
|
||||
{t("author", { name: item.authorName || "Community member" })}
|
||||
{t("author", { name: item.authorName || t("communityMember") })}
|
||||
</Text>
|
||||
<Text size="sm" c="dimmed">
|
||||
<IconThumbUp size={13} /> {item.upvotes} · <IconThumbDown size={13} /> {item.downvotes}
|
||||
</Text>
|
||||
</Group>
|
||||
<Group gap="xs">
|
||||
{item.outdated && <Badge color="yellow">Outdated</Badge>}
|
||||
{item.reportCount > 0 && (
|
||||
<Badge color="red">
|
||||
{item.reportCount} report{item.reportCount === 1 ? "" : "s"}
|
||||
</Badge>
|
||||
)}
|
||||
{item.outdated && <Badge color="yellow">{t("outdated")}</Badge>}
|
||||
{item.reportCount > 0 && <Badge color="red">{t("reportCount", { count: item.reportCount })}</Badge>}
|
||||
</Group>
|
||||
<Text fw={700} lineClamp={1}>
|
||||
{item.title}
|
||||
@@ -349,7 +345,7 @@ export function WorkshopBrowser({ type = "customWidget", onInstall, onUseCss, mo
|
||||
<Box>
|
||||
<Title order={3}>{detail.data.title}</Title>
|
||||
<Text size="sm" c="dimmed">
|
||||
{t("author", { name: detail.data.authorName || "Community member" })}
|
||||
{t("author", { name: detail.data.authorName || t("communityMember") })}
|
||||
</Text>
|
||||
</Box>
|
||||
<Button
|
||||
@@ -366,20 +362,18 @@ export function WorkshopBrowser({ type = "customWidget", onInstall, onUseCss, mo
|
||||
{detail.data.description && <Text>{detail.data.description}</Text>}
|
||||
{detail.data.outdated && (
|
||||
<Alert color="yellow" icon={<IconAlertTriangle size={18} />}>
|
||||
The author marked this submission as outdated. Review it carefully before importing.
|
||||
{t("outdatedWarning")}
|
||||
</Alert>
|
||||
)}
|
||||
{detail.data.reportCount > 0 && (
|
||||
<Stack gap="xs">
|
||||
<Alert color="red" icon={<IconAlertTriangle size={18} />}>
|
||||
This submission has {detail.data.reportCount} open community report
|
||||
{detail.data.reportCount === 1 ? "" : "s"}. It remains installable, but you should review its content
|
||||
first.
|
||||
{t("reportWarning", { count: detail.data.reportCount })}
|
||||
</Alert>
|
||||
{reportSummaries.data?.map((summary) => (
|
||||
<Card key={summary.id} withBorder p="sm">
|
||||
<Badge color="red" variant="light" mb="xs">
|
||||
{summary.category}
|
||||
{t(`reportCategory.${summary.category}`)}
|
||||
</Badge>
|
||||
<Text size="sm" style={{ whiteSpace: "pre-wrap" }}>
|
||||
{summary.explanation}
|
||||
@@ -388,7 +382,7 @@ export function WorkshopBrowser({ type = "customWidget", onInstall, onUseCss, mo
|
||||
))}
|
||||
{reportSummaries.data?.length === 0 && (
|
||||
<Text size="xs" c="dimmed">
|
||||
Report details are visible only to this submission's author and Workshop admins.
|
||||
{t("reportVisibility")}
|
||||
</Text>
|
||||
)}
|
||||
</Stack>
|
||||
@@ -400,7 +394,7 @@ export function WorkshopBrowser({ type = "customWidget", onInstall, onUseCss, mo
|
||||
key={file}
|
||||
src={client.fileUrl(detail.data.id, file, "960x640")}
|
||||
radius="md"
|
||||
alt={`${detail.data.title} screenshot ${index + 1}`}
|
||||
alt={t("screenshotAlt", { title: detail.data.title, count: index + 1 })}
|
||||
/>
|
||||
))}
|
||||
</SimpleGrid>
|
||||
@@ -433,6 +427,7 @@ export function WorkshopBrowser({ type = "customWidget", onInstall, onUseCss, mo
|
||||
leftSection={<IconThumbUp size={16} />}
|
||||
loading={vote.isPending && vote.variables?.value === 1}
|
||||
disabled={!user || vote.isPending}
|
||||
aria-label={t("upvote", { count: detail.data.upvotes })}
|
||||
onClick={() => vote.mutate({ submission: detail.data.id, value: 1 })}
|
||||
>
|
||||
{detail.data.upvotes}
|
||||
@@ -442,6 +437,7 @@ export function WorkshopBrowser({ type = "customWidget", onInstall, onUseCss, mo
|
||||
leftSection={<IconThumbDown size={16} />}
|
||||
loading={vote.isPending && vote.variables?.value === -1}
|
||||
disabled={!user || vote.isPending}
|
||||
aria-label={t("downvote", { count: detail.data.downvotes })}
|
||||
onClick={() => vote.mutate({ submission: detail.data.id, value: -1 })}
|
||||
>
|
||||
{detail.data.downvotes}
|
||||
@@ -508,7 +504,10 @@ export function WorkshopBrowser({ type = "customWidget", onInstall, onUseCss, mo
|
||||
label={t("reportReason")}
|
||||
value={reportCategory}
|
||||
onChange={(value) => setReportCategory((value as WorkshopReport["category"]) ?? "other")}
|
||||
data={["outdated", "malicious", "spam", "copyright", "inappropriate", "other"]}
|
||||
data={(["outdated", "malicious", "spam", "copyright", "inappropriate", "other"] as const).map((value) => ({
|
||||
value,
|
||||
label: t(`reportCategory.${value}`),
|
||||
}))}
|
||||
allowDeselect={false}
|
||||
/>
|
||||
<Textarea
|
||||
|
||||
@@ -6,6 +6,7 @@ import { Alert, Button, Checkbox, FileInput, Group, Modal, Stack, Textarea, Text
|
||||
import { clientApi } from "@homarr/api/client";
|
||||
import { showErrorNotification } from "@homarr/notifications";
|
||||
import { useWorkshopCreateMutation } from "@homarr/workshop/backend";
|
||||
import { workshopScreenshotsSchema } from "@homarr/workshop/schema";
|
||||
import type { WorkshopUser } from "@homarr/workshop/schema";
|
||||
import { useScopedI18n } from "@homarr/translation/client";
|
||||
|
||||
@@ -59,6 +60,9 @@ export function WorkshopPublishModal({
|
||||
setError(null);
|
||||
try {
|
||||
if (!definition.data) throw new Error(t("publish.error"));
|
||||
if (!workshopScreenshotsSchema.safeParse(screenshots).success) {
|
||||
throw new Error(t("publish.invalidScreenshot"));
|
||||
}
|
||||
const inspectedDefinition = definition.data;
|
||||
const result = await publishWorkshopDefinition({
|
||||
inspectedDefinition,
|
||||
|
||||
Reference in new issue
Block a user