fix: keybinding changes failing silently due to incorrect key ordering (#524)

* fix: enhance error handling for shortcut bindings and settings persistence

Signed-off-by: Noorain Panjwani <noorain.panjwani@gmail.com>

* Oops. This wasn't needed

Signed-off-by: Noorain Panjwani <noorain.panjwani@gmail.com>

* style

* format

---------

Signed-off-by: Noorain Panjwani <noorain.panjwani@gmail.com>
Co-authored-by: CJ Pais <cj@cjpais.com>

src-tauri/src/shortcut.rs

@@ -624,10 +624,13 @@ pub fn change_app_language_setting(app: AppHandle, language: String) -> Result<(
     Ok(())
 }
 
-/// Determine whether a shortcut string contains at least one non-modifier key.
+/// Validate that a shortcut contains at least one non-modifier key.
-/// We allow single non-modifier keys (e.g. "f5" or "space") but disallow
+/// The tauri-plugin-global-shortcut library requires at least one main key.
-/// modifier-only combos (e.g. "ctrl" or "ctrl+shift").
 fn validate_shortcut_string(raw: &str) -> Result<(), String> {
+    if raw.trim().is_empty() {
+        return Err("Shortcut cannot be empty".into());
+    }
+
     let modifiers = [
         "ctrl", "control", "shift", "alt", "option", "meta", "command", "cmd", "super", "win",
         "windows",
@@ -635,10 +638,11 @@ fn validate_shortcut_string(raw: &str) -> Result<(), String> {
     let has_non_modifier = raw
         .split('+')
         .any(|part| !modifiers.contains(&part.trim().to_lowercase().as_str()));
+
     if has_non_modifier {
         Ok(())
     } else {
-        Err("Shortcut must contain at least one non-modifier key".into())
+        Err("Shortcut must include a main key (letter, number, F-key, etc.) in addition to modifiers".into())
     }
 }
 

src/App.tsx

@@ -75,7 +75,18 @@ function App() {
 
   return (
     <div className="h-screen flex flex-col select-none cursor-default">
-      <Toaster />
+      <Toaster
+        theme="system"
+        toastOptions={{
+          unstyled: true,
+          classNames: {
+            toast:
+              "bg-background border border-mid-gray/20 rounded-lg shadow-lg px-4 py-3 flex items-center gap-3 text-sm",
+            title: "font-medium",
+            description: "text-mid-gray",
+          },
+        }}
+      />
       {/* Main content area that takes remaining space */}
       <div className="flex-1 flex overflow-hidden">
         <Sidebar

src/components/settings/HandyShortcut.tsx

@@ -86,9 +86,6 @@ export const HandyShortcut: React.FC<HandyShortcutProps> = ({
         if (editingShortcutId && originalBinding) {
           try {
             await updateBinding(editingShortcutId, originalBinding);
-            await commands
-              .resumeBinding(editingShortcutId)
-              .catch(console.error);
           } catch (error) {
             console.error("Failed to restore original binding:", error);
             toast.error(t("settings.general.shortcut.errors.restore"));
@@ -132,15 +129,32 @@ export const HandyShortcut: React.FC<HandyShortcutProps> = ({
       const updatedKeyPressed = keyPressed.filter((k) => k !== key);
       if (updatedKeyPressed.length === 0 && recordedKeys.length > 0) {
         // Create the shortcut string from all recorded keys
-        const newShortcut = recordedKeys.join("+");
+        // Sort keys so modifiers come first, then the main key
+        const modifiers = [
+          "ctrl",
+          "control",
+          "shift",
+          "alt",
+          "option",
+          "meta",
+          "command",
+          "cmd",
+          "super",
+          "win",
+          "windows",
+        ];
+        const sortedKeys = recordedKeys.sort((a, b) => {
+          const aIsModifier = modifiers.includes(a.toLowerCase());
+          const bIsModifier = modifiers.includes(b.toLowerCase());
+          if (aIsModifier && !bIsModifier) return -1;
+          if (!aIsModifier && bIsModifier) return 1;
+          return 0;
+        });
+        const newShortcut = sortedKeys.join("+");
 
         if (editingShortcutId && bindings[editingShortcutId]) {
           try {
             await updateBinding(editingShortcutId, newShortcut);
-            // Re-register the shortcut now that recording is finished
-            await commands
-              .resumeBinding(editingShortcutId)
-              .catch(console.error);
           } catch (error) {
             console.error("Failed to change binding:", error);
             toast.error(
@@ -153,9 +167,6 @@ export const HandyShortcut: React.FC<HandyShortcutProps> = ({
             if (originalBinding) {
               try {
                 await updateBinding(editingShortcutId, originalBinding);
-                await commands
-                  .resumeBinding(editingShortcutId)
-                  .catch(console.error);
               } catch (resetError) {
                 console.error("Failed to reset binding:", resetError);
                 toast.error(t("settings.general.shortcut.errors.reset"));
@@ -181,9 +192,6 @@ export const HandyShortcut: React.FC<HandyShortcutProps> = ({
         if (editingShortcutId && originalBinding) {
           try {
             await updateBinding(editingShortcutId, originalBinding);
-            await commands
-              .resumeBinding(editingShortcutId)
-              .catch(console.error);
           } catch (error) {
             console.error("Failed to restore original binding:", error);
             toast.error(t("settings.general.shortcut.errors.restore"));

src/stores/settingsStore.ts

@@ -307,7 +307,17 @@ export const useSettingsStore = create<SettingsStore>()(
             : null,
         }));
 
-        await commands.changeBinding(id, binding);
+        const result = await commands.changeBinding(id, binding);
+
+        // Check if the command executed successfully
+        if (result.status === "error") {
+          throw new Error(result.error);
+        }
+
+        // Check if the binding change was successful
+        if (!result.data.success) {
+          throw new Error(result.data.error || "Failed to update binding");
+        }
       } catch (error) {
         console.error(`Failed to update binding ${id}:`, error);
 
@@ -328,6 +338,9 @@ export const useSettingsStore = create<SettingsStore>()(
               : null,
           }));
         }
+
+        // Re-throw to let the caller know it failed
+        throw error;
       } finally {
         setUpdating(updateKey, false);
       }