Skip to content

Conversation

@glupi-borna
Copy link
Collaborator

I added four new actions that swap windows, but move them across monitor boundaries if the focused window is the first/last window on the current monitor. These actions are supposed to complement the switch-global-* actions, and so the behavior is intended to be similar.

The implementation is simple: I used swap(...) when the window is not at the "edge" of the monitor, and switchMonitor(...) otherwise.

There are some odd edge cases that I'd like to solve though, but I need a bit of guidance:

  1. Sometimes (specifically, when moving from my left monitor to my right monitor), the window will move into the second spot on the right monitor instead of the first.
  2. When moving windows within one monitor (via swap), if the window is in a column, the whole column will move. However, when moving across monitors, because switchMonitor only moves the focused window, the rest of the stack is left behind on the first monitor.

I'm sure that just calling switchMonitor is simply not the right thing to do here, and I should probably write a function that moves the whole column to the other monitor, but I'm not sure where to start with that. Should I simply move all of the windows in the column to the other monitor, and then call slurp each of them in the right order? Are there other things I should be mindful of?

@glupi-borna glupi-borna added the enhancement Adds a new feature or extends scope label Oct 27, 2024
@glupi-borna
Copy link
Collaborator Author

I just experienced some glitchy behavior where windows were offset from their intended spot. I'm not sure what I did to cause it, but I think registering the new keybinds via registerAction is not the right thing to do -- keybinds that call out to swap(...) (specifically, the "move-*" keybinds) seem to be registered via registerMinimapAction. Should I just use registerMinimapAction then?

@Lythenas
Copy link
Collaborator

The differences here are relatively subtle. Without trying it out I can't say which one is the correct one to use. Or if you maybe even need a new one with slightly different options. The register*Actions just call registerAction with different options.

Regarding your other question about switchMonitor and column, you very likely need a new function. But you can probably make use of some of the existing functions.

Also regarding the position of the window when you switch monitor. I believe the window will end up next to the focused window on that monitor. You probably want to move it to the end or start of the window list, depending on which direction the window comes from. I think there are existing functions for this, but not 100% if they can be used here. Maybe you need to add a new parameter to switchMonitor that can configure where the window is inserted.

@Lythenas Lythenas changed the base branch from release to develop October 28, 2024 15:56
Added four new actions that swap windows, but move them across monitor
boundaries if the focused window is the first/last window on the current
monitor. These actions are supposed to complement the switch-global-*
actions, and so the behavior is intended to be similar.
@glupi-borna
Copy link
Collaborator Author

Alright, I think I'm on the right path now.

I ended up passing the PER_WINDOW flag to mutter when registering the action - I'm still not 100% sure that this is the right thing to do, but I think it's better than what I had before. I tried to figure out how the opensNavigator and opensMinimap flags work, and I think I don't want that, but I could be totally wrong in my understanding.

I went ahead and added two new functions to the Space class - moveToStart and moveToEnd. They both take a window and move it to the leftmost (start) or rightmost (end) position in the space. moveGlobal calls out to these functions after calling switchMonitor when moving LEFT or RIGHT. It didn't make sense to me to do something like that when moving UP/DOWN (or rather, moving the window to be next to the currently focused one seems to be the right thing to do in this case).

Since moveToStart and moveToEnd have to be called on the space we just moved the window to, I made switchMonitor return the space it switches to.

Also, disregard what I said earlier about glitchy behaviour. I was being dumb and installed DashToPanel instead of DashToDock, and it was causing all sorts of problems. I switched to DashToDock and everything seems fine for now.

The last thing I still have to do is moving the entire column of windows when switching monitors instead of just the focused window. I think I'll try to keep it simple and just reconstruct the column on the new monitor. I'm trying to think of a way to do this without creating another switchMonitor function, and without complicating the code of switchMonitor further.

@jtaala
Copy link
Collaborator

jtaala commented Nov 5, 2024

Nice work @glupi-borna, I'll try look at some of this.

As for moving entire columns, the closet implementation we have is take/drop functionality:

https://github.com/paperwm/PaperWM/blob/release/tiling.js#L5485-L5501

We're we have an array of metaWindows and change their space, and then call insertWindow with various options (if the openWindowPosition is set to a column mode, then they will be put into a single column).

Moving a column (as is) to another space/monitor sounds like good functionality to add for sure.

Copy link
Member

@smichel17 smichel17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wow. I guess I should have looked at open PRs before I implemented nearly same thing in #1082. Your approach to return the new space from switchMonitor is much cleaner than what I hacked together. Although I prefer the index handling in my PR.

I'll play around with this tomorrow.

@smichel17
Copy link
Member

smichel17 commented Sep 27, 2025

The animation handling here is nicer! I added the index handling (quick & dirty) in smichel17@eebe516 and that works too.

I think we'll also want to add minimap handling; the regular "move left" and "move right" shortcuts show the minimap (if you don't immediately release Super) and I think that makes sense. Especially when the window you're using is "paperwm fullscreen" (horizontally maximized), it's hard to tell where in the space you are without the minimap.

Once we get those in place I'm happy to merge.

I ended up passing the PER_WINDOW flag to mutter when registering the action

The mutter flags are kinda weird, they don't seem to actually do much in the source. Like, as far as I can tell the main (only?) effect of PER_WINDOW (vs NONE) is that it will only trigger if there is a window selected. Which is what we want here, so PER_WINDOW seems correct.

@glupi-borna
Copy link
Collaborator Author

Oh man, I totally forgot about this! Do you wanna create a new PR from smichel17@eebe516, maybe? If not, I'm gonna be swamped with work for the rest of the week, but I can pull your changes in and try to get the minimap working this weekend, if that's how you wanna do it. Let me know!

@smichel17
Copy link
Member

I'm also going to be busy this week. How about you run 27e787e (from #1082) for the week and see if you agree with the UX, and then, assuming you agree with it, one of us can get the minimap working on this PR over the weekend?

@glupi-borna
Copy link
Collaborator Author

Sounds like a plan!

@glupi-borna
Copy link
Collaborator Author

I ran it for a while, and the only issue I've encountered is the one where, when moving to a different monitor via the move-global commands, the focused window is popped out of it's column, instead of the entire column moving all at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Adds a new feature or extends scope

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants