Skip to content

Commit 9f70da4

Browse files
Copilothotlong
andcommitted
fix: address code review feedback - UUID, delete total check, a11y labels, key matching
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 319e887 commit 9f70da4

3 files changed

Lines changed: 22 additions & 10 deletions

File tree

apps/web/src/components/sync/ConflictResolutionDialog.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,12 @@ export function ConflictResolutionDialog({
5454
<thead>
5555
<tr className="border-b bg-muted/50">
5656
<th className="px-3 py-2 text-left font-medium" scope="col">Field</th>
57-
<th className="px-3 py-2 text-left font-medium" scope="col">Local</th>
58-
<th className="px-3 py-2 text-left font-medium" scope="col">Server</th>
57+
<th className="px-3 py-2 text-left font-medium" scope="col">
58+
<span className="mr-1"></span> Local
59+
</th>
60+
<th className="px-3 py-2 text-left font-medium" scope="col">
61+
Server <span className="ml-1"></span>
62+
</th>
5963
</tr>
6064
</thead>
6165
<tbody>

apps/web/src/hooks/use-keyboard-shortcuts.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,15 @@ export function useKeyboardShortcuts(shortcuts: KeyboardShortcut[]) {
4747
const shiftMatch = shortcut.shift ? event.shiftKey : !event.shiftKey;
4848
const altMatch = shortcut.alt ? event.altKey : !event.altKey;
4949

50-
if (
51-
event.key.toLowerCase() === shortcut.key.toLowerCase() &&
52-
ctrlMatch &&
53-
shiftMatch &&
54-
altMatch
55-
) {
50+
// Compare key with case sensitivity: use event.key directly for
51+
// special keys (length > 1) or shifted characters (e.g. '?').
52+
// For single alpha characters, compare case-insensitively.
53+
const keyMatches =
54+
shortcut.key.length === 1 && /[a-zA-Z]/.test(shortcut.key)
55+
? event.key.toLowerCase() === shortcut.key.toLowerCase()
56+
: event.key === shortcut.key;
57+
58+
if (keyMatches && ctrlMatch && shiftMatch && altMatch) {
5659
if (!shortcut.allowInInput && isInputFocused()) continue;
5760

5861
event.preventDefault();

apps/web/src/hooks/use-records.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ export function useCreateRecord({ objectName }: UseCreateRecordOptions) {
107107
const previous = queryClient.getQueriesData<RecordListResponse>({ queryKey: ['records', objectName] });
108108
queryClient.setQueriesData<RecordListResponse>(
109109
{ queryKey: ['records', objectName] },
110-
(old) => old ? { ...old, records: [...old.records, { id: `optimistic-${Date.now()}`, ...newData } as RecordData], total: old.total + 1 } : old,
110+
(old) => old ? { ...old, records: [...old.records, { id: crypto.randomUUID(), ...newData } as RecordData], total: old.total + 1 } : old,
111111
);
112112
return { previous };
113113
},
@@ -181,7 +181,12 @@ export function useDeleteRecord({ objectName }: UseDeleteRecordOptions) {
181181
const previous = queryClient.getQueriesData<RecordListResponse>({ queryKey: ['records', objectName] });
182182
queryClient.setQueriesData<RecordListResponse>(
183183
{ queryKey: ['records', objectName] },
184-
(old) => old ? { ...old, records: old.records.filter((r) => String(r.id) !== recordId), total: Math.max(0, old.total - 1) } : old,
184+
(old) => {
185+
if (!old) return old;
186+
const filtered = old.records.filter((r) => String(r.id) !== recordId);
187+
const removed = filtered.length < old.records.length;
188+
return { ...old, records: filtered, total: removed ? Math.max(0, old.total - 1) : old.total };
189+
},
185190
);
186191
return { previous };
187192
},

0 commit comments

Comments
 (0)