fix(ui): Fix reference guide key hint for PC #782
Conversation
… hints The KeyboardShortcutHint component now detects the user's platform and displays 'Ctrl' instead of '⌘' for Windows/Linux users. This fixes the issue where the Reference Guide hint (and other keyboard shortcut hints) incorrectly showed Mac-specific keyboard symbols on non-Mac platforms. Co-authored-by: michael <michael@sourcebot.dev>
|
Cursor Agent can help with this pull request. Just |
|
Caution Review failedThe pull request is closed. WalkthroughA platform-aware keyboard shortcut rendering fix is implemented to display "Ctrl" on non-Mac keyboards instead of the Mac command key (⌘). The CHANGELOG.md documents this fix, and the keyboardShortcutHint.tsx component is updated with platform detection logic using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
msukkari
changed the title
|
Thanks for raising, fix will be in new release which should be up in a few minutes |
| return shortcut; | ||
| } | ||
| // Replace Mac Command key symbol with Ctrl for non-Mac platforms | ||
| return shortcut.replace(/⌘/g, 'Ctrl'); |
There was a problem hiding this comment.
@msukkari we probably want mapping for ⌥ and ^ as well.
There was a problem hiding this comment.
I can spin up another PR to replace all of them, because it looks like ⌘ is referenced in a lot of other places too. Just to sanity check ⌘ == ctrl, ⌥ == alt in Sourcebot?
There was a problem hiding this comment.
i.e. is the actual logic mapping for the key correct in Sourcebot for PC or does that need to be fixed too
There was a problem hiding this comment.
I would look at https://www.npmjs.com/package/react-hotkeys-hook to see what mapping it uses
There was a problem hiding this comment.
I think an improvement here would actually to just take whatever format react-hotkeys expects in KeyboardShortcutHint and translate that into what we display in the shortcut. I believe every KeyboardShortcutHint will be paired with a useHotkeys, so it probably makes sense to make it consistent and share the same hotkey as a variable s.t., we avoid running into situations where the hint and the actual hotkey do not match.
Dynamically display keyboard shortcuts as '⌘' on Mac and 'Ctrl' on non-Mac platforms to fix incorrect key hints (SOU-255).
Fixes #780
Linear Issue: SOU-255
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.