refactor(keyboard): use declarative routing table for shortcut handling
Replaced scattered if-else logic with centralized ShortcutRouting table that defines how each shortcut should be handled. Changes: - Add ShortcutRouting struct with forwardToJS, blockWebKit, description fields - Create shortcutRoutingTable map with routing decisions for each shortcut - Add defaultRouting for shortcuts not in table - Simplify bridge logic to table lookup Benefits: - No magic strings - all routing in one place - Self-documenting with descriptions - Easy to extend - just add table entries - Clear behavior for each shortcut Routes: - Alt+Arrow: GTK only (pane navigation) - Ctrl+P: Forward to JS + block WebKit print dialog - Others: Forward to JS, allow WebKit (default)
- commit
- 00b77e6ffe1537a5b8101f49301f28239fd52578
- author
- bamen · · 2025-10-09 21:08 UTC
- committer
- bamen · · 2025-10-09 20:20 UTC
- parent
- 3f10f36
- signature
- unsigned
1 file · +55 -36
M pkg/webkit/keyboard_bridge.go +55 -36
| @@ -29,22 +29,49 @@ const ( | |||
| 29 | 29 | ||
| 30 | 30 | // Key name constants for shortcut mapping | |
| 31 | 31 | const ( | |
| 32 | - | keyP = "p" | |
| 33 | - | keyL = "l" | |
| 34 | - | keyF = "f" | |
| 35 | - | keyC = "c" | |
| 36 | - | keyW = "w" | |
| 37 | - | keyR = "r" | |
| 38 | - | keyArrowLeft = "arrowleft" | |
| 32 | + | keyP = "p" | |
| 33 | + | keyL = "l" | |
| 34 | + | keyF = "f" | |
| 35 | + | keyC = "c" | |
| 36 | + | keyW = "w" | |
| 37 | + | keyR = "r" | |
| 38 | + | keyArrowLeft = "arrowleft" | |
| 39 | 39 | keyArrowRight = "arrowright" | |
| 40 | - | keyArrowUp = "arrowup" | |
| 41 | - | keyArrowDown = "arrowdown" | |
| 42 | - | keyF12 = "F12" | |
| 43 | - | keyPlus = "plus" | |
| 44 | - | keyMinus = "minus" | |
| 45 | - | keyZero = "0" | |
| 40 | + | keyArrowUp = "arrowup" | |
| 41 | + | keyArrowDown = "arrowdown" | |
| 42 | + | keyF12 = "F12" | |
| 43 | + | keyPlus = "plus" | |
| 44 | + | keyMinus = "minus" | |
| 45 | + | keyZero = "0" | |
| 46 | 46 | ) | |
| 47 | 47 | ||
| 48 | + | // ShortcutRouting defines how a keyboard shortcut should be handled by the bridge | |
| 49 | + | type ShortcutRouting struct { | |
| 50 | + | forwardToJS bool // Should dispatch dumber:key event to JavaScript | |
| 51 | + | blockWebKit bool // Should block WebKit's default handling (return true from handler) | |
| 52 | + | description string // Human-readable description for debugging | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | // shortcutRoutingTable maps normalized shortcuts to their routing behavior | |
| 56 | + | // This centralizes all routing decisions in one place, avoiding magic strings and scattered logic | |
| 57 | + | var shortcutRoutingTable = map[string]ShortcutRouting{ | |
| 58 | + | // Workspace pane navigation - handled by GTK window-level shortcuts only | |
| 59 | + | "alt+arrowleft": {forwardToJS: false, blockWebKit: true, description: "Navigate to left pane (GTK handler)"}, | |
| 60 | + | "alt+arrowright": {forwardToJS: false, blockWebKit: true, description: "Navigate to right pane (GTK handler)"}, | |
| 61 | + | "alt+arrowup": {forwardToJS: false, blockWebKit: true, description: "Navigate to upper pane (GTK handler)"}, | |
| 62 | + | "alt+arrowdown": {forwardToJS: false, blockWebKit: true, description: "Navigate to lower pane (GTK handler)"}, | |
| 63 | + | ||
| 64 | + | // Pane mode - forward to JavaScript but block WebKit's print dialog | |
| 65 | + | "cmdorctrl+p": {forwardToJS: true, blockWebKit: true, description: "Enter pane mode (JS handler + block print dialog)"}, | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | // defaultRouting defines the default behavior for shortcuts not in the routing table | |
| 69 | + | var defaultRouting = ShortcutRouting{ | |
| 70 | + | forwardToJS: true, // Forward all shortcuts to JS by default | |
| 71 | + | blockWebKit: false, // Allow WebKit to handle (for reload, devtools, zoom, etc.) | |
| 72 | + | description: "Default: forward to JS, allow WebKit", | |
| 73 | + | } | |
| 74 | + | ||
| 48 | 75 | // AttachKeyboardBridge attaches an EventControllerKey to bridge keyboard events to JavaScript | |
| 49 | 76 | // This is critical for allowing JavaScript KeyboardService to receive keyboard events | |
| 50 | 77 | // that would otherwise be consumed by GTK shortcuts | |
| @@ -134,33 +161,25 @@ func (w *WebView) AttachKeyboardBridge() { | |||
| 134 | 161 | } | |
| 135 | 162 | ||
| 136 | 163 | if shortcut != "" { | |
| 137 | - | // Dispatch custom dumber:key event to JavaScript | |
| 138 | - | script := fmt.Sprintf(`document.dispatchEvent(new CustomEvent('dumber:key',{detail:{shortcut:'%s'}}));`, shortcut) | |
| 139 | - | if err := w.InjectScript(script); err != nil { | |
| 140 | - | log.Printf("[keyboard-bridge] Failed to dispatch key event: %v", err) | |
| 164 | + | // Look up routing behavior from table, use default if not found | |
| 165 | + | routing, exists := shortcutRoutingTable[shortcut] | |
| 166 | + | if !exists { | |
| 167 | + | routing = defaultRouting | |
| 141 | 168 | } | |
| 142 | - | log.Printf("[keyboard-bridge] Forwarded shortcut to JS: %s", shortcut) | |
| 143 | - | ||
| 144 | - | // Only block WebKit's default handling for shortcuts that would conflict: | |
| 145 | - | // - Alt+Arrow: WebKit uses these for back/forward navigation -> BLOCK | |
| 146 | - | // - Ctrl+P: Pane mode, would trigger print dialog -> BLOCK | |
| 147 | - | // - Ctrl+R, Ctrl+Shift+R: Page reload, WebKit should handle -> ALLOW | |
| 148 | - | // - F12: DevTools, WebKit should handle -> ALLOW | |
| 149 | - | // - All others: Custom app handlers, don't conflict with WebKit -> ALLOW | |
| 150 | - | if len(parts) >= 2 { | |
| 151 | - | modifier := parts[0] | |
| 152 | - | if modifier == modifierAlt { | |
| 153 | - | // Alt+Arrow keys - block WebKit's back/forward navigation | |
| 154 | - | return true | |
| 155 | - | } | |
| 156 | - | if modifier == modifierCmdOrCtrl && keyName == keyP { | |
| 157 | - | // Ctrl+P (with or without shift) - block WebKit's print dialog | |
| 158 | - | return true | |
| 169 | + | ||
| 170 | + | // Dispatch to JavaScript if routing says to | |
| 171 | + | if routing.forwardToJS { | |
| 172 | + | script := fmt.Sprintf(`document.dispatchEvent(new CustomEvent('dumber:key',{detail:{shortcut:'%s'}}));`, shortcut) | |
| 173 | + | if err := w.InjectScript(script); err != nil { | |
| 174 | + | log.Printf("[keyboard-bridge] Failed to dispatch key event: %v", err) | |
| 159 | 175 | } | |
| 176 | + | log.Printf("[keyboard-bridge] Forwarded shortcut to JS: %s", shortcut) | |
| 177 | + | } else { | |
| 178 | + | log.Printf("[keyboard-bridge] Shortcut %s: %s (not forwarded to JS)", shortcut, routing.description) | |
| 160 | 179 | } | |
| 161 | 180 | ||
| 162 | - | // Allow WebKit to also handle this event (for reload, devtools, zoom, etc.) | |
| 163 | - | return false | |
| 181 | + | // Return routing decision to block or allow WebKit | |
| 182 | + | return routing.blockWebKit | |
| 164 | 183 | } | |
| 165 | 184 | ||
| 166 | 185 | // No shortcut matched - allow WebKit to handle normally | |
| @@ -29,22 +29,49 @@ const ( | |||||
| 29 | 29 | ||||
| 30 | // Key name constants for shortcut mapping | 30 | // Key name constants for shortcut mapping | ||
| 31 | const ( | 31 | const ( | ||
| 32 | - | keyP = "p" | 32 | + | keyP = "p" |
| 33 | - | keyL = "l" | 33 | + | keyL = "l" |
| 34 | - | keyF = "f" | 34 | + | keyF = "f" |
| 35 | - | keyC = "c" | 35 | + | keyC = "c" |
| 36 | - | keyW = "w" | 36 | + | keyW = "w" |
| 37 | - | keyR = "r" | 37 | + | keyR = "r" |
| 38 | - | keyArrowLeft = "arrowleft" | 38 | + | keyArrowLeft = "arrowleft" |
| 39 | keyArrowRight = "arrowright" | 39 | keyArrowRight = "arrowright" | ||
| 40 | - | keyArrowUp = "arrowup" | 40 | + | keyArrowUp = "arrowup" |
| 41 | - | keyArrowDown = "arrowdown" | 41 | + | keyArrowDown = "arrowdown" |
| 42 | - | keyF12 = "F12" | 42 | + | keyF12 = "F12" |
| 43 | - | keyPlus = "plus" | 43 | + | keyPlus = "plus" |
| 44 | - | keyMinus = "minus" | 44 | + | keyMinus = "minus" |
| 45 | - | keyZero = "0" | 45 | + | keyZero = "0" |
| 46 | ) | 46 | ) | ||
| 47 | 47 | ||||
| 48 | + | // ShortcutRouting defines how a keyboard shortcut should be handled by the bridge | |||
| 49 | + | type ShortcutRouting struct { | |||
| 50 | + | forwardToJS bool // Should dispatch dumber:key event to JavaScript | |||
| 51 | + | blockWebKit bool // Should block WebKit's default handling (return true from handler) | |||
| 52 | + | description string // Human-readable description for debugging | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | // shortcutRoutingTable maps normalized shortcuts to their routing behavior | |||
| 56 | + | // This centralizes all routing decisions in one place, avoiding magic strings and scattered logic | |||
| 57 | + | var shortcutRoutingTable = map[string]ShortcutRouting{ | |||
| 58 | + | // Workspace pane navigation - handled by GTK window-level shortcuts only | |||
| 59 | + | "alt+arrowleft": {forwardToJS: false, blockWebKit: true, description: "Navigate to left pane (GTK handler)"}, | |||
| 60 | + | "alt+arrowright": {forwardToJS: false, blockWebKit: true, description: "Navigate to right pane (GTK handler)"}, | |||
| 61 | + | "alt+arrowup": {forwardToJS: false, blockWebKit: true, description: "Navigate to upper pane (GTK handler)"}, | |||
| 62 | + | "alt+arrowdown": {forwardToJS: false, blockWebKit: true, description: "Navigate to lower pane (GTK handler)"}, | |||
| 63 | + | ||||
| 64 | + | // Pane mode - forward to JavaScript but block WebKit's print dialog | |||
| 65 | + | "cmdorctrl+p": {forwardToJS: true, blockWebKit: true, description: "Enter pane mode (JS handler + block print dialog)"}, | |||
| 66 | + | } | |||
| 67 | + | ||||
| 68 | + | // defaultRouting defines the default behavior for shortcuts not in the routing table | |||
| 69 | + | var defaultRouting = ShortcutRouting{ | |||
| 70 | + | forwardToJS: true, // Forward all shortcuts to JS by default | |||
| 71 | + | blockWebKit: false, // Allow WebKit to handle (for reload, devtools, zoom, etc.) | |||
| 72 | + | description: "Default: forward to JS, allow WebKit", | |||
| 73 | + | } | |||
| 74 | + | ||||
| 48 | // AttachKeyboardBridge attaches an EventControllerKey to bridge keyboard events to JavaScript | 75 | // AttachKeyboardBridge attaches an EventControllerKey to bridge keyboard events to JavaScript | ||
| 49 | // This is critical for allowing JavaScript KeyboardService to receive keyboard events | 76 | // This is critical for allowing JavaScript KeyboardService to receive keyboard events | ||
| 50 | // that would otherwise be consumed by GTK shortcuts | 77 | // that would otherwise be consumed by GTK shortcuts | ||
| @@ -134,33 +161,25 @@ func (w *WebView) AttachKeyboardBridge() { | |||||
| 134 | } | 161 | } | ||
| 135 | 162 | ||||
| 136 | if shortcut != "" { | 163 | if shortcut != "" { | ||
| 137 | - | // Dispatch custom dumber:key event to JavaScript | 164 | + | // Look up routing behavior from table, use default if not found |
| 138 | - | script := fmt.Sprintf(`document.dispatchEvent(new CustomEvent('dumber:key',{detail:{shortcut:'%s'}}));`, shortcut) | 165 | + | routing, exists := shortcutRoutingTable[shortcut] |
| 139 | - | if err := w.InjectScript(script); err != nil { | 166 | + | if !exists { |
| 140 | - | log.Printf("[keyboard-bridge] Failed to dispatch key event: %v", err) | 167 | + | routing = defaultRouting |
| 141 | } | 168 | } | ||
| 142 | - | log.Printf("[keyboard-bridge] Forwarded shortcut to JS: %s", shortcut) | 169 | + | |
| 143 | - | 170 | + | // Dispatch to JavaScript if routing says to | |
| 144 | - | // Only block WebKit's default handling for shortcuts that would conflict: | 171 | + | if routing.forwardToJS { |
| 145 | - | // - Alt+Arrow: WebKit uses these for back/forward navigation -> BLOCK | 172 | + | script := fmt.Sprintf(`document.dispatchEvent(new CustomEvent('dumber:key',{detail:{shortcut:'%s'}}));`, shortcut) |
| 146 | - | // - Ctrl+P: Pane mode, would trigger print dialog -> BLOCK | 173 | + | if err := w.InjectScript(script); err != nil { |
| 147 | - | // - Ctrl+R, Ctrl+Shift+R: Page reload, WebKit should handle -> ALLOW | 174 | + | log.Printf("[keyboard-bridge] Failed to dispatch key event: %v", err) |
| 148 | - | // - F12: DevTools, WebKit should handle -> ALLOW | |||
| 149 | - | // - All others: Custom app handlers, don't conflict with WebKit -> ALLOW | |||
| 150 | - | if len(parts) >= 2 { | |||
| 151 | - | modifier := parts[0] | |||
| 152 | - | if modifier == modifierAlt { | |||
| 153 | - | // Alt+Arrow keys - block WebKit's back/forward navigation | |||
| 154 | - | return true | |||
| 155 | - | } | |||
| 156 | - | if modifier == modifierCmdOrCtrl && keyName == keyP { | |||
| 157 | - | // Ctrl+P (with or without shift) - block WebKit's print dialog | |||
| 158 | - | return true | |||
| 159 | } | 175 | } | ||
| 176 | + | log.Printf("[keyboard-bridge] Forwarded shortcut to JS: %s", shortcut) | |||
| 177 | + | } else { | |||
| 178 | + | log.Printf("[keyboard-bridge] Shortcut %s: %s (not forwarded to JS)", shortcut, routing.description) | |||
| 160 | } | 179 | } | ||
| 161 | 180 | ||||
| 162 | - | // Allow WebKit to also handle this event (for reload, devtools, zoom, etc.) | 181 | + | // Return routing decision to block or allow WebKit |
| 163 | - | return false | 182 | + | return routing.blockWebKit |
| 164 | } | 183 | } | ||
| 165 | 184 | ||||
| 166 | // No shortcut matched - allow WebKit to handle normally | 185 | // No shortcut matched - allow WebKit to handle normally | ||