Add the ability to unmaximize a window by dragging. #172
Conversation
internal/x11/win/frame.go
Outdated
| if moveDeltaX == 0 && moveDeltaY == 0 { | ||
| return | ||
| } | ||
| if f.client.Maximized() { | ||
| if uint16(moveDeltaX) > x11.TitleHeight(x11.XWin(f.client)) || |
There was a problem hiding this comment.
Might it be clearer if we add a new variable that avoids this call twice? Might help show intent too.
dragThreshold := x11.TitleHeight(x11.XWin(f.client))There was a problem hiding this comment.
With the new changes it's only one threshold now, vertical. Per @Jacalz horizontal drag shouldn't unmaximize. That will change we do quarter snapping though I'm sure.
There was a problem hiding this comment.
There is now a magic "*3" so I think the comment still applies with regard to intent.
Looking at the code it is also not clear why the y position needs to be checked twice.
There was a problem hiding this comment.
Semantically we don't want to start counting the delta until the cursor has been dragged out of the titlebar vertically. So that's the first check. The second check is to calculate the resistance at which we want to unmaximize. I understand currently I'm using titlebar height * 3 to calculate that distance - and thus the if could be shortened to just check a delta of titlebar height * 4 and get the same effect - I figured they were two different things and we may not always use titlebar height to calculate the delta resistance. I can make a const for the resistance and then I think this if check will make sense if the y position of the cursor > titlebar position && the delta of how far the mouse has been dragged > resistance const then unmaximize.
There was a problem hiding this comment.
Thanks for the explanation, if the first check is to ensure it is within the bar then you’re missing a check for y >= f.y.
I don’t think a const makes sense, as the title height is a variable.
However the same border theme lookup is being done twice in this line, so is it not more efficient to capture in a var?
Fixes #138