Skip to content

Commit 1554dc8

Browse files
committed
feat: apply CodeRabbit's review suggestions
- Remove unused props from BackupSettings component - Fix config mutation by using useEffect in LLMSettings - Add block scopes in e2e test switch cases - Improve parseSessionId error handling for invalid dates - Add keyboard accessibility to expandable table rows
1 parent 7607761 commit 1554dc8

File tree

4 files changed

+40
-20
lines changed

4 files changed

+40
-20
lines changed

src/__tests__/e2e/backup-system-e2e.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ describe('Backup System E2E Tests', () => {
6464
case 'generate_session_id':
6565
return sessionId;
6666

67-
case 'backup_snbt_files':
67+
case 'backup_snbt_files': {
6868
// Simulate SNBT backup
6969
const backupDir = path.join(args.sessionPath, 'backup', 'snbt_original');
7070
await fs.mkdir(backupDir, { recursive: true });
@@ -73,15 +73,17 @@ describe('Backup System E2E Tests', () => {
7373
await fs.writeFile(path.join(backupDir, fileName), 'original content');
7474
}
7575
return;
76+
}
7677

77-
case 'backup_resource_pack':
78+
case 'backup_resource_pack': {
7879
// Simulate resource pack backup
7980
const packBackupDir = path.join(args.sessionPath, 'backup', 'resource_pack');
8081
await fs.mkdir(packBackupDir, { recursive: true });
8182
await fs.writeFile(path.join(packBackupDir, 'pack.mcmeta'), '{}');
8283
return;
84+
}
8385

84-
case 'update_translation_summary':
86+
case 'update_translation_summary': {
8587
// Simulate summary update
8688
const summaryPath = path.join(logsDir, 'localizer', args.sessionId, 'translation_summary.json');
8789
await fs.mkdir(path.dirname(summaryPath), { recursive: true });
@@ -101,6 +103,7 @@ describe('Backup System E2E Tests', () => {
101103

102104
await fs.writeFile(summaryPath, JSON.stringify(summary, null, 2));
103105
return;
106+
}
104107

105108
case 'list_translation_sessions':
106109
// Return mock sessions

src/components/settings/backup-settings.tsx

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,8 @@
33
import { useAppTranslation } from "@/lib/i18n";
44
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card";
55
import { HardDrive } from "lucide-react";
6-
import { type AppConfig } from "@/lib/types/config";
76

8-
interface BackupSettingsProps {
9-
config: AppConfig;
10-
setConfig: (config: AppConfig) => void;
11-
}
12-
13-
export function BackupSettings({ }: BackupSettingsProps) {
7+
export function BackupSettings() {
148
const { t } = useAppTranslation();
159

1610
return (

src/components/settings/llm-settings.tsx

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,26 @@ export function LLMSettings({ config, setConfig }: LLMSettingsProps) {
2121
const [showApiKey, setShowApiKey] = useState(false);
2222

2323
// Initialize apiKeys if not present
24-
if (!config.llm.apiKeys) {
25-
config.llm.apiKeys = {
26-
openai: "",
27-
anthropic: "",
28-
google: ""
29-
};
30-
}
24+
useEffect(() => {
25+
if (!config.llm.apiKeys) {
26+
const newConfig = { ...config };
27+
newConfig.llm = {
28+
...newConfig.llm,
29+
apiKeys: {
30+
openai: "",
31+
anthropic: "",
32+
google: ""
33+
}
34+
};
35+
setConfig(newConfig);
36+
}
37+
}, [config, setConfig]);
3138

3239
// Get current API key based on selected provider
3340
const getCurrentApiKey = () => {
3441
const provider = config.llm.provider as keyof typeof config.llm.apiKeys;
3542
// Use provider-specific key if available, fallback to legacy apiKey
36-
return config.llm.apiKeys[provider] || config.llm.apiKey || "";
43+
return config.llm.apiKeys?.[provider] || config.llm.apiKey || "";
3744
};
3845

3946
// Set API key for current provider

src/components/ui/translation-history-dialog.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,11 @@ const parseSessionId = (id: string): Date => {
6565
const match = id.match(/(\d{4})-(\d{2})-(\d{2})_(\d{2})-(\d{2})-(\d{2})/);
6666
if (match) {
6767
const [, year, month, day, hour, minute, second] = match;
68-
return new Date(parseInt(year), parseInt(month) - 1, parseInt(day), parseInt(hour), parseInt(minute), parseInt(second));
68+
const parsedDate = new Date(parseInt(year), parseInt(month) - 1, parseInt(day), parseInt(hour), parseInt(minute), parseInt(second));
69+
// Return current date if parsing resulted in invalid date
70+
return isNaN(parsedDate.getTime()) ? new Date() : parsedDate;
6971
}
72+
// Return current date for invalid format
7073
return new Date();
7174
};
7275

@@ -202,7 +205,20 @@ function SessionRow({ sessionSummary, onToggle, minecraftDir, updateSession }: {
202205

203206
return (
204207
<>
205-
<TableRow className="cursor-pointer" onClick={onToggle}>
208+
<TableRow
209+
className="cursor-pointer hover:bg-muted/50 focus:bg-muted/50 focus:outline-none"
210+
onClick={onToggle}
211+
onKeyDown={(e) => {
212+
if (e.key === 'Enter' || e.key === ' ') {
213+
e.preventDefault();
214+
onToggle();
215+
}
216+
}}
217+
tabIndex={0}
218+
role="button"
219+
aria-expanded={sessionSummary.expanded}
220+
aria-label={`${sessionSummary.expanded ? 'Collapse' : 'Expand'} session ${formatSessionId(sessionSummary.sessionId)}`}
221+
>
206222
<TableCell>
207223
<div className="flex items-center space-x-2">
208224
{sessionSummary.expanded ? <ChevronDown className="h-4 w-4" /> : <ChevronRight className="h-4 w-4" />}

0 commit comments

Comments
 (0)