Skip to content

fix(wm): notify subscribers on floating window focus change (update the focus index of floating windows) - #1488

Draft
JustForFun88 wants to merge 2 commits into
LGUG2Z:masterfrom
JustForFun88:fix_floating_windows_focus
Draft

fix(wm): notify subscribers on floating window focus change (update the focus index of floating windows)#1488
JustForFun88 wants to merge 2 commits into
LGUG2Z:masterfrom
JustForFun88:fix_floating_windows_focus

Conversation

@JustForFun88

Copy link
Copy Markdown
Contributor

Issue discovered during testing #1465:

When more than two floating windows are present, switching focus between floating windows does not update the Komorebi bar status - meaning the bar keeps showing the previously focused floating window as active, instead of updating to the new focused window.

Root cause:

We did not notify subscribers about floating window focus changes, because the current focused floating window index in the workspace was not updated on focus change, so has_been_modified didn鈥檛 detect a state change.

The PR adds code to update the focused floating window in the current workspace before notifying subscribers. As a result, notifications about floating window focus changes are now correctly sent to all subscribers, and the Komorebi bar updates as expected.

How to reproduce (before fix):

  • Open 2+ floating windows.
  • Switch focus between them.
  • Notice that the Komorebi bar does not reflect the focus change.
bug.mp4

How to test (after fix):

  • Repeat the steps above: the Komorebi bar now updates when focus changes between floating windows.

@JustForFun88

This comment was marked as resolved.

Some(idx) => {
if let Some(_window) = workspace.floating_windows().get(idx) {
workspace.set_layer(WorkspaceLayer::Floating);
workspace.floating_windows.focus(idx);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@LGUG2Z I鈥檓 a little bit confused. Since we are working with floating windows, this line of code should be enough as it is. However, in the following scenario there is a problem:

  • There are three tiling windows.
  • I make one of them floating.
  • I hover over one of the remaining tiling windows, select it, and also make it floating. It now appears on top of the first floating window.
  • I drag this second floating window slightly so the first floating window is visible underneath, and then select that first floating window.
  • At this point the call of workspace.floating_windows.focus(idx) should update the focused-window index, but it seems the state isn鈥檛 refreshed, because notify_subscribers does not send a message.
  • If I then select the second floating window again, everything works as expected and notify_subscribers fires. After that, switching focus between the two floating windows continues to work normally.
example.mp4

However, if I add the following piece of code to update_focused_workspace function, everything works from the start:

let fl_windows = &mut self.focused_workspace_mut()?.floating_windows;
if let Some(idx) = fl_windows.elements().iter().position(|w| w.is_focused()) {
    fl_windows.focus(idx);
}

I don't understand why this happens. Do you have any idea?

Comment on lines +1459 to +1463
let fl_windows = &mut self.focused_workspace_mut()?.floating_windows;
if let Some(idx) = fl_windows.elements().iter().position(|w| w.is_focused()) {
fl_windows.focus(idx);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This alone is not sufficient. It does not update the indices when the workspace is empty, so it should be used together with workspace.floating_windows.focus(idx) in the WindowManagerEvent::FocusChange(_, window) handler. However, if you remove this code and leave only workspace.floating_windows.focus(idx) in WindowManagerEvent::FocusChange(_, window), it won鈥檛 work immediately after making two windows floating one after another (and I don't know why).

@LGUG2Z

LGUG2Z commented Mar 22, 2026

Copy link
Copy Markdown
Owner

I would like to revisit this and understand if this is still an issue on v0.1.40 that needs addressing; I'll convert this to a draft until we can make a determination and resolve any conflicts

@LGUG2Z
LGUG2Z marked this pull request as draft March 22, 2026 17:08
@LGUG2Z
LGUG2Z force-pushed the master branch 2 times, most recently from d99b4b5 to a3b2e28 Compare May 5, 2026 02:09
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