-
Notifications
You must be signed in to change notification settings - Fork 0
fix: Improve Nav layer overlay consistency and TouchID icon #120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
95b511a
78829c7
ceb8f8d
01c1977
90edbbe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -541,24 +541,31 @@ class KeyboardVisualizationViewModel: ObservableObject { | |
| /// Update the current layer and rebuild key mapping | ||
| func updateLayer(_ layerName: String) { | ||
| let wasLauncherMode = isLauncherModeActive | ||
| currentLayerName = layerName | ||
|
|
||
| // IMPORTANT: Don't update currentLayerName yet - wait until mapping is ready | ||
| // This prevents UI flash where old mapping shows with new layer name | ||
| let targetLayerName = layerName | ||
|
|
||
| // Clear tap-hold sources on layer change to prevent stale suppressions | ||
| // (e.g., user switches layers while holding a tap-hold key) | ||
| activeTapHoldSources.removeAll() | ||
|
|
||
| // Check if we'll be entering/exiting launcher mode | ||
| let willBeLauncherMode = targetLayerName.lowercased() == Self.launcherLayerName | ||
|
|
||
| // Load/clear launcher mappings when entering/exiting launcher mode | ||
| let isNowLauncherMode = isLauncherModeActive | ||
| if isNowLauncherMode, !wasLauncherMode { | ||
| if willBeLauncherMode, !wasLauncherMode { | ||
| loadLauncherMappings() | ||
| } else if !isNowLauncherMode, wasLauncherMode { | ||
| } else if !willBeLauncherMode, wasLauncherMode { | ||
| launcherMappings.removeAll() | ||
| } | ||
|
|
||
| // Reset idle timer on any layer change (including returning to base) | ||
| noteInteraction() | ||
| noteTcpEventReceived() | ||
| rebuildLayerMapping() | ||
|
|
||
| // Build mapping first, then update layer name atomically when ready | ||
| rebuildLayerMappingForLayer(targetLayerName) | ||
| } | ||
|
|
||
| /// Load launcher mappings from the Quick Launcher rule collection | ||
|
|
@@ -626,6 +633,12 @@ class KeyboardVisualizationViewModel: ObservableObject { | |
|
|
||
| /// Rebuild the key mapping for the current layer | ||
| func rebuildLayerMapping() { | ||
| rebuildLayerMappingForLayer(currentLayerName) | ||
| } | ||
|
|
||
| /// Rebuild the key mapping for a specific layer | ||
| /// Updates both the layer name and mapping atomically to prevent UI flash | ||
| private func rebuildLayerMappingForLayer(_ targetLayerName: String) { | ||
| // Cancel any in-flight mapping task | ||
| layerMapTask?.cancel() | ||
|
|
||
|
|
@@ -636,7 +649,7 @@ class KeyboardVisualizationViewModel: ObservableObject { | |
| } | ||
|
|
||
| isLoadingLayerMap = true | ||
| AppLogger.shared.info("🗺️ [KeyboardViz] Starting layer mapping build for '\(currentLayerName)'...") | ||
| AppLogger.shared.info("🗺️ [KeyboardViz] Starting layer mapping build for '\(targetLayerName)'...") | ||
|
|
||
| layerMapTask = Task { [weak self] in | ||
| guard let self else { return } | ||
|
|
@@ -645,37 +658,47 @@ class KeyboardVisualizationViewModel: ObservableObject { | |
| let configPath = WizardSystemPaths.userConfigPath | ||
| AppLogger.shared.debug("🗺️ [KeyboardViz] Using config: \(configPath)") | ||
|
|
||
| // Build mapping for current layer | ||
| // Build mapping for target layer | ||
| var mapping = try await layerKeyMapper.getMapping( | ||
| for: currentLayerName, | ||
| for: targetLayerName, | ||
| configPath: configPath, | ||
| layout: layout | ||
| ) | ||
|
|
||
| // DEBUG: Log what simulator returned | ||
| AppLogger.shared.info("🗺️ [KeyboardViz] Simulator returned \(mapping.count) entries for '\(targetLayerName)'") | ||
| for (keyCode, info) in mapping.prefix(20) { | ||
| AppLogger.shared.debug(" [\(targetLayerName)] keyCode \(keyCode) -> '\(info.displayLabel)'") | ||
| } | ||
|
|
||
| // Augment mapping with push-msg actions from custom rules and rule collections | ||
| // Include base layer so app/system/URL icons display for remapped keys | ||
| // Only include actions targeting this specific layer | ||
| let customRules = await CustomRulesStore.shared.loadRules() | ||
| let ruleCollections = await RuleCollectionStore.shared.loadCollections() | ||
| AppLogger.shared.info("🗺️ [KeyboardViz] Augmenting '\(currentLayerName)' with \(customRules.count) custom rules and \(ruleCollections.count) collections") | ||
| AppLogger.shared.info("🗺️ [KeyboardViz] Augmenting '\(targetLayerName)' with \(customRules.count) custom rules and \(ruleCollections.count) collections") | ||
| mapping = augmentWithPushMsgActions( | ||
| mapping: mapping, | ||
| customRules: customRules, | ||
| ruleCollections: ruleCollections | ||
| ruleCollections: ruleCollections, | ||
| currentLayerName: targetLayerName | ||
| ) | ||
|
|
||
| // Apply app-specific overrides for the current frontmost app | ||
| mapping = await applyAppSpecificOverrides(to: mapping) | ||
|
|
||
| // Update on main actor with explicit objectWillChange to ensure SwiftUI notices | ||
| // Update layer name and mapping atomically to prevent UI flash | ||
| // This ensures the UI never shows mismatched layer name + old mapping | ||
| await MainActor.run { | ||
| self.objectWillChange.send() | ||
| self.currentLayerName = targetLayerName | ||
| self.layerKeyMap = mapping | ||
| self.remapOutputMap = self.buildRemapOutputMap(from: mapping) | ||
| self.isLoadingLayerMap = false | ||
| AppLogger.shared.info("🗺️ [KeyboardViz] Updated layerKeyMap with \(mapping.count) entries, remapOutputMap with \(self.remapOutputMap.count) remaps") | ||
| AppLogger.shared | ||
| .info("🗺️ [KeyboardViz] Updated currentLayerName to '\(targetLayerName)' and layerKeyMap with \(mapping.count) entries, remapOutputMap with \(self.remapOutputMap.count) remaps") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cancelled task may still update layer state causing inconsistencyMedium Severity The Additional Locations (1) |
||
| } | ||
|
|
||
| AppLogger.shared.info("🗺️ [KeyboardViz] Built layer mapping for '\(currentLayerName)': \(mapping.count) keys") | ||
| AppLogger.shared.info("🗺️ [KeyboardViz] Built layer mapping for '\(targetLayerName)': \(mapping.count) keys") | ||
|
|
||
| // Log a few sample mappings for debugging | ||
| for (keyCode, info) in mapping.prefix(5) { | ||
|
|
@@ -714,19 +737,35 @@ class KeyboardVisualizationViewModel: ObservableObject { | |
| /// - mapping: The base layer key mapping from the simulator | ||
| /// - customRules: Custom rules to check for push-msg patterns | ||
| /// - ruleCollections: Preset rule collections to check for push-msg patterns | ||
| /// - currentLayerName: The layer name to filter collections/rules by (only include matching layers) | ||
| /// - Returns: Mapping with action info added where applicable | ||
| private func augmentWithPushMsgActions( | ||
| mapping: [UInt16: LayerKeyInfo], | ||
| customRules: [CustomRule], | ||
| ruleCollections: [RuleCollection] | ||
| ruleCollections: [RuleCollection], | ||
| currentLayerName: String | ||
| ) -> [UInt16: LayerKeyInfo] { | ||
| var augmented = mapping | ||
|
|
||
| // Build lookups from input key -> LayerKeyInfo | ||
| var actionByInput: [String: LayerKeyInfo] = [:] | ||
|
|
||
| // First, process rule collections (lower priority - can be overridden by custom rules) | ||
| // Only process collections that target the current layer or base layer | ||
| for collection in ruleCollections where collection.isEnabled { | ||
| // Check if this collection targets the current layer | ||
| let collectionLayerName = collection.targetLayer.kanataName.lowercased() | ||
| let currentLayer = currentLayerName.lowercased() | ||
|
|
||
| // Only include mappings from collections targeting this layer | ||
| // Exception: base layer gets base-layer collections only | ||
| guard collectionLayerName == currentLayer else { | ||
| AppLogger.shared.debug("🗺️ [KeyboardViz] Skipping collection '\(collection.name)' (targets '\(collectionLayerName)', current layer '\(currentLayer)')") | ||
| continue | ||
| } | ||
|
|
||
| AppLogger.shared.debug("🗺️ [KeyboardViz] Including collection '\(collection.name)' (\(collection.mappings.count) mappings)") | ||
|
|
||
| for keyMapping in collection.mappings { | ||
| let input = keyMapping.input.lowercased() | ||
| // First try push-msg pattern (apps, system actions, URLs) | ||
|
|
@@ -749,6 +788,15 @@ class KeyboardVisualizationViewModel: ObservableObject { | |
|
|
||
| // Then, process custom rules (higher priority - overrides collections) | ||
| for rule in customRules where rule.isEnabled { | ||
| // Check if this rule targets the current layer | ||
| let ruleLayerName = rule.targetLayer.kanataName.lowercased() | ||
| let currentLayer = currentLayerName.lowercased() | ||
|
|
||
| // Only include rules targeting this layer | ||
| guard ruleLayerName == currentLayer else { | ||
| continue | ||
| } | ||
|
|
||
| let input = rule.input.lowercased() | ||
| // First try push-msg pattern (apps, system actions, URLs) | ||
| if let info = Self.extractPushMsgInfo(from: rule.output, description: rule.notes) { | ||
|
|
@@ -779,9 +827,17 @@ class KeyboardVisualizationViewModel: ObservableObject { | |
| AppLogger.shared.info("🗺️ [KeyboardViz] Found \(actionByInput.count) actions (push-msg + simple remaps)") | ||
|
|
||
| // Update mapping entries | ||
| for (keyCode, _) in mapping { | ||
| // IMPORTANT: Only augment keys that are NOT transparent (XX) | ||
| // Transparent keys should pass through without showing action labels | ||
| for (keyCode, originalInfo) in mapping { | ||
| let keyName = OverlayKeyboardView.keyCodeToKanataName(keyCode).lowercased() | ||
| if let info = actionByInput[keyName] { | ||
| // Skip augmentation if the original key is transparent (XX) | ||
| // Transparent keys should not show action labels from collections/rules | ||
| if originalInfo.isTransparent { | ||
| AppLogger.shared.debug("🗺️ [KeyboardViz] Skipping augmentation for transparent key \(keyName)(\(keyCode))") | ||
| continue | ||
| } | ||
| augmented[keyCode] = info | ||
| AppLogger.shared.debug("🗺️ [KeyboardViz] Key \(keyName)(\(keyCode)) -> '\(info.displayLabel)'") | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -419,6 +419,14 @@ final class LiveKeyboardOverlayController: NSObject, NSWindowDelegate { | |
| } | ||
| } | ||
|
|
||
| private func bringOverlayToFront() { | ||
| if !isVisible { | ||
| isVisible = true | ||
| } | ||
| NSApp.activate(ignoringOtherApps: true) | ||
| window?.makeKeyAndOrderFront(nil) | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused private method added but never calledLow Severity The private method |
||
|
|
||
| /// Restore overlay state from previous session | ||
| /// Only restores if system status is healthy (Kanata running) | ||
| func restoreState() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because
rebuildLayerMappingForLayernow capturestargetLayerNameand updatescurrentLayerNameonly when the async task finishes, a canceled/slow task can still complete and overwrite the UI with an older layer if the user switches layers quickly (cancellation is cooperative andLayerKeyMapper.getMappingdoesn’t check it). This can leave the overlay showing the wrong layer/mapping after rapid layer changes. Consider checkingTask.isCancelled(or comparing against a “latest requested layer” token) before applying results on the MainActor.Useful? React with 👍 / 👎.