-
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
Conversation
Remove borders completely during both per-key release fade and global overlay fade. Keys now rely entirely on shadows for separation at all times, maintaining the clean borderless aesthetic even during transitions. Changes: - keyStroke: Always returns opacity 0 (no borders ever) - strokeWidth: Always returns 0 (no stroke width ever) User preference: cleaner look without any outlines, including fade states.
…r labels
Layer action labels (e.g., window snapping, navigation) are now readable and
discoverable. Previously, labels were truncated ("M...", "Ya...") due to
limited keycap space, making layers difficult to learn and use.
Changes:
- Add SF Symbol mapping for 30+ common actions (window management, navigation,
text editing, search)
- Add hover tooltips showing full action descriptions on all layer keys
- Add dynamic font sizing + multi-line wrapping for labels without SF Symbols
- Fix layer filtering: collections/rules now only apply to their target layer
(prevents Window Snapping labels appearing on Nav layer)
- Remove redundant "App Launcher" sequence preset (use Quick Launcher drawer)
SF Symbol coverage:
- Window halves/thirds/corners (rectangle symbols)
- Display/space movement (arrows with lines/squares)
- Text editing (copy, paste, delete, undo/redo)
- Navigation (arrow symbols)
Fallback text labels dynamically shrink from 10pt → 8pt → 6pt based on length,
wrap to 2 lines, and use minimumScaleFactor for extreme cases.
Related: #118 (floating HUD for future enhancement)
Related: #119 (cache invalidation for toggled collections)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Transparent keys (XX in config) were incorrectly showing action labels from rule collections, making them appear "mapped" when they should pass through to the base layer. Root cause: augmentWithPushMsgActions() was replacing LayerKeyInfo for ALL keys that matched collection/rule inputs, including transparent keys. The replacement LayerKeyInfo (created via .mapped(), .systemAction(), etc.) had isTransparent hardcoded to false, losing the transparency flag from the simulator. Fix: Skip augmentation for keys where originalInfo.isTransparent == true. Only augment keys that actually have mappings on the current layer. Example: On Nav layer, symbol keys like [, ], \, numbers are XX (transparent). Previously they showed orange highlights incorrectly. Now they remain dark. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ayers Transparent keys (XX in config) were being incorrectly marked as isTransparent=false when the simulator returned no output events, causing them to appear highlighted despite being transparent. Root cause: LayerKeyMapper.parseRawSimulationEvents() returns nil when there are no output events (the correct behavior for XX keys). However, the caller was creating LayerKeyInfo with isTransparent hardcoded to false for these nil results. Fix: On non-base layers, no simulator outputs = transparent (XX). Set isTransparent=true when parseRawSimulationEvents returns nil on layers other than base. Result: Symbol keys ([, ], \, numbers, etc.) now correctly appear dark on Nav layer instead of being highlighted orange. Tested: Verified on Nav layer - only actual Vim mappings (H/J/K/L, etc.) are highlighted, while transparent keys remain dark. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace TouchID lock emoji with drawer icon (sidebar.right) across all layers - Prevent bottom row modifiers (fn, ctrl, opt, cmd) from showing as mapped in layer modes - Render unmapped Nav layer keys with lowercase text labels (tab, shift, delete, return) - Maintain modifier symbols (⌃, ⌥, ⌘) instead of converting to text - Position hyper symbol (✦) at bottom of caps lock key to avoid indicator overlap - Keep bottom row styling consistent between base and Nav layers - Add special fn key rendering in mapped layer mode to preserve globe + "fn" appearance Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Code Review: PR #120This PR makes solid improvements to the Nav layer overlay consistency. The core logic is sound with excellent work on atomic layer switching and transparent key handling. Key Strengths
Areas for ImprovementShould Fix Before Merge
Nice to Have
Security & Architecture✅ No security concerns - purely UI/UX changes Approval: Conditional ✅The implementation is solid (8/10). Before merging: add kTouchIDKeyCode constant, verify sfSymbolForAction() completeness, and run full test suite. Great work on the atomic layer switching! 🎉 |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90edbbe006
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await MainActor.run { | ||
| self.objectWillChange.send() | ||
| self.currentLayerName = targetLayerName | ||
| self.layerKeyMap = mapping |
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.
Guard against stale layer-map tasks overwriting state
Because rebuildLayerMappingForLayer now captures targetLayerName and updates currentLayerName only 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 and LayerKeyMapper.getMapping doesn’t check it). This can leave the overlay showing the wrong layer/mapping after rapid layer changes. Consider checking Task.isCancelled (or comparing against a “latest requested layer” token) before applying results on the MainActor.
Useful? React with 👍 / 👎.
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.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 24
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Cancelled task may still update layer state causing inconsistency
Medium Severity
The rebuildLayerMappingForLayer function cancels the previous task at line 643 but the MainActor.run block that updates currentLayerName and layerKeyMap has no Task.isCancelled check. Swift task cancellation is cooperative, so a cancelled task can still complete its scheduled work. With rapid layer switches, an older cancelled task may update state after a newer task, causing the overlay to display the wrong layer. The file uses Task.isCancelled checks elsewhere (lines 425, 459) but not before this critical state update.
Additional Locations (1)
| } | ||
| NSApp.activate(ignoringOtherApps: true) | ||
| window?.makeKeyAndOrderFront(nil) | ||
| } |
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.
Unused private method added but never called
Low Severity
The private method bringOverlayToFront() is defined but never called anywhere in the codebase. This appears to be dead code that was accidentally included in the commit, as the PR description does not mention adding this functionality. The method duplicates functionality already present in other methods like showForQuickLaunch.
Summary
Changes
TouchID Key
Bottom Row Modifiers
Nav Layer Text Labels
Code Quality
Testing
🤖 Generated with Claude Code
Note
Enhances overlay behavior and Nav layer consistency while tightening mapping logic.
currentLayerNameto avoid UI flash; added detailed loggingfn; unmapped keys in Nav show word labels; dynamic SF Symbols/text for actions; TouchID always shows drawer (sidebar.right) icon; header pill gets layer-specific icons/animations; minor header layout tweaksWritten by Cursor Bugbot for commit 90edbbe. This will update automatically on new commits. Configure here.