From 968af7902ed20bbf47a10417edcc4c9be4b57272 Mon Sep 17 00:00:00 2001 From: Stephen Michel Date: Sat, 20 Sep 2025 13:08:33 -0400 Subject: [PATCH 1/5] Braces around if No braces around if statements make me nervous. --- tiling.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tiling.js b/tiling.js index 65fdf13b9..43c238398 100644 --- a/tiling.js +++ b/tiling.js @@ -1208,8 +1208,9 @@ export class Space extends Array { let dir = index < 0 ? Meta.DisplayDirection.LEFT : Meta.DisplayDirection.RIGHT; let i = display.get_monitor_neighbor_index(monitor.index, dir); - if (i === -1) + if (i === -1) { return; + } let newMonitor = Main.layoutManager.monitors[i]; space = spaces.monitors.get(newMonitor); @@ -1241,8 +1242,9 @@ export class Space extends Array { let dir = row < 0 ? Meta.DisplayDirection.UP : Meta.DisplayDirection.DOWN; let i = display.get_monitor_neighbor_index(monitor.index, dir); - if (i === -1) + if (i === -1) { return; + } let newMonitor = Main.layoutManager.monitors[i]; space = spaces.monitors.get(newMonitor); From 7ba866a81fc2e69ae43439780a06c3417e4705de Mon Sep 17 00:00:00 2001 From: Stephen Michel Date: Sat, 20 Sep 2025 13:20:42 -0400 Subject: [PATCH 2/5] More readable index bounds calculations --- tiling.js | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/tiling.js b/tiling.js index 43c238398..3eca7f06a 100644 --- a/tiling.js +++ b/tiling.js @@ -828,17 +828,18 @@ export class Space extends Array { // Space.prototype.isVisible = function isVisible(metaWindow, margin = 0) { - let clone = metaWindow.clone; - let x = clone.x + this.cloneContainer.x; - let workArea = this.workArea(); - let min = workArea.x; + const clone = metaWindow.clone; + const left = clone.x + this.cloneContainer.x; + const right = left + clone.width; - if (x - margin + clone.width < min || - x + margin > min + workArea.width) { - return false; - } else { - return true; - } + const workArea = this.workArea(); + const areaLeft = workArea.x; + const areaRight = areaLeft + workArea.width; + + const isOffscreenLeft = right < areaLeft + margin; + const isOffscreenRight = left > areaRight - margin; + + return !isOffscreenLeft && !isOffscreenRight; } isFullyVisible(metaWindow) { @@ -1219,16 +1220,14 @@ export class Space extends Array { } else { index = 0; } - if (space[index].length <= row) - row = space[index].length - 1; + row = Math.min(row, space[index].length - 1) space.activate(false, false); Navigator.finishNavigation(); Navigator.getNavigator().showMinimap(space); } let column = space[index]; - if (column.length <= row) - row = column.length - 1; + row = Math.min(row, column.length - 1) switch (direction) { case Meta.MotionDirection.UP: @@ -1248,8 +1247,7 @@ export class Space extends Array { let newMonitor = Main.layoutManager.monitors[i]; space = spaces.monitors.get(newMonitor); - if (space.length <= index) - index = space.length - 1; + index = Math.min(index, space.length - 1) if (dir === Meta.DisplayDirection.UP) { row = space[index].length - 1; } else { From 0dc5b3d72194162eb16654236a75919194107af3 Mon Sep 17 00:00:00 2001 From: Stephen Michel Date: Tue, 23 Sep 2025 20:06:03 -0400 Subject: [PATCH 3/5] More straightforward direction mappings We have two direction enums, which basically mean the same thing. Previously we were figuring out which monitor we need to go to depending on which way we were going out of bounds, but that's unnecessary, since we already know which direction we're heading. If this mapping is needed elsewhere I'll move it into a util or something. --- tiling.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tiling.js b/tiling.js index 3eca7f06a..cbcda13c9 100644 --- a/tiling.js +++ b/tiling.js @@ -1190,6 +1190,14 @@ export class Space extends Array { switchGlobalUp() { this.switchGlobal(Meta.MotionDirection.UP); } switchGlobalDown() { this.switchGlobal(Meta.MotionDirection.DOWN); } switchGlobal(direction) { + const motionToDisplayDirection = { + [Meta.MotionDirection.LEFT]: Meta.DisplayDirection.LEFT, + [Meta.MotionDirection.RIGHT]: Meta.DisplayDirection.RIGHT, + [Meta.MotionDirection.UP]: Meta.DisplayDirection.UP, + [Meta.MotionDirection.DOWN]: Meta.DisplayDirection.DOWN, + }; + const dir = motionToDisplayDirection[direction] + let space = this; let index = space.selectedIndex(); if (index === -1) { @@ -1206,8 +1214,6 @@ export class Space extends Array { } if (index < 0 || index >= space.length) { let monitor = focusMonitor(); - let dir = index < 0 - ? Meta.DisplayDirection.LEFT : Meta.DisplayDirection.RIGHT; let i = display.get_monitor_neighbor_index(monitor.index, dir); if (i === -1) { return; @@ -1238,8 +1244,6 @@ export class Space extends Array { } if (row < 0 || row >= column.length) { let monitor = focusMonitor(); - let dir = row < 0 - ? Meta.DisplayDirection.UP : Meta.DisplayDirection.DOWN; let i = display.get_monitor_neighbor_index(monitor.index, dir); if (i === -1) { return; From f6f09fd68d15a634c7ffc5ec985873a2f45d82b0 Mon Sep 17 00:00:00 2001 From: Stephen Michel Date: Mon, 22 Sep 2025 13:34:06 -0400 Subject: [PATCH 4/5] Fix bug with multiple boundary transitions --- tiling.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tiling.js b/tiling.js index cbcda13c9..27a1270dd 100644 --- a/tiling.js +++ b/tiling.js @@ -1219,6 +1219,10 @@ export class Space extends Array { return; } + // Ensure if we change workspaces and then monitors, + // that the new workspace stays active on the starting monitor. + space.activateWithFocus(space.selectedWindow, false, true); + let newMonitor = Main.layoutManager.monitors[i]; space = spaces.monitors.get(newMonitor); if (dir === Meta.DisplayDirection.LEFT) { From a16a6e4780e438d6da04c47dac7a452f790e9be5 Mon Sep 17 00:00:00 2001 From: Stephen Michel Date: Sat, 20 Sep 2025 13:07:12 -0400 Subject: [PATCH 5/5] Focus the closest *visible* window In the diagrams below, assume parens are workspaces, brackets are windows currently showing on a monitor, and letters are windows. Uppercase means focused, and assume all windows are 50% sized. If we start like this: ([A b] c d) ([e f]) Then I use the mouse to move to the monitor to the right: ([a b] c d) ([E, f]) Then I use this keybind to move left again. I'd get whiplash as the monitor on the left spins to move windows c and d back onscreen. (a b [c D]) ([u, f]) Now, instead it'll just pick the closest window on screen: ([a B] c d) ([e, f]) Which makes sense visually, since in this situation I'm thinking "I want to move left" and what I see is this transition: [a b][E f] [a B][e f] It might make sense to add some margin here in case window c is just barely peeking a few pixels on screen (in which case we might still prefer to focus window b). Also, some small code cleanup: using more `const` It's hard for me to keep track of what variables mean, when we are re-assigning them. So, create a new variable to track the new space, and make everything const so we know it's not being updated. _Maybe_ re-using a variable saves a few bytes of memory, or a little bit of extra garbage collection time, but this function is only being run once per user input, so I doubt it's a significant performance impact. --- tiling.js | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/tiling.js b/tiling.js index 27a1270dd..5b3aff71a 100644 --- a/tiling.js +++ b/tiling.js @@ -1213,8 +1213,8 @@ export class Space extends Array { index--; } if (index < 0 || index >= space.length) { - let monitor = focusMonitor(); - let i = display.get_monitor_neighbor_index(monitor.index, dir); + const monitor = focusMonitor(); + const i = display.get_monitor_neighbor_index(monitor.index, dir); if (i === -1) { return; } @@ -1223,17 +1223,24 @@ export class Space extends Array { // that the new workspace stays active on the starting monitor. space.activateWithFocus(space.selectedWindow, false, true); - let newMonitor = Main.layoutManager.monitors[i]; - space = spaces.monitors.get(newMonitor); - if (dir === Meta.DisplayDirection.LEFT) { - index = space.length - 1; - } else { - index = 0; + const newMonitor = Main.layoutManager.monitors[i]; + const newSpace = spaces.monitors.get(newMonitor); + const visibleColumns = newSpace.filter((column) => newSpace.isVisible(column[0])); + if (visibleColumns.length === 0) { + return; } - row = Math.min(row, space[index].length - 1) - space.activate(false, false); + + const newColumn = dir === Meta.DisplayDirection.LEFT + ? visibleColumns[visibleColumns.length - 1] + : visibleColumns[0]; + const newRow = Math.min(row, newColumn.length - 1); + const newWindow = newColumn[newRow]; + + newSpace.activate(false, false); Navigator.finishNavigation(); - Navigator.getNavigator().showMinimap(space); + Navigator.getNavigator().showMinimap(newSpace); + ensureViewport(newWindow, newSpace); + return } let column = space[index]; @@ -1264,6 +1271,7 @@ export class Space extends Array { space.activate(false, false); Navigator.finishNavigation(); Navigator.getNavigator().showMinimap(space); + return } let metaWindow = space.getWindow(index, row);