double-tapping space to open/close the node graph instead of Ctrl+Space#3786
double-tapping space to open/close the node graph instead of Ctrl+Space#3786jsjgdh wants to merge 12 commits into
Conversation
Summary of ChangesHello @jsjgdh, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a DoubleTap input event and maps it to toggling the node graph overlay, replacing the old Ctrl+Space shortcut. The implementation correctly adds the new event type throughout the input mapping system.
However, I've identified a functional issue in the double-tap detection logic within input_preprocessor_message_handler.rs. The current implementation causes both taps of a double tap to also be processed as single taps, leading to unintended side effects. I've provided a suggestion to correct this behavior for the second tap. I've also noted that a more complete fix to prevent the first tap from firing immediately would require a more significant change, which could be addressed in a follow-up.
Additionally, I've made a minor suggestion to improve code clarity regarding a constant name.
|
!build (build link) |
|
|
!build (Run ID 22263581839) |
|
|
This needs to trigger upon the second tap's release, not upon its depress. |
|
!build (Run ID 22272302375) |
|
|
!build (Run ID 22273785584) |
|
|
!build (Run ID 22296196398) |
|
|
!build (Run ID 22324976474) |
|
|
!build (Run ID 22353351358) |
|
Keavon
force-pushed
the
master
branch
6 times, most recently
from
d6228da to
e58c1de
Compare
Keavon
force-pushed
the
master
branch
3 times, most recently
from
05c5187 to
fcc53f5
Compare
Keavon
force-pushed
the
master
branch
2 times, most recently
from
9b97ab7 to
2e842cb
Compare
Keavon
force-pushed
the
master
branch
2 times, most recently
from
f07c79b to
76938eb
Compare
Keavon
force-pushed
the
master
branch
2 times, most recently
from
4b7a823 to
847b8e9
Compare
timon-schelling
force-pushed
the
master
branch
2 times, most recently
from
15fcaac to
d5f0140
Compare
|
@jsjgdh I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 8 files
Confidence score: 4/5
- This PR appears safe to merge: the only flagged item is a PR title formatting/policy issue, not a functional defect in runtime behavior.
- The reported issue has moderate stated severity but is process-oriented (sentence case/imperative mood enforcement), so user-facing regression risk is low.
- Pay close attention to
editor/src/messages/input_preprocessor/input_preprocessor_message_handler.rs- verify the title-enforcement check aligns with repository contribution rules and won’t cause unnecessary workflow friction.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/messages/input_preprocessor/input_preprocessor_message_handler.rs">
<violation number="1" location="editor/src/messages/input_preprocessor/input_preprocessor_message_handler.rs:2">
P2: Custom agent: **PR title enforcement**
PR title violates sentence case and imperative mood requirements</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -1,4 +1,5 @@ | |||
| use crate::application::Editor; | |||
There was a problem hiding this comment.
P2: Custom agent: PR title enforcement
PR title violates sentence case and imperative mood requirements
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/input_preprocessor/input_preprocessor_message_handler.rs, line 2:
<comment>PR title violates sentence case and imperative mood requirements</comment>
<file context>
@@ -1,4 +1,5 @@
use crate::application::Editor;
+use crate::consts::DOUBLE_CLICK_MILLISECONDS;
use crate::messages::input_mapper::utility_types::input_keyboard::{Key, KeyStates, ModifierKeys};
use crate::messages::input_mapper::utility_types::input_mouse::{MouseButton, MouseKeys, MouseState};
</file context>
2 participants
discussion : https://discord.com/channels/@me/1461994742291234997/1473606245364662417 from DM with @Keavon