fix(UI): stuck keys#2123
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a const query for hotkey-capture state and updates Menu::ProcessInputEventQueue to selectively suppress key-down forwarding (while still forwarding key-up), allow Enter/Escape during first-time setup, and clear ImGui input keys when toggling the menu to avoid stuck toggle keys. Changes
Sequence Diagram(s)sequenceDiagram
participant User
rect rgba(200,220,255,0.5)
participant InputQueue
end
participant Menu
participant ImGuiIO as ImGui::IO
User->>InputQueue: KeyDown/KeyUp events
InputQueue->>Menu: Deliver event
alt Menu is capturing hotkey input
Menu->>Menu: IsCapturingHotkeyInput() == true
alt Event is KeyDown and not Enter/Escape (setup-close)
Menu-->>ImGuiIO: swallow (do not forward)
else KeyUp or Enter/Escape
Menu->>ImGuiIO: forward event
end
else Not capturing
Menu->>ImGuiIO: forward all events (KeyDown/KeyUp)
end
Note right of Menu: On toggle-open -> Menu calls ImGui::GetIO().ClearInputKeys()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Menu.cpp`:
- Around line 1093-1097: The printable character path still calls
io.AddInputCharacter() unconditionally during hotkey capture; update the logic
so fingerprint characters are suppressed using the same gate that blocks key
button events (the condition using isHotkey, event.IsPressed(),
wasCapturingHotkey, allowSetupCloseKey). Concretely, wrap or gate the
io.AddInputCharacter() call (and any related character-forwarding code) behind
that same boolean expression (or compute a local forwardKeyboard variable from
isHotkey/event.IsPressed()/wasCapturingHotkey/allowSetupCloseKey) so printable
keys do not reach ImGui while capturing a hotkey except when allowSetupCloseKey
permits it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3534812c-fff2-4c46-a5d7-71254ef0294b
📒 Files selected for processing (2)
src/Menu.cppsrc/Menu.h
|
✅ A pre-release build is available for this PR: |
|
Please confirm this doesn't break combo keys. Like shift f10. Once someone confirms this works we'll merge. Thanks! |
|
just tested for that, works |
|
I also tested and seems to work on stuck key fix. |
|
The problems are no longer reproducible, great job! |
(cherry picked from commit 60bd93b)
fixes multiple cases of stuck keys:
Closes: #2099
Summary by CodeRabbit
Bug Fixes