Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
89 changes: 81 additions & 8 deletions apps/desktop/src/components/shortcut-input.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import React, { useState, useEffect } from "react";
import { Button } from "@/components/ui/button";
import { Pencil, X } from "lucide-react";
import { Button, buttonVariants } from "@/components/ui/button";
import { Undo2, X } from "lucide-react";
import { TooltipProvider } from "@/components/ui/tooltip";
import { api } from "@/trpc/react";
import { toast } from "sonner";
import { getKeyFromKeycode } from "@/utils/keycode-map";
import { cn } from "@/lib/utils";
import { usePreviousShortcut } from "@/hooks/usePreviousShortcut";

interface ShortcutInputProps {
value?: number[];
Expand Down Expand Up @@ -109,17 +111,24 @@ 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={cn(
buttonVariants({ variant: "outline", size: "sm" }),
"gap-2",
)}
>
{displayValue && (
<kbd
onClick={onEdit}
Expand All @@ -132,11 +141,44 @@ function ShortcutDisplay({
variant="ghost"
size="sm"
className="h-6 w-6 p-0"
onClick={onEdit}
onClick={onClear}
>
<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={cn(
buttonVariants({ variant: "outline", size: "sm" }),
"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}
>
<Undo2 className="h-3 w-3" />
</Button>
)}
</div>
);
}

Expand All @@ -147,9 +189,12 @@ export function ShortcutInput({
onRecordingShortcutChange,
}: ShortcutInputProps) {
const [activeKeys, setActiveKeys] = useState<number[]>([]);
const { previousKeys, savePrevious, clearPrevious } = usePreviousShortcut();
const setRecordingStateMutation =
api.settings.setShortcutRecordingState.useMutation();

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

const handleStartRecording = () => {
onRecordingShortcutChange(true);
setRecordingStateMutation.mutate(true);
Expand All @@ -161,6 +206,20 @@ 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();
}
};

// 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
Expand All @@ -176,8 +235,8 @@ export function ShortcutInput({
const result = validateShortcutFormat(previousKeys);

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 +257,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 +269,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
21 changes: 21 additions & 0 deletions apps/desktop/src/hooks/useLocalStorage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { useState, useEffect } from "react";

export function useLocalStorage<T>(
key: string,
initialValue: T,
): [T, React.Dispatch<React.SetStateAction<T>>] {
const [storedValue, setStoredValue] = useState<T>(() => {
const item = localStorage.getItem(key);
return item ? JSON.parse(item) : initialValue;
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

useEffect(() => {
if (JSON.stringify(storedValue) === JSON.stringify(initialValue)) {
localStorage.removeItem(key);
} else {
localStorage.setItem(key, JSON.stringify(storedValue));
}
}, [key, storedValue, initialValue]);

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

const STORAGE_KEY = "previous-shortcut";

export function usePreviousShortcut() {
const [previousKeys, setPreviousKeys] = useLocalStorage<number[]>(
STORAGE_KEY,
[],
);

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
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