Skip to content

Commit 01bb4b6

Browse files
committed
fix: add validation feedback for empty fields in group command save
Problem: - Saving group button with empty commands showed no feedback - Users couldn't tell what went wrong Solution: - Added 3 Zod schema validations (empty group, empty command, empty commands in group) - Display red border (aria-invalid) on empty fields - Added i18n error messages (en/ko) - Added related E2E tests
1 parent 7d81194 commit 01bb4b6

11 files changed

Lines changed: 129 additions & 160 deletions

src/view/e2e/empty-state.spec.ts

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

3-
type DefaultCommand = { name: string; command: string } | { name: string; isGroup: true };
3+
type DefaultCommand = { name: string; command: string };
44

55
const DEFAULT_COMMANDS: DefaultCommand[] = [
66
{ name: "$(pass) Test", command: "npm test" },
77
{ name: "$(terminal) Terminal", command: "echo 'Hello World'" },
8-
{ name: "$(git-branch) Git", isGroup: true },
8+
{ name: "$(git-branch) Git", command: "git status" },
99
];
1010

1111
test.describe("Test 15: Empty State UI", () => {
@@ -52,11 +52,7 @@ test.describe("Test 15: Empty State UI", () => {
5252
for (const cmd of DEFAULT_COMMANDS) {
5353
await page.getByRole("button", { name: "Add new command" }).click();
5454
await page.getByRole("textbox", { name: "Command Name" }).fill(cmd.name);
55-
if ("command" in cmd) {
56-
await page.getByRole("textbox", { name: "Command", exact: true }).fill(cmd.command);
57-
} else {
58-
await page.getByLabel("Group Commands").click();
59-
}
55+
await page.getByRole("textbox", { name: "Command", exact: true }).fill(cmd.command);
6056
await page.getByRole("button", { name: "Save" }).click();
6157
await expect(page.getByRole("dialog")).not.toBeVisible();
6258
}

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

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,26 @@ test.describe("Test C5: Form Required Field Validation", () => {
2828
await expect(dialog).toBeVisible();
2929
});
3030

31+
test("should show validation error when command field is empty for single command", async ({
32+
page,
33+
}) => {
34+
// Given: Open add command dialog
35+
await page.getByRole("button", { name: "Add new command" }).click();
36+
37+
const dialog = page.getByRole("dialog", { name: "Add New Command" });
38+
await expect(dialog).toBeVisible();
39+
40+
// When: Fill name but leave command empty, then save
41+
await page.getByRole("textbox", { name: "Command Name" }).fill("Test");
42+
await page.getByRole("button", { name: "Save" }).click();
43+
44+
// Then: Command validation error should be displayed
45+
await expect(page.getByText("Command is required")).toBeVisible();
46+
47+
// Dialog should remain open
48+
await expect(dialog).toBeVisible();
49+
});
50+
3151
test("should clear validation error after valid input and re-submit", async ({
3252
page,
3353
}) => {
@@ -38,16 +58,17 @@ test.describe("Test C5: Form Required Field Validation", () => {
3858
const errorMessage = page.getByText("Command name is required");
3959
await expect(errorMessage).toBeVisible();
4060

41-
// When: Fill name field and attempt save again
61+
// When: Fill all required fields and attempt save again
4262
await page.getByRole("textbox", { name: "Command Name" }).fill("Test");
63+
await page.getByPlaceholder("e.g., npm start").fill("npm test");
4364
await page.getByRole("button", { name: "Save" }).click();
4465

4566
// Then: Dialog should close (validation passed)
4667
const dialog = page.getByRole("dialog", { name: "Add New Command" });
4768
await expect(dialog).not.toBeVisible();
4869
});
4970

50-
test("should allow save after filling required name field", async ({
71+
test("should allow save after filling all required fields", async ({
5172
page,
5273
}) => {
5374
// Given: Open dialog
@@ -57,10 +78,11 @@ test.describe("Test C5: Form Required Field Validation", () => {
5778
await page.getByRole("button", { name: "Save" }).click();
5879
await expect(page.getByText("Command name is required")).toBeVisible();
5980

60-
// When: Fill the required name field and save
81+
// When: Fill all required fields and save
6182
await page
6283
.getByRole("textbox", { name: "Command Name" })
6384
.fill("ValidCommand");
85+
await page.getByPlaceholder("e.g., npm start").fill("npm run build");
6486
await page.getByRole("button", { name: "Save" }).click();
6587

6688
// Then: Dialog should close (form submitted successfully)

src/view/e2e/keyboard-navigation-dialog.spec.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,11 @@ test.describe("Test H3: Keyboard Navigation - Dialog", () => {
5050
const dialog = page.getByRole("dialog", { name: "Add New Command" });
5151
await expect(dialog).toBeVisible();
5252

53-
// Fill command name and press Enter
53+
// Fill command name and command (required field)
5454
const nameInput = page.getByRole("textbox", { name: "Command Name" });
5555
await nameInput.fill("EnterKeyTest");
56+
const commandInput = page.getByRole("textbox", { name: "Command", exact: true });
57+
await commandInput.fill("echo test");
5658
await page.keyboard.press("Enter");
5759

5860
// Dialog should close and command should be added

src/view/e2e/validate-duplicate-shortcuts.spec.ts

Lines changed: 0 additions & 132 deletions
This file was deleted.

src/view/src/components/command-form.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const createDefaultValues = (command?: ButtonConfig | null): ButtonConfigDraft =
4646
color: "",
4747
command: "",
4848
executeAll: false,
49-
group: [],
49+
group: undefined,
5050
id: crypto.randomUUID(),
5151
insertOnly: false,
5252
name: "",
@@ -192,6 +192,7 @@ export const CommandForm = ({ command, formId, onSave }: CommandFormProps) => {
192192
<div className="space-y-2">
193193
<FormLabel htmlFor="command">{t("commandForm.command")}</FormLabel>
194194
<Input
195+
aria-invalid={!!errors.command}
195196
id="command"
196197
placeholder={
197198
useVsCodeApi
@@ -200,6 +201,9 @@ export const CommandForm = ({ command, formId, onSave }: CommandFormProps) => {
200201
}
201202
{...register("command")}
202203
/>
204+
{errors.command && (
205+
<p className="text-sm text-red-500">{t("commandForm.errors.commandRequired")}</p>
206+
)}
203207
{useVsCodeApi && (
204208
<p className="text-xs text-muted-foreground">
205209
{t("commandForm.vsCodeApiTip")}{" "}
@@ -295,8 +299,16 @@ export const CommandForm = ({ command, formId, onSave }: CommandFormProps) => {
295299
<FormLabel>{t("commandForm.groupCommandsLabel")}</FormLabel>
296300
<GroupCommandEditor
297301
commands={groupCommands || []}
302+
hasError={!!errors.group}
298303
onChange={(commands) => setValue("group", commands)}
299304
/>
305+
{errors.group && (
306+
<p className="text-sm text-red-500">
307+
{errors.group.message?.includes("at least one")
308+
? t("commandForm.errors.groupEmpty")
309+
: t("commandForm.errors.groupCommandRequired")}
310+
</p>
311+
)}
300312
</div>
301313
</div>
302314
)}

src/view/src/components/group-command-editor.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,20 @@ import { GroupEditDialog } from "./group-edit-dialog";
88
type GroupCommandEditorProps = {
99
commands: ButtonConfig[];
1010
depth?: number;
11+
hasError?: boolean;
1112
onChange: (commands: ButtonConfig[]) => void;
1213
title?: string;
1314
};
1415

1516
export const GroupCommandEditor = ({
1617
commands,
1718
depth = 0,
19+
hasError = false,
1820
onChange,
1921
title,
2022
}: GroupCommandEditorProps) => {
2123
return (
22-
<CommandEditProvider commands={commands} onCommandsChange={onChange}>
24+
<CommandEditProvider commands={commands} hasError={hasError} onCommandsChange={onChange}>
2325
<GroupCommandEditorContent depth={depth} title={title} />
2426
</CommandEditProvider>
2527
);

src/view/src/components/group-command-item.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ type GroupCommandItemProps = {
3535
export const GroupCommandItem = ({ command, id, index, onEditGroup }: GroupCommandItemProps) => {
3636
const { t } = useTranslation();
3737
const isGroup = !!command.group;
38-
const { deleteCommand, updateCommand } = useCommandEdit();
38+
const { deleteCommand, hasError, updateCommand } = useCommandEdit();
39+
const isCommandEmpty = !isGroup && (!command.command || command.command.trim() === "");
40+
const isNameEmpty = !command.name || command.name.trim() === "";
3941

4042
const { attributes, listeners, setNodeRef, style } = useSortableItem(id);
4143

@@ -74,6 +76,7 @@ export const GroupCommandItem = ({ command, id, index, onEditGroup }: GroupComma
7476

7577
<div className="flex-1 space-y-2">
7678
<Input
79+
aria-invalid={hasError && isNameEmpty}
7780
onChange={(e) => updateCommand(index, { name: e.target.value })}
7881
placeholder={
7982
isGroup
@@ -86,6 +89,7 @@ export const GroupCommandItem = ({ command, id, index, onEditGroup }: GroupComma
8689
{!isGroup && (
8790
<>
8891
<Input
92+
aria-invalid={hasError && isCommandEmpty}
8993
onChange={(e) => updateCommand(index, { command: e.target.value })}
9094
placeholder={t("groupCommandItem.commandPlaceholder")}
9195
value={command.command || ""}

src/view/src/context/command-edit-context.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ type CommandEditContextType = {
2323
commands: ButtonConfig[];
2424
deleteCommand: (index: number) => void;
2525
editingGroup: EditingGroup;
26+
hasError: boolean;
2627
onCommandsChange: (commands: ButtonConfig[]) => void;
2728
openGroupEditor: (group: GroupButton, index: number, depth: number) => void;
2829
saveGroup: (updatedGroup: GroupButton) => void;
@@ -42,12 +43,14 @@ export const useCommandEdit = () => {
4243
type CommandEditProviderProps = {
4344
children: ReactNode;
4445
commands: ButtonConfig[];
46+
hasError?: boolean;
4547
onCommandsChange: (commands: ButtonConfig[]) => void;
4648
};
4749

4850
export const CommandEditProvider = ({
4951
children,
5052
commands,
53+
hasError = false,
5154
onCommandsChange,
5255
}: CommandEditProviderProps) => {
5356
const [editingGroup, setEditingGroup] = useState<EditingGroup>(null);
@@ -126,6 +129,7 @@ export const CommandEditProvider = ({
126129
commands,
127130
deleteCommand,
128131
editingGroup,
132+
hasError,
129133
onCommandsChange,
130134
openGroupEditor,
131135
saveGroup,

src/view/src/i18n/locales/en.json

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,12 @@
5050
"groupCommandsLabel": "Group Commands",
5151
"color": "Color (optional)",
5252
"shortcut": "Shortcut (optional)",
53-
"shortcutPlaceholder": "e.g., t"
53+
"shortcutPlaceholder": "e.g., t",
54+
"errors": {
55+
"commandRequired": "Command is required",
56+
"groupEmpty": "Group must have at least one command",
57+
"groupCommandRequired": "All commands in group must have a command"
58+
}
5459
},
5560
"commandFormDialog": {
5661
"editCommand": "Edit Command",

src/view/src/i18n/locales/ko.json

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,12 @@
5050
"groupCommandsLabel": "그룹 명령",
5151
"color": "색상 (optional)",
5252
"shortcut": "단축키 (optional)",
53-
"shortcutPlaceholder": "예: t"
53+
"shortcutPlaceholder": "예: t",
54+
"errors": {
55+
"commandRequired": "명령어를 입력해주세요",
56+
"groupEmpty": "그룹에 최소 하나의 커맨드가 필요합니다",
57+
"groupCommandRequired": "그룹 내 모든 커맨드에 명령어를 입력해주세요"
58+
}
5459
},
5560
"commandFormDialog": {
5661
"editCommand": "명령 편집",

0 commit comments

Comments
 (0)