Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to maximize windows within the tiling flow #871

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

valpackett
Copy link
Collaborator

Not quite a fix, but does work as a workaround for #870, along with just being a desirable behavior for some users. (Like me xD)


Surprisingly it's all I've had to do, with zero special handling the unmaximize operation just worked as restoring to previous size, it worked with vertical tiling too, etc :)

paperwm-maxi-within.webm

(Windows flashing blank here is a screencast thing 0_o that wasn't visible on the actual screen!)


Unconditionally doing a relayout fixes the weirdly-cropped state described in #870, but I have no idea about the slightly-off-screen-and-not-clickable state that happens with more than one window open…

Not quite a fix, but does work as a workaround for #870, along with just
being a desired behavior for some users.
Copy link

github-actions bot commented Jun 5, 2024

Thanks for your contribution! We don't accept pull requests to the release branch. I have rebased your pull request onto develop, check for any conflicts.

@github-actions github-actions bot changed the base branch from release to develop June 5, 2024 22:27
@jtaala
Copy link
Collaborator

jtaala commented Jun 5, 2024

Thanks @valpackett!

Question - does this need to be an option? E.g. shouldn't this be enabled as normal behaviour?

@jtaala
Copy link
Collaborator

jtaala commented Jun 5, 2024

but I have no idea about the slightly-off-screen-and-not-clickable state that happens with more than one window open

I can have a look at this one.

@valpackett
Copy link
Collaborator Author

valpackett commented Jun 5, 2024

shouldn't this be enabled as normal behaviour?

I mean, I can definitely imagine people wanting to have gaps/margins for non-maximized windows, but maximized windows being actually fully maximized beyond that?

But if you like this as well, we can change the default to true :)

@jtaala
Copy link
Collaborator

jtaala commented Jun 5, 2024

shouldn't this be enabled as normal behaviour?

I mean, I can definitely imagine people wanting to have gaps/margins for non-maximized windows, but maximized windows being actually fully maximized beyond that?

But if you like this as well, we can change the default to true :)

Sorry, I may have not expressed that correctly. I'll have a look at this PR tonight. What I meant was though, this appears to add/enable super+f type maximising width when double-clicking the titlebar or clicking a window's maximise button (if it has one). That sounds like something we should just enable (i.e. doesn't need an option to turn that on).

I may be misunderstanding something though. Will check out out tonight.

@jtaala
Copy link
Collaborator

jtaala commented Jun 5, 2024

ah, I see, it's changes the maximise window behaviour - gotcha! Misunderstood that one.

Okay, I like that behaviour too (max-width in tiling rather than maxmising the window).

Will give it a test. I'm inclined to make that behaviour true by default as it fits better with PaperWM's tiling concepts (and doesn't seem so out of place as opposed to maximising).

@jtaala
Copy link
Collaborator

jtaala commented Jun 6, 2024

Hey @valpackett - made a few small changes (mainly setting this to true by default).

Have a last look/test and let me know. Otherwise, happy with this. Will merge in after that.

@valpackett
Copy link
Collaborator Author

LGTM of course

@jtaala jtaala merged commit 971916a into develop Jun 7, 2024
@jtaala jtaala deleted the maximize-within branch June 7, 2024 06:34
@jtaala
Copy link
Collaborator

jtaala commented Jun 7, 2024

Hey @valpackett - noticed an issue with some windows where they don't get restored to original width (happens for me in code). Likely to do with the fact that I set the Maximised horizontal width to 97%. Anyways, just looking into that now.

@jtaala
Copy link
Collaborator

jtaala commented Jun 7, 2024

Likely to do with the fact that I set the Maximised horizontal width to 97%. Anyways, just looking into that now.

Ah, no, that wasn't it. Anyways, will fix that one before releasing these changes.

@jtaala
Copy link
Collaborator

jtaala commented Jun 8, 2024

Okay, that's the problem, we're calling an unmaximize within the signal that catches the event. Which means, that we'll be triggering toggleMaximizeHorizontally twice. For some windows there looks to be a race condition, so the second one breaks the method to restore the original frame.

@valpackett
Copy link
Collaborator Author

hm. I'm honestly not sure how restore even ended up working :)

@jtaala
Copy link
Collaborator

jtaala commented Jun 8, 2024

hm. I'm honestly not sure how restore even ended up working :)

Ah, I implemented that a while ago. We save the window frame dimensions when maximising, for restore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants