Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 86 additions & 15 deletions apps/desktop/src/components/shortcut-input.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import React, { useState, useEffect } from "react";
import { Undo2, X } from "lucide-react";
import { useEffect, useRef, useState } from "react";
import { toast } from "sonner";
import { Button } from "@/components/ui/button";
import { Pencil, X } from "lucide-react";
import { TooltipProvider } from "@/components/ui/tooltip";
import { usePreviousShortcut } from "@/hooks/usePreviousShortcut";
import { api } from "@/trpc/react";
import { toast } from "sonner";
import { getKeyFromKeycode } from "@/utils/keycode-map";

interface ShortcutInputProps {
value?: number[];
onChange: (value: number[]) => void;
isRecordingShortcut?: boolean;
onRecordingShortcutChange: (recording: boolean) => void;
shortcutId: string;
}

const MODIFIER_KEYS = new Set([
Expand Down Expand Up @@ -99,6 +101,7 @@ function RecordingDisplay({
size="sm"
className="h-6 w-6 p-0"
onClick={onCancel}
aria-label="Cancel recording"
>
<X className="h-3 w-3" />
</Button>
Expand All @@ -109,17 +112,19 @@ function RecordingDisplay({
function ShortcutDisplay({
value,
onEdit,
onClear,
}: {
value?: number[];
onEdit: () => void;
onClear: () => void;
}) {
// Format array as display string (e.g., ["Fn", "Space"] -> "Fn+Space")
const displayValue = value?.length
? value.map((key) => keycodeToDisplay(key)).join("+")
: undefined;

return (
<>
<div className="inline-flex items-center gap-2">
{displayValue && (
<kbd
onClick={onEdit}
Expand All @@ -132,11 +137,41 @@ function ShortcutDisplay({
variant="ghost"
size="sm"
className="h-6 w-6 p-0"
onClick={onEdit}
onClick={onClear}
aria-label="Clear shortcut"
>
<Pencil className="h-3 w-3" />
<X className="h-3 w-3" />
</Button>
</>
</div>
);
}

function NoneDisplay({
previousKeys,
onEdit,
onRestore,
}: {
previousKeys?: number[];
onEdit: () => void;
onRestore: () => void;
}) {
return (
<div className="inline-flex items-center gap-2">
<Button variant="ghost" size="sm" onClick={onEdit}>
Set shortcut...
</Button>
{previousKeys && previousKeys.length > 0 && (
<Button
variant="ghost"
size="sm"
className="h-6 w-6 p-0"
onClick={onRestore}
aria-label="Restore previous shortcut"
>
<Undo2 className="h-3 w-3" />
</Button>
)}
</div>
);
}

Expand All @@ -145,11 +180,17 @@ export function ShortcutInput({
onChange,
isRecordingShortcut = false,
onRecordingShortcutChange,
shortcutId,
}: ShortcutInputProps) {
const [activeKeys, setActiveKeys] = useState<number[]>([]);
const activeKeysRef = useRef<number[]>([]);
const { previousKeys, savePrevious, clearPrevious } =
usePreviousShortcut(shortcutId);
const setRecordingStateMutation =
api.settings.setShortcutRecordingState.useMutation();

const hasShortcut = value && value.length > 0;

const handleStartRecording = () => {
onRecordingShortcutChange(true);
setRecordingStateMutation.mutate(true);
Expand All @@ -161,23 +202,39 @@ export function ShortcutInput({
setRecordingStateMutation.mutate(false);
};

const handleClearRecording = () => {
if (value && value.length > 0) {
savePrevious(value);
}
onChange([]);
};

const handleRestorePrevious = () => {
if (previousKeys.length > 0) {
onChange(previousKeys);
clearPrevious();
}
};

// Keep ref in sync with state for use in subscription callback
useEffect(() => {
activeKeysRef.current = activeKeys;
}, [activeKeys]);

// Subscribe to key events when recording
// Note: activeKeys closure is fresh on each render because useSubscription
// updates its callback reference, so previousKeys correctly captures the
// previous state value when onData fires.
api.settings.activeKeysUpdates.useSubscription(undefined, {
enabled: isRecordingShortcut,
onData: (keys: number[]) => {
const previousKeys = activeKeys;
const prevActiveKeys = activeKeysRef.current;
setActiveKeys(keys);

// When any key is released, validate the combination
if (previousKeys.length > 0 && keys.length < previousKeys.length) {
const result = validateShortcutFormat(previousKeys);
if (prevActiveKeys.length > 0 && keys.length < prevActiveKeys.length) {
const result = validateShortcutFormat(prevActiveKeys);

if (result.valid && result.shortcut) {
// Basic format is valid - let parent handle backend validation
onChange(result.shortcut);
clearPrevious();
} else {
toast.error(result.error || "Invalid key combination");
}
Expand All @@ -198,6 +255,10 @@ export function ShortcutInput({
}
}, [isRecordingShortcut]);

if (value === undefined) {
return null;
}
Comment on lines +258 to +260

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all ShortcutInput call sites
rg -n --type=tsx -C2 '<ShortcutInput'

Repository: amicalhq/amical

Length of output: 86


🏁 Script executed:

#!/bin/bash
# Find all ShortcutInput call sites - search in all files
rg -n '<ShortcutInput' -A 2 -B 2

Repository: amicalhq/amical

Length of output: 2450


🏁 Script executed:

#!/bin/bash
# First, check the ShortcutInput component definition in the target file
cat -n apps/desktop/src/components/shortcut-input.tsx | head -100

Repository: amicalhq/amical

Length of output: 3348


🏁 Script executed:

#!/bin/bash
# Check lines around 258-260
sed -n '250,270p' apps/desktop/src/components/shortcut-input.tsx | cat -n

Repository: amicalhq/amical

Length of output: 679


🏁 Script executed:

#!/bin/bash
# Check the settings file for state initialization
sed -n '1,150p' apps/desktop/src/renderer/main/pages/settings/shortcuts/index.tsx | cat -n

Repository: amicalhq/amical

Length of output: 6660


🏁 Script executed:

#!/bin/bash
# Check the onboarding component
sed -n '1,80p' apps/desktop/src/renderer/onboarding/components/shared/OnboardingShortcutInput.tsx | cat -n

Repository: amicalhq/amical

Length of output: 2441


Components render nothing during shortcut data load.
ShortcutsSettingsPage initializes shortcuts as undefined and relies on async data loading, causing ShortcutInput to return null until the query completes. This leaves blank space in the UI during load. Consider:

  • Adding a default initial state (e.g., []) instead of undefined
  • Showing a loading skeleton or placeholder during the async phase

Note: OnboardingShortcutInput already defaults to [], so this is primarily an issue in the settings page.

🤖 Prompt for AI Agents
In `@apps/desktop/src/components/shortcut-input.tsx` around lines 258 - 260,
ShortcutInput currently returns null when prop value is undefined, causing blank
UI while ShortcutsSettingsPage loads async data; change the behavior so the UI
shows a placeholder instead of nothing or ensure the parent provides a default
empty array. Either (A) update ShortcutsSettingsPage to initialize its shortcuts
state to [] (match OnboardingShortcutInput) so ShortcutInput always receives an
array, or (B) modify ShortcutInput (component ShortcutInput in
shortcut-input.tsx) to render a lightweight loading skeleton/placeholder when
value === undefined rather than returning null, keeping existing behavior for
real empty arrays.


return (
<TooltipProvider>
<div className="inline-flex items-center gap-2">
Expand All @@ -206,8 +267,18 @@ export function ShortcutInput({
activeKeys={activeKeys}
onCancel={handleCancelRecording}
/>
) : hasShortcut ? (
<ShortcutDisplay
value={value}
onEdit={handleStartRecording}
onClear={handleClearRecording}
/>
) : (
<ShortcutDisplay value={value} onEdit={handleStartRecording} />
<NoneDisplay
previousKeys={previousKeys}
onEdit={handleStartRecording}
onRestore={handleRestorePrevious}
/>
)}
</div>
</TooltipProvider>
Expand Down
50 changes: 50 additions & 0 deletions apps/desktop/src/hooks/useLocalStorage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { useState, useEffect } from "react";

// Safe localStorage utilities that never throw
export const safeStorage = {
getItem(key: string): string | null {
try {
return localStorage.getItem(key);
} catch {
return null;
}
},
setItem(key: string, value: string): boolean {
try {
localStorage.setItem(key, value);
return true;
} catch {
return false;
}
},
removeItem(key: string): boolean {
try {
localStorage.removeItem(key);
return true;
} catch {
return false;
}
},
};

export function useLocalStorage<T>(
key: string,
initialValue: T,
): [T, React.Dispatch<React.SetStateAction<T>>] {
const [value, setValue] = useState<T>(() => {
const item = safeStorage.getItem(key);
if (item === null) return initialValue;
try {
return JSON.parse(item);
} catch {
safeStorage.removeItem(key);
return initialValue;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
});

useEffect(() => {
safeStorage.setItem(key, JSON.stringify(value));
}, [key, value]);

return [value, setValue];
}
37 changes: 37 additions & 0 deletions apps/desktop/src/hooks/usePreviousShortcut.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { useCallback } from "react";
import { useLocalStorage } from "./useLocalStorage";

export function usePreviousShortcut(shortcutId: string) {
const storageKey = `previous-shortcut-${shortcutId}`;
const [previousKeys, setPreviousKeys] = useLocalStorage<number[]>(
storageKey,
[],
);

const savePrevious = useCallback(
(keys: number[]) => {
if (keys.length > 0) {
setPreviousKeys(keys);
}
},
[setPreviousKeys],
);

const restorePrevious = useCallback(() => {
const keys = previousKeys;
setPreviousKeys([]);
return keys;
}, [previousKeys, setPreviousKeys]);

const clearPrevious = useCallback(() => {
setPreviousKeys([]);
}, [setPreviousKeys]);

return {
previousKeys,
savePrevious,
restorePrevious,
clearPrevious,
hasPrevious: previousKeys.length > 0,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import { api } from "@/trpc/react";
import { toast } from "sonner";

export function ShortcutsSettingsPage() {
const [pushToTalkShortcut, setPushToTalkShortcut] = useState<number[]>([]);
const [pushToTalkShortcut, setPushToTalkShortcut] = useState<
number[] | undefined
>();
const [toggleRecordingShortcut, setToggleRecordingShortcut] = useState<
number[]
>([]);
number[] | undefined
>();
const [pasteLastTranscriptShortcut, setPasteLastTranscriptShortcut] =
useState<number[]>([]);
useState<number[] | undefined>();
const [recordingShortcut, setRecordingShortcut] = useState<
"pushToTalk" | "toggleRecording" | "pasteLastTranscript" | null
>(null);
Expand Down Expand Up @@ -113,6 +115,7 @@ export function ShortcutsSettingsPage() {
onRecordingShortcutChange={(recording) =>
setRecordingShortcut(recording ? "pushToTalk" : null)
}
shortcutId="pushToTalk"
/>
</div>
</div>
Expand Down Expand Up @@ -140,6 +143,7 @@ export function ShortcutsSettingsPage() {
onRecordingShortcutChange={(recording) =>
setRecordingShortcut(recording ? "toggleRecording" : null)
}
shortcutId="toggleRecording"
/>
</div>
</div>
Expand Down Expand Up @@ -168,6 +172,7 @@ export function ShortcutsSettingsPage() {
recording ? "pasteLastTranscript" : null,
)
}
shortcutId="pasteLastTranscript"
/>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export function OnboardingShortcutInput() {
onChange={handleShortcutChange}
isRecordingShortcut={isRecording}
onRecordingShortcutChange={setIsRecording}
shortcutId="pushToTalk"
/>
</div>
</div>
Expand Down
5 changes: 5 additions & 0 deletions apps/desktop/src/utils/shortcut-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,11 @@ export function validateShortcutComprehensive(
const { candidateShortcut, candidateType, shortcutsByType, platform } =
context;

// Allow empty shortcut (user intentionally disabling)
if (candidateShortcut.length === 0) {
return { valid: true };
}

const otherShortcuts = Object.entries(shortcutsByType)
.filter(([shortcutType]) => shortcutType !== candidateType)
.map(([, shortcutKeys]) => shortcutKeys);
Expand Down