Conversation
85ad46a to
775459d
Compare
| scopeId, | ||
| hotkey: 'ctrl+c', | ||
| condition: () => | ||
| Boolean(bigChatOpen() || isRightPanelOpen()) && Boolean(stream()), |
There was a problem hiding this comment.
my stance is that we shouldn't set unnecessary conditions. I think they are best used for situations where you would not want to surface the hotkey to users unless certain conditions are met. i think you can remove all of these. what I'd do instead, is, if there is not stream, have the keyDownHandler return false, so that it doesn't capture the keypress.
|
|
||
| createEffect(() => { | ||
| console.log('activescope', activeScope()); | ||
| }); |
| const handleKeyUp = (e: KeyboardEvent) => { | ||
| if (e.key === 'Meta') setIsMetaHeld(false); | ||
| if (e.key === 'Control') setIsCtrlHeld(false); | ||
| }; |
There was a problem hiding this comment.
we have a pressedKeys() signal that you can use to see what keys are currently pressed.
| } else { | ||
| sendMessage(); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think these should just be a registered hotkeys?
There was a problem hiding this comment.
This is directly attached to an md area and it's a bit annoying to rewrite as a hotkey. I'd rather not change this.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Code reviewI found one issue that needs attention: Issue: Cmd/Ctrl+Enter shortcut broken on Windows and LinuxLocation: Problem: The The UI correctly displays "Ctrl + Enter" on non-Mac platforms via Evidence: The codebase has an established cross-platform pattern that should be used here. Examples:
Suggested fix: Change line 203 from: if (e.metaKey) {To: if (IS_MAC ? e.metaKey : e.ctrlKey) {This requires importing import { IS_MAC } from '@core/constant/isMac';✅ No CLAUDE.md violations found. Checked for adherence to coding standards and guidelines. |
| class="flex items-center gap-1.5" | ||
| classList={{ | ||
| 'text-accent': isMetaHeld(), | ||
| 'text-accent': pressedKeys().has("ctrl"), |
There was a problem hiding this comment.
i believe this should be "cmd"
No description provided.