Allow windows to be moved and resized with the cursor#179964
Allow windows to be moved and resized with the cursor#179964robert-ancell wants to merge 9 commits into
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
|
See #179859 for related PR to disable window decorations. |
There was a problem hiding this comment.
Code Review
This pull request introduces a new API to allow moving and resizing windows with the cursor, with a full implementation for Linux and plumbing for Windows and macOS. My review focuses on improving the example code's correctness, enhancing the documentation for the new API, removing code duplication in the Linux implementation, and ensuring consistency in the macOS implementation. Overall, the changes are a good step towards the new feature.
| Widget build(BuildContext context) { | ||
| return Listener( | ||
| onPointerDown: (PointerDownEvent event) { | ||
| onStart(0); |
There was a problem hiding this comment.
The button parameter is hardcoded to 0. This is likely incorrect for some platforms like Linux (GTK), which expect a button index (e.g., 1 for the left button). The PointerDownEvent provides a buttons property (which is a bitmask) that could be used to determine the correct button. Using event.buttons would pass 1 for a primary mouse button click, which is what GTK expects for the left button.
| onStart(0); | |
| onStart(event.buttons); |
There was a problem hiding this comment.
It's not clear the best way to handle the button pressed - event.buttons contains a bitmask of the buttons that are down, we probably just want the one button. So this is hardcoded for now.
| /// Start moving this window. | ||
| /// FIXME | ||
| @internal | ||
| void beginMoveDrag(int button); | ||
|
|
||
| /// Start resizing this window. | ||
| /// FIXME | ||
| @internal | ||
| void beginResizeDrag(int button, WindowDragEdge edge); |
There was a problem hiding this comment.
The documentation for beginMoveDrag and beginResizeDrag is just "FIXME". According to the style guide, documentation should be useful and explain the "why" and "how". Please add proper documentation for these methods, explaining their purpose, parameters (especially what button represents, e.g., a bitmask or an index), and current implementation status across platforms.
References
- The style guide requires documentation to be useful, explaining the 'why' and the 'how' (line 55). It also states that all public members should have documentation (line 53), and that public-quality documentation should be used even for private members (line 62). (link)
There was a problem hiding this comment.
Documentation to be added...
| /// Start moving this window. | ||
| /// FIXME | ||
| @internal | ||
| void beginMoveDrag(int button); | ||
|
|
||
| /// Start resizing this window. | ||
| /// FIXME | ||
| @internal | ||
| void beginResizeDrag(int button, WindowDragEdge edge); |
There was a problem hiding this comment.
The documentation for beginMoveDrag and beginResizeDrag is just "FIXME". According to the style guide, documentation should be useful and explain the "why" and "how". Please add proper documentation for these methods, explaining their purpose, parameters (especially what button represents, e.g., a bitmask or an index), and current implementation status across platforms.
References
- The style guide requires documentation to be useful, explaining the 'why' and the 'how' (line 55). It also states that all public members should have documentation (line 53), and that public-quality documentation should be used even for private members (line 62). (link)
There was a problem hiding this comment.
Documentation to be added...
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
I don't think this abstraction will map well on macOS. We can implement the dragging part ( On windows you can somewhat emulate this with |
|
TBF, even |
|
Greetings from stale PR triage! 👋 |
|
We can do synchronous hittest from native. This means we can have a "WindowDraggable" widget in hierarchy and do a hit test to determine if said part of window should be draggable. This allows for proper NC_HITTEST implementation on windows, but more importantly also have |
|
Still on radar but will withdraw this as the method required will be different as noted by @knopp. |
Add new API to allow windows to be moved and resized while a button is held down. This will be useful when drawing window decorations using Flutter. This feature is implemented for Linux which has native APIs for this. Some plumbing is implemented for Windows and MacOS, however based on the GTK source these systems do not have native API and this will have to be implemented by intercepting the move events and moving the window to match them.