Repository navigation
Change the node graph view shortcut from Ctrl+Space to Space+Space - #3786
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) |
|
d6228da to
e58c1de
Compare
05c5187 to
fcc53f5
Compare
f07c79b to
76938eb
Compare
4b7a823 to
847b8e9
Compare
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>
eca5ba8 to
aec8f79
Compare
There was a problem hiding this comment.
1 existing issue remains and no new issues found across 8 files
Confidence score: 5/5
- The PR title starts with lowercase gerund text, which doesn't meet the required sentence case and imperative mood. Rename it to an imperative title, such as
Make double-tapping space open….
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
A first tap that arrived with a modifier or mouse button held still armed the detector, so pressing Shift and Space for live preview and then tapping Space promptly toggled the node graph. A press that cannot take part in a double tap no longer seeds one and breaks a tap already waiting, and a test pins that case.
|
!build (Run ID 38034031982) |
Keavon
changed the title
Wasm: 40.14 MB — JS: 0.69 MB — CSS: 0.12 MB — Fonts: 0.69 MB — Images: 0.09 MB — All Assets: 41.73 MB |
2 participants
discussion : https://discord.com/channels/@me/1461994742291234997/1473606245364662417 from DM with @Keavon