Skip to content

Commit 4915d26

Browse files
kubrickcodekubrickcode
committed
fix: introduce FormField component and enhance nested group validation
Problem: - Inconsistent error handling patterns across form input fields - Mixed approaches: aria-invalid vs className for error styling - Missing validation for nested groups (empty name, command, group allowed) - No validation on save in GroupEditDialog - Accessibility (aria-describedby) missing in some places Solution: - Add new FormField, FormError shared components - Refactor Input/Textarea to use FormField internally - Unify error/errorMessage props across all form fields - Context-based automatic aria attribute linking - Add nested group validation to schema: - hasEmptyNameInGroup: detect empty names in group items - hasEmptyNestedGroup: detect empty nested groups - hasEmptyCommandInGroup: properly distinguish group vs single command - Add validation logic to GroupEditDialog on save - Initialize group field when switching to Group mode in command-form - Add i18n error message keys (groupNameRequired, nestedGroupEmpty) - Add 12 e2e tests (form-required-field-validation.spec.ts)
1 parent 01bb4b6 commit 4915d26

15 files changed

Lines changed: 664 additions & 69 deletions

‎src/view/e2e/form-required-field-validation.spec.ts‎

Lines changed: 282 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { expect, test } from "@playwright/test";
22

3+
import { clearAllCommands } from "./helpers/test-helpers";
4+
35
test.describe("Test C5: Form Required Field Validation", () => {
46
test.beforeEach(async ({ page }) => {
57
await page.goto("/");
@@ -89,4 +91,284 @@ test.describe("Test C5: Form Required Field Validation", () => {
8991
const dialog = page.getByRole("dialog", { name: "Add New Command" });
9092
await expect(dialog).not.toBeVisible();
9193
});
94+
95+
test("should show validation error when group mode selected but no commands added", async ({
96+
page,
97+
}) => {
98+
await clearAllCommands(page);
99+
100+
// Given: Open add command dialog
101+
await page.getByRole("button", { name: "Add new command" }).click();
102+
await page.getByRole("textbox", { name: "Command Name" }).fill("Empty Group");
103+
104+
// When: Switch to group mode but don't add any commands
105+
await page.getByRole("radio", { name: "Group Commands" }).click();
106+
107+
// Try to save without adding any child commands
108+
const dialog = page.getByRole("dialog");
109+
await dialog.getByRole("button", { name: "Save" }).click();
110+
111+
// Then: Validation error should be displayed
112+
await expect(page.getByText("Group must have at least one command")).toBeVisible();
113+
114+
// Dialog should remain open
115+
await expect(dialog).toBeVisible();
116+
});
117+
118+
test("should show validation error for empty group child command fields", async ({
119+
page,
120+
}) => {
121+
await clearAllCommands(page);
122+
123+
// Given: Open add command dialog and switch to group mode
124+
await page.getByRole("button", { name: "Add new command" }).click();
125+
await page.getByRole("textbox", { name: "Command Name" }).fill("Test Group");
126+
await page.getByRole("radio", { name: "Group Commands" }).click();
127+
128+
// When: Add a child command but leave fields empty
129+
const dialog = page.getByRole("dialog");
130+
await dialog.getByRole("button", { name: "Add new command" }).click();
131+
132+
// Try to save without filling child command fields
133+
await dialog.getByRole("button", { name: "Save" }).click();
134+
135+
// Then: Validation errors should be displayed for empty fields
136+
await expect(page.getByText("Name is required")).toBeVisible();
137+
await expect(page.getByText("Command is required")).toBeVisible();
138+
139+
// Dialog should remain open
140+
await expect(dialog).toBeVisible();
141+
});
142+
143+
test("should show validation error in GroupEditDialog when commands are empty", async ({
144+
page,
145+
}) => {
146+
await clearAllCommands(page);
147+
148+
// Create a group first
149+
await page.getByRole("button", { name: "Add new command" }).click();
150+
await page.getByRole("textbox", { name: "Command Name" }).fill("Test Group");
151+
await page.getByRole("radio", { name: "Group Commands" }).click();
152+
153+
const dialog = page.getByRole("dialog");
154+
await dialog.getByRole("button", { name: "Add new command" }).click();
155+
156+
const commandItem = dialog.locator('[data-testid="group-command-item"]').last();
157+
await commandItem.getByPlaceholder("Command name").fill("Child");
158+
await commandItem.getByPlaceholder("Command (e.g., npm start)").fill("npm test");
159+
160+
await dialog.getByRole("button", { name: "Save" }).click();
161+
162+
// Edit and add nested group
163+
const groupCard = page.locator('[data-testid="command-card"]', { hasText: "Test Group" });
164+
await groupCard.getByRole("button", { name: /Edit command/i }).click();
165+
166+
const editDialog = page.getByRole("dialog", { name: "Edit Command" });
167+
await editDialog.getByRole("button", { name: "Add new group" }).click();
168+
169+
// Fill nested group name and click Edit
170+
const nestedGroupItem = editDialog.locator('[data-testid="group-command-item"]').last();
171+
await nestedGroupItem.getByPlaceholder("Group name").fill("Nested Group");
172+
await nestedGroupItem.getByRole("button", { name: /Edit group/i }).click();
173+
174+
// In nested dialog, try to save without adding commands
175+
const nestedDialog = page.getByRole("dialog", { name: /Edit Group/i });
176+
await expect(nestedDialog).toBeVisible();
177+
await nestedDialog.getByRole("button", { name: "Save" }).click();
178+
179+
// Should show error about empty group
180+
await expect(
181+
nestedDialog.getByText("Group must have at least one command")
182+
).toBeVisible();
183+
await expect(nestedDialog).toBeVisible();
184+
});
185+
186+
test("should show validation error in GroupEditDialog when child has empty name", async ({
187+
page,
188+
}) => {
189+
await clearAllCommands(page);
190+
191+
// Create a group first
192+
await page.getByRole("button", { name: "Add new command" }).click();
193+
await page.getByRole("textbox", { name: "Command Name" }).fill("Test Group");
194+
await page.getByRole("radio", { name: "Group Commands" }).click();
195+
196+
const dialog = page.getByRole("dialog");
197+
await dialog.getByRole("button", { name: "Add new command" }).click();
198+
199+
const commandItem = dialog.locator('[data-testid="group-command-item"]').last();
200+
await commandItem.getByPlaceholder("Command name").fill("Child");
201+
await commandItem.getByPlaceholder("Command (e.g., npm start)").fill("npm test");
202+
203+
await dialog.getByRole("button", { name: "Save" }).click();
204+
205+
// Edit and add nested group
206+
const groupCard = page.locator('[data-testid="command-card"]', { hasText: "Test Group" });
207+
await groupCard.getByRole("button", { name: /Edit command/i }).click();
208+
209+
const editDialog = page.getByRole("dialog", { name: "Edit Command" });
210+
await editDialog.getByRole("button", { name: "Add new group" }).click();
211+
212+
// Fill nested group name and click Edit
213+
const nestedGroupItem = editDialog.locator('[data-testid="group-command-item"]').last();
214+
await nestedGroupItem.getByPlaceholder("Group name").fill("Nested Group");
215+
await nestedGroupItem.getByRole("button", { name: /Edit group/i }).click();
216+
217+
// In nested dialog, add a command but leave name empty
218+
const nestedDialog = page.getByRole("dialog", { name: /Edit Group/i });
219+
await nestedDialog.getByRole("button", { name: "Add new command" }).click();
220+
await nestedDialog.getByRole("button", { name: "Save" }).click();
221+
222+
// Should show error about empty name
223+
await expect(
224+
nestedDialog.getByText("All items in group must have a name")
225+
).toBeVisible();
226+
});
227+
228+
test("should show validation error when GroupEditDialog group name is empty", async ({
229+
page,
230+
}) => {
231+
// Given: Create a group command first
232+
await clearAllCommands(page);
233+
await page.getByRole("button", { name: "Add new command" }).click();
234+
await page.getByRole("textbox", { name: "Command Name" }).fill("Test Group");
235+
await page.getByRole("radio", { name: "Group Commands" }).click();
236+
237+
const dialog = page.getByRole("dialog");
238+
await dialog.getByRole("button", { name: "Add new command" }).click();
239+
240+
const commandItem = dialog.locator('[data-testid="group-command-item"]').last();
241+
await commandItem.getByPlaceholder("Command name").fill("Child Command");
242+
await commandItem.getByPlaceholder("Command (e.g., npm start)").fill("npm test");
243+
244+
await dialog.getByRole("button", { name: "Save" }).click();
245+
await expect(dialog).not.toBeVisible();
246+
247+
// When: Edit the group and open nested group edit dialog
248+
const groupCard = page.locator('[data-testid="command-card"]', { hasText: "Test Group" });
249+
await groupCard.getByRole("button", { name: /Edit command/i }).click();
250+
251+
const editDialog = page.getByRole("dialog", { name: "Edit Command" });
252+
253+
// Add a nested group
254+
await editDialog.getByRole("button", { name: "Add new group" }).click();
255+
256+
// Edit the newly added group (which has empty name)
257+
const nestedGroupItem = editDialog.locator('[data-testid="group-command-item"]').last();
258+
await nestedGroupItem.getByRole("button", { name: /Edit group/i }).click();
259+
260+
// Then: GroupEditDialog should be visible
261+
const nestedDialog = page.getByRole("dialog", { name: /Edit Group/i });
262+
await expect(nestedDialog).toBeVisible();
263+
264+
// When: Try to save with empty group name
265+
await nestedDialog.locator("#group-name").clear();
266+
await nestedDialog.getByRole("button", { name: "Save" }).click();
267+
268+
// Then: Validation error should be displayed
269+
await expect(page.getByText("Group name is required")).toBeVisible();
270+
271+
// Dialog should remain open
272+
await expect(nestedDialog).toBeVisible();
273+
});
274+
275+
test("should show validation error when nested group has empty name", async ({
276+
page,
277+
}) => {
278+
await clearAllCommands(page);
279+
280+
// Given: Create a group with a nested group that has empty name
281+
await page.getByRole("button", { name: "Add new command" }).click();
282+
await page.getByRole("textbox", { name: "Command Name" }).fill("Parent Group");
283+
await page.getByRole("radio", { name: "Group Commands" }).click();
284+
285+
const dialog = page.getByRole("dialog");
286+
287+
// Add a nested group (with empty name)
288+
await dialog.getByRole("button", { name: "Add new group" }).click();
289+
290+
// Try to save - should fail because nested group has empty name
291+
await dialog.getByRole("button", { name: "Save" }).click();
292+
293+
// Then: Should show error about name
294+
await expect(page.getByText("All items in group must have a name")).toBeVisible();
295+
await expect(dialog).toBeVisible();
296+
});
297+
298+
test("should show validation error when nested group is empty", async ({
299+
page,
300+
}) => {
301+
await clearAllCommands(page);
302+
303+
// Given: Create a group with a nested group that has no commands
304+
await page.getByRole("button", { name: "Add new command" }).click();
305+
await page.getByRole("textbox", { name: "Command Name" }).fill("Parent Group");
306+
await page.getByRole("radio", { name: "Group Commands" }).click();
307+
308+
const dialog = page.getByRole("dialog");
309+
310+
// Add a nested group with name but no commands
311+
await dialog.getByRole("button", { name: "Add new group" }).click();
312+
const nestedGroupItem = dialog.locator('[data-testid="group-command-item"]').last();
313+
await nestedGroupItem.getByPlaceholder("Group name").fill("Empty Nested Group");
314+
315+
// Try to save - should fail because nested group has no commands
316+
await dialog.getByRole("button", { name: "Save" }).click();
317+
318+
// Then: Should show error about nested group being empty
319+
await expect(page.getByText("Nested groups must have at least one command")).toBeVisible();
320+
await expect(dialog).toBeVisible();
321+
});
322+
323+
test("should clear validation error when valid group name is entered", async ({
324+
page,
325+
}) => {
326+
// Given: Create a group command first
327+
await clearAllCommands(page);
328+
await page.getByRole("button", { name: "Add new command" }).click();
329+
await page.getByRole("textbox", { name: "Command Name" }).fill("Test Group");
330+
await page.getByRole("radio", { name: "Group Commands" }).click();
331+
332+
const dialog = page.getByRole("dialog");
333+
await dialog.getByRole("button", { name: "Add new command" }).click();
334+
335+
const commandItem = dialog.locator('[data-testid="group-command-item"]').last();
336+
await commandItem.getByPlaceholder("Command name").fill("Child");
337+
await commandItem.getByPlaceholder("Command (e.g., npm start)").fill("npm test");
338+
339+
await dialog.getByRole("button", { name: "Save" }).click();
340+
341+
// Edit and add nested group
342+
const groupCard = page.locator('[data-testid="command-card"]', { hasText: "Test Group" });
343+
await groupCard.getByRole("button", { name: /Edit command/i }).click();
344+
345+
const editDialog = page.getByRole("dialog", { name: "Edit Command" });
346+
await editDialog.getByRole("button", { name: "Add new group" }).click();
347+
348+
const nestedGroupItem = editDialog.locator('[data-testid="group-command-item"]').last();
349+
await nestedGroupItem.getByRole("button", { name: /Edit group/i }).click();
350+
351+
const nestedDialog = page.getByRole("dialog", { name: /Edit Group/i });
352+
353+
// Add a command to nested group first (so we can test name validation only)
354+
await nestedDialog.getByRole("button", { name: "Add new command" }).click();
355+
const nestedCommandItem = nestedDialog.locator('[data-testid="group-command-item"]').last();
356+
await nestedCommandItem.getByPlaceholder("Command name").fill("Nested Child");
357+
await nestedCommandItem.getByPlaceholder("Command (e.g., npm start)").fill("echo test");
358+
359+
// Trigger validation error by clearing group name
360+
await nestedDialog.locator("#group-name").clear();
361+
await nestedDialog.getByRole("button", { name: "Save" }).click();
362+
await expect(nestedDialog.getByText("Group name is required")).toBeVisible();
363+
364+
// When: Enter valid group name
365+
await nestedDialog.locator("#group-name").fill("Valid Group Name");
366+
367+
// Then: Error should be cleared
368+
await expect(nestedDialog.getByText("Group name is required")).not.toBeVisible();
369+
370+
// And: Save should succeed
371+
await nestedDialog.getByRole("button", { name: "Save" }).click();
372+
await expect(nestedDialog).not.toBeVisible();
373+
});
92374
});

‎src/view/src/components/color-input.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { HexColorPicker } from "react-colorful";
44
import { Button, Input, Popover, PopoverContent, PopoverTrigger } from "~/core";
55

66
export type ColorInputProps = {
7+
error?: boolean;
8+
errorMessage?: string;
79
id?: string;
810
onChange: (value: string) => void;
911
placeholder?: string;
@@ -52,6 +54,8 @@ const isValidColor = (color: string): boolean => {
5254
};
5355

5456
export const ColorInput = ({
57+
error,
58+
errorMessage,
5559
id,
5660
onChange,
5761
placeholder = "e.g., #FF5722, red, blue",
@@ -112,6 +116,8 @@ export const ColorInput = ({
112116
</Popover>
113117
<Input
114118
className="flex-1"
119+
error={error}
120+
errorMessage={errorMessage}
115121
id={id}
116122
onChange={(e) => onChange(e.target.value)}
117123
placeholder={placeholder}

0 commit comments

Comments
 (0)