From af501d2eb7fc261c5cc4815931ca6956d88f8ca0 Mon Sep 17 00:00:00 2001
From: LGUG2Z <LGUG2Z@fastmail.com>
Date: Sat, 31 Oct 2026 15:11:25 -0700
Subject: [PATCH] refactor(core): the floating-window hook is called
 before_floating_removed

Workspace::remove_floating_window calls the hook before it takes the
window out of the ring (the KDE and macOS originals move the focus to the
previous floating window first, and the focus index moves back by one
whatever the length), but the hook was named after_floating_removed and its
doc said after. Name and doc now say before; the behaviour is the one KDE
and macOS had. KDE's and macOS's implementations are renamed with it (the
only users; Windows keeps the no-op default). A stub test pins the order:
the hook sees both floating windows when one is removed.

Reviewer finding F2 (S/review/core.md). No behaviour change.

Verified: cargo test -p komorebi-core --features schemars (native): 30
passed, 0 failed; KDE cargo test -p komorebi-linux (lite env): 664
passed, 0 failed; Windows clippy --locked --all-targets -D warnings:
PASS; macOS fake-CC check: PASS. Not run: VM suites, Windows e2e.
---
 crates/komorebi-core/src/workspace.rs      | 31 +++++++++++++++++++---
 komorebi-for-kde/komorebi/src/workspace.rs |  2 +-
 komorebi-for-mac/komorebi/src/workspace.rs |  2 +-
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/crates/komorebi-core/src/workspace.rs b/crates/komorebi-core/src/workspace.rs
index a91a2eaa..6863d74b 100644
--- a/crates/komorebi-core/src/workspace.rs
+++ b/crates/komorebi-core/src/workspace.rs
@@ -150,9 +150,11 @@ pub trait WorkspacePlatform: Sized + Clone + std::fmt::Debug + PartialEq + 'stat
     /// Tells the scrolling layout that the container at `idx` is about to leave the workspace.
     fn note_scrolling_departure(workspace: &Workspace<Self>, idx: usize);
 
-    /// Called after a floating window was taken out of the workspace; KDE refocuses a floating
-    /// window, Windows does nothing.
-    fn after_floating_removed(_workspace: &mut Workspace<Self>) {}
+    /// Called by [`Workspace::remove_floating_window`] before the floating window is taken out of the
+    /// workspace, while it is still in the ring: KDE and macOS move the focus to the previous
+    /// floating window first (the focus index moves back by one whatever the length), Windows does
+    /// nothing.
+    fn before_floating_removed(_workspace: &mut Workspace<Self>) {}
 
     /// The index at which [`Workspace::promote_container`] inserts the container: the primary
     /// position of the layout. The default layouts have it first.
@@ -1161,7 +1163,7 @@ impl<P: WorkspacePlatform> Workspace<P> {
             None => None,
             Some(idx) => {
                 if self.floating_windows().get(idx).is_some() {
-                    P::after_floating_removed(self);
+                    P::before_floating_removed(self);
                     self.floating_windows_mut().remove(idx)
                 } else {
                     None
@@ -1985,6 +1987,13 @@ mod tests {
 
         fn note_scrolling_departure(_: &Workspace<Self>, _: usize) {}
 
+        fn before_floating_removed(workspace: &mut Workspace<Self>) {
+            LOG.with(|log| {
+                log.borrow_mut()
+                    .push(format!("floating {}", workspace.floating_windows().len()))
+            });
+        }
+
         fn begin_update(_: &mut Workspace<Self>) -> eyre::Result<Flow> {
             LOG.with(|log| log.borrow_mut().push("begin".to_string()));
             Ok(Flow::Run(None))
@@ -2089,6 +2098,20 @@ mod tests {
         assert_eq!(workspace.resize_dimensions.len(), 2);
     }
 
+    #[test]
+    fn the_hook_for_a_removed_floating_window_runs_before_the_removal() {
+        let mut workspace = Workspace::<Plat>::default();
+        workspace.floating_windows_mut().push_back(W(1));
+        workspace.floating_windows_mut().push_back(W(2));
+        log();
+
+        assert_eq!(workspace.remove_floating_window(2), Some(W(2)));
+
+        // the ring still had both windows when the hook ran: the focus moves first
+        assert_eq!(log(), ["floating 2"]);
+        assert_eq!(workspace.floating_windows().len(), 1);
+    }
+
     #[test]
     fn promoting_a_container_puts_it_first_and_focuses_it() {
         let mut workspace = workspace_of(&[1, 2, 3]);
diff --git a/komorebi-for-kde/komorebi/src/workspace.rs b/komorebi-for-kde/komorebi/src/workspace.rs
index 228fabd1..751aea0d 100644
--- a/komorebi-for-kde/komorebi/src/workspace.rs
+++ b/komorebi-for-kde/komorebi/src/workspace.rs
@@ -119,7 +119,7 @@ impl komorebi_core::WorkspacePlatform for KdePlatform {
         workspace.note_scrolling_departure(idx);
     }
 
-    fn after_floating_removed(workspace: &mut Workspace) {
+    fn before_floating_removed(workspace: &mut Workspace) {
         workspace.focus_previous_floating_window();
     }
 
diff --git a/komorebi-for-mac/komorebi/src/workspace.rs b/komorebi-for-mac/komorebi/src/workspace.rs
index 4845a177..bdacfb6c 100644
--- a/komorebi-for-mac/komorebi/src/workspace.rs
+++ b/komorebi-for-mac/komorebi/src/workspace.rs
@@ -143,7 +143,7 @@ impl komorebi_core::WorkspacePlatform for MacosPlatform {
         workspace.note_scrolling_departure(idx);
     }
 
-    fn after_floating_removed(workspace: &mut Workspace) {
+    fn before_floating_removed(workspace: &mut Workspace) {
         workspace.focus_previous_floating_window();
     }
 }