Monocle Layout "minimize unfocused windows" fix and improvements

[bladedvox]

Summary

Fixes Monocle Layout with Minimize Unfocused Windows (internally config.monocleMinimizeRest) option enabled.

Restores previous functionality (before Wayland patches) and adds

• minimize inactive windows only on same monitor with multimonitor setup
• can switch windows with focus-changing Actions
• switches to the next window when closing the active window (still a little buggy in multimonitor, but a big improvment)
• handles moving window onto a Surface with this layout properly

Breaking Changes

None

UI Changes

Removed "(WIP)" from "Minimize unfocused windows" option in the config UI.

Test Plan

1. Reload script
2. Verify all behavior in the Summary

Related Issues

Closes #43, #55

[gikari]

Could you also change your commit messages to respect CC?

[bladedvox]

@gikari I've got the original functionality restored. It's kind of hacky but the only way I could find to do it. Basically I added a hook to controller.onWindowFocused() to do the minimization under the correct layout (and if the setting is enabled). I also had controller.showNotification() raise the event as well so that it works properly when first changing to the Monocle layout.

How do you want to handle the finer points?

• Should we enable this by default?
• Should we make an exception for the focus cycle (or add a separate Action) that includes minimized windows in the queue when switching focus?

[gikari]

Should we enable this by default?

Not yet. If it works well - we probably should in the future.

Should we make an exception for the focus cycle (or add a separate Action) that includes minimized windows in the queue when switching focus?

We have FocusNextWindow and FocusPreviousWindow. Those should work with this option on as in before. As for Focus Left, Right and so on, I am not sure. How do those work in classic tilings WMs with monocle layout (It seems like Monocle is the terminology, that comes from dwm)?

[bladedvox]

I will address the comments you raised today -- as for the keyboard shortcuts, it's been a long time since I used DWM, but looking at the source code now it seems that it doesn't implement minimization at all. I currently like the behavior we have of cycling only through visible windows with the FocusNext/PrevWindow actions. I use a keybind for a task switcher that cycles through minimized windows only if I need to quickly get access to one, and is how I currently switch focus by keyboard in Monocle mode with the monocleMinimizeRest option enabled.

I think, just to avoid needing a separate keyboard shortcut, we should either
a. include minimized windows for consideration in FoucsNext/Prev
b. add an optional argument to engine.focusOrder() that is used in MonocleLayout (when this option is enabled) when overriding FocusNext/Prev/Bottom/Upper/Left/Right that includes minimized windows for consideration

I lean towards b since I like the current focus switching model.

[gikari]

it's been a long time since I used DWM, but looking at the source code now it seems that it doesn't implement minimization at all.

I meant if the switching between windows in monocle mode works with Next/Previous window shortcuts. If it is, we should do the same, but we can provide an option to exclude minimized windows from the list.

[bladedvox]

Using this branch on my machine and so far so good. Here's some notes on the behavior:

Assuming monocle layout and config.monocleMinimizeRest == true:

• You can switch windows with the WindowFocusNext/Prev/Right/Left/Upper/Bottom Actions
• Closing the active window switches to the next window
• Floating windows on the DriverSurface are handled properly
• Moving a tiled window into the workspace/screen results in it being the only visible window

There is still an issue with using this layout in a multi-monitor setup -- if you close the active window in Monocle layout in your secondary monitor, it switches to a window in the primary monitor instead of the same monitor that the closed window was in. I Haven't really figured out why this is happening. I have some code that tries to fix it but it isn't really working. Will update with progress on that. Other than that minor issue, I am comfortable with the experience so far.

[bladedvox]

@gikari I wasn't able to get focus to change to the same monitor as the closed window when closing the active window in Moncole mode with monocleMinimizeRest == true in a multimonitor setup, but other than that it's a smooth experience and I'm pretty satisfied. I think we should wrap this PR up and try to address that bug later. Let me know if any architectural issues still need to be addressed :)

[bladedvox]

I see the failing tests. Edited the action.test locally and will rebase/reword to shorten my commit messages.

[gikari]

I see the failing tests. Edited the action.test locally and will rebase/reword to shorten my commit messages.

Could you also please update the MR description according to the new template please?

[bladedvox]

PR description updated, hope it's all up to standard :)

[gikari]

Awesome! 🎉 I just tested, and it works very well (except after closing a window in multimonitor it focuses the one, on the other monitor, so after merging you could create an issue for this strange behavior). Just a little small nitpicks (including in-code comments):

I think you can remove WIP it the UI. It still has the multimonitor problem, but I think it works good to be considered WIP feature, and in general marking something WIP in the UI lies not a good impression on the end user.

Also, I apologize for some pasts of CI not working as intended right now. So, could you please format your code with prettier?

And you need to rebase your branch on the current master.

[bladedvox]

Addressed the issues you mentioned, got the husky hooks set up, verified with ESLint (apparently if you have a default parameter in a function, you don't need to declare its type, good to know) and formatted with Prettier. Rebuilt and restarted KWin for good measure. Everything seems good on my end!

[gikari]

Hm, I noticed another thing - if I switch to another layout, the windows, that were minimized by the script are not brought back. I think this, can be addressed in another PR. For now, it is good.

[gikari]

You don't have to duplicate JSDoc here.