Skip to content

Commit 1cfedf3

Browse files
MODSetterMODSetter
authored
Merge pull request #987 from JoeMakuta/fix/improve-settings-form-semantics
fix: improve semantics and structure of settings forms
2 parents 959999f + f00f782 commit 1cfedf3

2 files changed

Lines changed: 376 additions & 320 deletions

File tree

‎surfsense_web/components/settings/general-settings-manager.tsx‎

Lines changed: 177 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -9,160 +9,190 @@ import { toast } from "sonner";
99
import { updateSearchSpaceMutationAtom } from "@/atoms/search-spaces/search-space-mutation.atoms";
1010
import { Alert, AlertDescription } from "@/components/ui/alert";
1111
import { Button } from "@/components/ui/button";
12-
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card";
12+
import {
13+
Card,
14+
CardContent,
15+
CardDescription,
16+
CardHeader,
17+
CardTitle,
18+
} from "@/components/ui/card";
1319
import { Input } from "@/components/ui/input";
1420
import { Label } from "@/components/ui/label";
1521
import { Skeleton } from "@/components/ui/skeleton";
1622
import { searchSpacesApiService } from "@/lib/apis/search-spaces-api.service";
1723
import { cacheKeys } from "@/lib/query-client/cache-keys";
24+
import { Spinner } from "../ui/spinner";
1825

1926
interface GeneralSettingsManagerProps {
20-
searchSpaceId: number;
27+
searchSpaceId: number;
2128
}
2229

23-
export function GeneralSettingsManager({ searchSpaceId }: GeneralSettingsManagerProps) {
24-
const t = useTranslations("searchSpaceSettings");
25-
const tCommon = useTranslations("common");
26-
const {
27-
data: searchSpace,
28-
isLoading: loading,
29-
refetch: fetchSearchSpace,
30-
} = useQuery({
31-
queryKey: cacheKeys.searchSpaces.detail(searchSpaceId.toString()),
32-
queryFn: () => searchSpacesApiService.getSearchSpace({ id: searchSpaceId }),
33-
enabled: !!searchSpaceId,
34-
});
35-
36-
const { mutateAsync: updateSearchSpace } = useAtomValue(updateSearchSpaceMutationAtom);
37-
38-
const [name, setName] = useState("");
39-
const [description, setDescription] = useState("");
40-
const [saving, setSaving] = useState(false);
41-
const [hasChanges, setHasChanges] = useState(false);
42-
43-
// Initialize state from fetched search space
44-
useEffect(() => {
45-
if (searchSpace) {
46-
setName(searchSpace.name || "");
47-
setDescription(searchSpace.description || "");
48-
setHasChanges(false);
49-
}
50-
}, [searchSpace]);
51-
52-
// Track changes
53-
useEffect(() => {
54-
if (searchSpace) {
55-
const currentName = searchSpace.name || "";
56-
const currentDescription = searchSpace.description || "";
57-
const changed = currentName !== name || currentDescription !== description;
58-
setHasChanges(changed);
59-
}
60-
}, [searchSpace, name, description]);
61-
62-
const handleSave = async () => {
63-
try {
64-
setSaving(true);
65-
66-
await updateSearchSpace({
67-
id: searchSpaceId,
68-
data: {
69-
name: name.trim(),
70-
description: description.trim() || undefined,
71-
},
72-
});
73-
74-
setHasChanges(false);
75-
await fetchSearchSpace();
76-
} catch (error: any) {
77-
console.error("Error saving search space details:", error);
78-
toast.error(error.message || "Failed to save search space details");
79-
} finally {
80-
setSaving(false);
81-
}
82-
};
83-
84-
if (loading) {
85-
return (
86-
<div className="space-y-4 md:space-y-6">
87-
<Card>
88-
<CardHeader className="px-3 md:px-6 pt-3 md:pt-6 pb-2 md:pb-3">
89-
<Skeleton className="h-5 md:h-6 w-36 md:w-48" />
90-
<Skeleton className="h-3 md:h-4 w-full max-w-md mt-2" />
91-
</CardHeader>
92-
<CardContent className="space-y-3 md:space-y-4 px-3 md:px-6 pb-3 md:pb-6">
93-
<Skeleton className="h-10 md:h-12 w-full" />
94-
<Skeleton className="h-10 md:h-12 w-full" />
95-
</CardContent>
96-
</Card>
97-
</div>
98-
);
99-
}
100-
101-
return (
102-
<div className="space-y-4 md:space-y-6">
103-
<Alert className="bg-muted/50 py-3 md:py-4">
104-
<Info className="h-3 w-3 md:h-4 md:w-4 shrink-0" />
105-
<AlertDescription className="text-xs md:text-sm">
106-
Update your search space name and description. These details help identify and organize
107-
your workspace.
108-
</AlertDescription>
109-
</Alert>
110-
111-
{/* Search Space Details Card */}
112-
<Card>
113-
<CardHeader className="px-3 md:px-6 pt-3 md:pt-6 pb-2 md:pb-3">
114-
<CardTitle className="text-base md:text-lg">Search Space Details</CardTitle>
115-
<CardDescription className="text-xs md:text-sm">
116-
Manage the basic information for this search space.
117-
</CardDescription>
118-
</CardHeader>
119-
<CardContent className="space-y-4 md:space-y-5 px-3 md:px-6 pb-3 md:pb-6">
120-
<div className="space-y-1.5 md:space-y-2">
121-
<Label htmlFor="search-space-name" className="text-sm md:text-base font-medium">
122-
{t("general_name_label")}
123-
</Label>
124-
<Input
125-
id="search-space-name"
126-
placeholder={t("general_name_placeholder")}
127-
value={name}
128-
onChange={(e) => setName(e.target.value)}
129-
className="text-sm md:text-base h-9 md:h-10"
130-
/>
131-
<p className="text-[10px] md:text-xs text-muted-foreground">
132-
{t("general_name_description")}
133-
</p>
134-
</div>
135-
136-
<div className="space-y-1.5 md:space-y-2">
137-
<Label htmlFor="search-space-description" className="text-sm md:text-base font-medium">
138-
{t("general_description_label")}{" "}
139-
<span className="text-muted-foreground font-normal">({tCommon("optional")})</span>
140-
</Label>
141-
<Input
142-
id="search-space-description"
143-
placeholder={t("general_description_placeholder")}
144-
value={description}
145-
onChange={(e) => setDescription(e.target.value)}
146-
className="text-sm md:text-base h-9 md:h-10"
147-
/>
148-
<p className="text-[10px] md:text-xs text-muted-foreground">
149-
{t("general_description_description")}
150-
</p>
151-
</div>
152-
</CardContent>
153-
</Card>
154-
155-
{/* Action Buttons */}
156-
<div className="flex justify-end pt-3 md:pt-4">
157-
<Button
158-
variant="outline"
159-
onClick={handleSave}
160-
disabled={!hasChanges || saving || !name.trim()}
161-
className="gap-2 bg-white text-black hover:bg-neutral-100 dark:bg-white dark:text-black dark:hover:bg-neutral-200"
162-
>
163-
{saving ? t("general_saving") : t("general_save")}
164-
</Button>
165-
</div>
166-
</div>
167-
);
30+
export function GeneralSettingsManager({
31+
searchSpaceId,
32+
}: GeneralSettingsManagerProps) {
33+
const t = useTranslations("searchSpaceSettings");
34+
const tCommon = useTranslations("common");
35+
const {
36+
data: searchSpace,
37+
isLoading: loading,
38+
refetch: fetchSearchSpace,
39+
} = useQuery({
40+
queryKey: cacheKeys.searchSpaces.detail(searchSpaceId.toString()),
41+
queryFn: () => searchSpacesApiService.getSearchSpace({ id: searchSpaceId }),
42+
enabled: !!searchSpaceId,
43+
});
44+
45+
const { mutateAsync: updateSearchSpace } = useAtomValue(
46+
updateSearchSpaceMutationAtom,
47+
);
48+
49+
const [name, setName] = useState("");
50+
const [description, setDescription] = useState("");
51+
const [saving, setSaving] = useState(false);
52+
const [hasChanges, setHasChanges] = useState(false);
53+
54+
// Initialize state from fetched search space
55+
useEffect(() => {
56+
if (searchSpace) {
57+
setName(searchSpace.name || "");
58+
setDescription(searchSpace.description || "");
59+
setHasChanges(false);
60+
}
61+
}, [searchSpace]);
62+
63+
// Track changes
64+
useEffect(() => {
65+
if (searchSpace) {
66+
const currentName = searchSpace.name || "";
67+
const currentDescription = searchSpace.description || "";
68+
const changed =
69+
currentName !== name || currentDescription !== description;
70+
setHasChanges(changed);
71+
}
72+
}, [searchSpace, name, description]);
73+
74+
const handleSave = async () => {
75+
try {
76+
setSaving(true);
77+
78+
await updateSearchSpace({
79+
id: searchSpaceId,
80+
data: {
81+
name: name.trim(),
82+
description: description.trim() || undefined,
83+
},
84+
});
85+
86+
setHasChanges(false);
87+
await fetchSearchSpace();
88+
} catch (error: any) {
89+
console.error("Error saving search space details:", error);
90+
toast.error(error.message || "Failed to save search space details");
91+
} finally {
92+
setSaving(false);
93+
}
94+
};
95+
96+
const onSubmit = (e: React.FormEvent) => {
97+
e.preventDefault();
98+
handleSave();
99+
};
100+
101+
if (loading) {
102+
return (
103+
<div className="space-y-4 md:space-y-6">
104+
<Card>
105+
<CardHeader className="px-3 md:px-6 pt-3 md:pt-6 pb-2 md:pb-3">
106+
<Skeleton className="h-5 md:h-6 w-36 md:w-48" />
107+
<Skeleton className="h-3 md:h-4 w-full max-w-md mt-2" />
108+
</CardHeader>
109+
<CardContent className="space-y-3 md:space-y-4 px-3 md:px-6 pb-3 md:pb-6">
110+
<Skeleton className="h-10 md:h-12 w-full" />
111+
<Skeleton className="h-10 md:h-12 w-full" />
112+
</CardContent>
113+
</Card>
114+
</div>
115+
);
116+
}
117+
118+
return (
119+
<div className="space-y-4 md:space-y-6">
120+
<Alert className="bg-muted/50 py-3 md:py-4">
121+
<Info className="h-3 w-3 md:h-4 md:w-4 shrink-0" />
122+
<AlertDescription className="text-xs md:text-sm">
123+
Update your search space name and description. These details help
124+
identify and organize your workspace.
125+
</AlertDescription>
126+
</Alert>
127+
128+
{/* Search Space Details Card */}
129+
<form onSubmit={onSubmit} className="space-y-4 md:space-y-6">
130+
<Card>
131+
<CardHeader className="px-3 md:px-6 pt-3 md:pt-6 pb-2 md:pb-3">
132+
<CardTitle className="text-base md:text-lg">
133+
Search Space Details
134+
</CardTitle>
135+
<CardDescription className="text-xs md:text-sm">
136+
Manage the basic information for this search space.
137+
</CardDescription>
138+
</CardHeader>
139+
<CardContent className="space-y-4 md:space-y-5 px-3 md:px-6 pb-3 md:pb-6">
140+
<div className="space-y-1.5 md:space-y-2">
141+
<Label
142+
htmlFor="search-space-name"
143+
className="text-sm md:text-base font-medium"
144+
>
145+
{t("general_name_label")}
146+
</Label>
147+
<Input
148+
id="search-space-name"
149+
placeholder={t("general_name_placeholder")}
150+
value={name}
151+
onChange={(e) => setName(e.target.value)}
152+
className="text-sm md:text-base h-9 md:h-10"
153+
/>
154+
<p className="text-[10px] md:text-xs text-muted-foreground">
155+
{t("general_name_description")}
156+
</p>
157+
</div>
158+
159+
<div className="space-y-1.5 md:space-y-2">
160+
<Label
161+
htmlFor="search-space-description"
162+
className="text-sm md:text-base font-medium"
163+
>
164+
{t("general_description_label")}{" "}
165+
<span className="text-muted-foreground font-normal">
166+
({tCommon("optional")})
167+
</span>
168+
</Label>
169+
<Input
170+
id="search-space-description"
171+
placeholder={t("general_description_placeholder")}
172+
value={description}
173+
onChange={(e) => setDescription(e.target.value)}
174+
className="text-sm md:text-base h-9 md:h-10"
175+
/>
176+
<p className="text-[10px] md:text-xs text-muted-foreground">
177+
{t("general_description_description")}
178+
</p>
179+
</div>
180+
</CardContent>
181+
</Card>
182+
183+
{/* Action Buttons */}
184+
<div className="flex justify-end pt-3 md:pt-4">
185+
<Button
186+
type="submit"
187+
variant="outline"
188+
disabled={!hasChanges || saving || !name.trim()}
189+
className="gap-2 bg-white text-black hover:bg-neutral-100 dark:bg-white dark:text-black dark:hover:bg-neutral-200"
190+
>
191+
{saving ? <Spinner size="sm" /> : null}
192+
{saving ? t("general_saving") : t("general_save")}
193+
</Button>
194+
</div>
195+
</form>
196+
</div>
197+
);
168198
}

0 commit comments

Comments
 (0)