Desktop: Fix double-clicking top bar not toggling maximize in linux#3869
Desktop: Fix double-clicking top bar not toggling maximize in linux#3869he1senbrg wants to merge 1 commit intoGraphiteEditor:masterfrom
Conversation
Summary of ChangesHello, 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 resolves an issue preventing users on Linux from maximizing or restoring application windows by double-clicking the title bar. It introduces a custom event handling logic to accurately detect double-click gestures, ensuring consistent window management behavior across different desktop environments and closing issue #3594. 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
|
There was a problem hiding this comment.
Code Review
This pull request fixes an issue where double-clicking the top bar did not toggle maximization on Linux by implementing a manual double-click handler. The approach is sound. I've left a couple of comments with suggestions for improvement: one to fix a minor bug where a maximized window could be dragged, and another to improve code readability by removing a magic number.
| if (isFullscreen) return; | ||
|
|
||
| const now = Date.now(); | ||
| if (now - lastMouseDown < 500) { |
There was a problem hiding this comment.
Dear Sir or Madame Gemini Code Assist Bot,
Double click timing can be adjusted by the operating system for accessibility for people with reduced motor function.
The browser will automatically deal with this and report a double click event when appropriate; it can be bound using on:dblclick.
I would expect any fairly competent LLM to take this into account. I hope that will improve your code assistant capabilities as they are currently quite lacking.
Yours faithfully,
Hypercube
There was a problem hiding this comment.
@0HyperCube What should I do, I believe the desktop app uses a fixed interval for detecting multi click across the app. This doesn't appear to follow OS settings either. Should I just use this approach only on linux and let other platforms use on:dblclick?
desktop/src/cef/consts.rs
pub(crate) const MULTICLICK_TIMEOUT: Duration = Duration::from_millis(500);| editor.handle.appWindowMaximize(); | ||
| } else { | ||
| lastMouseDown = now; | ||
| editor.handle.appWindowDrag(); |
|
This needs to be implemented in desktop code. dblclick event should be fired by cef. we need to figure out why it's not doing that on some linux systems. could you check what happens with different double click intervals configured on you system? can't reproduce the issue on any of my Linux systems. |
Before
maximize_before.mp4
After
maximize_after.mp4
Closes #3594