-
Notifications
You must be signed in to change notification settings - Fork 155
shell: Updates to focus stack handling #1772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
`IcedElement` uses `Arc` internally (and compares with `Arc::ptr_eq`). So these structs are never cloned, and probably shouldn't be.
This seems like the correct way to use an `IndexSet`. It shouldn't be possible to have multiple entries that match, since it's a "set". We can't define `Borrow<CosmicMapped> for FocusTarget`, so the blanket impl of `indexmap::Equivalent` won't work, but implementing seems fine.
`Action::Close` already used the keyboard focus target, but some other bindings didn't. Presumably it's most intuitive if all "current window" key bindings affect the window with keyboard focus. These used the focus stack on the `focused_output()` (the one with keyboard focus), so I guess the main impact is when the keyboard target is a window being dragged? Then the binding will operate on that window, or have no effect. This seems related to some of the behaviors discussed in #453.
Drakulix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good so far
Replacement for #1687, that works correctly with multiple outputs. We don't want another window to show a focus indicator while a window is being dragged, so keep the window in the focus stack. If a window is being moved out of a stack, change the focus from the stack to the window. `refresh_focus_stack()` doesn't seem to be called here, but for good measure, make sure that calling that function also won't remove a `CosmicMapped` from the focus stack if it is currently part of a move grab for the seat.
8553f79 to
e16f876
Compare
|
I think the focus indicators are working well now, in all cases. |
|
This seems to fix almost everything. The only thing I noticed that still occurs is the trailing dynamic workspace disappearing if the only window on the next-to-last workspace is grabbed, which doesn't seem like it should be intended behavior (should only potentially change on drop). |
Drakulix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Yeah, that might make more sense. Though it's also not useful to move a window from a (non-pinned) workspace to the dynamic workspace after it, since that will just auto-remove one of the workspaces regardless. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a floating workspace, the N-1 focused window previously got an active hint along with the dragged window when dragging a window. Now, the dragged window keeps its active hint (and background windows do not get hints for the duration of the drag).
Confirmed this fixes the following issues from #453:
- When focusing Firefox and then a COSMIC app, then dragging the COSMIC app, Super-M or Super-Q operate on the un-focused Firefox instead of the dragged COSMIC app.
- Super-Q now closes the app being dragged, and Super-M doesn't do anything while dragging a window.
- The app tray shows the N-1 focused app as active instead of the window being dragged.
- The N-1 focused app is no longer shown as active, but the dragged app still has its active highlight disappear, which doesn't totally make sense-- even if I start typing (which interrupts the drag), the active highlight doesn't come back until I release the mouse button to drop the window. It's still an improvement, though.
I noticed that if I Super-Q to quit an app while dragging it, if it's pinned to the dock as a favorite, then the dock keeps a dot on it as if it has a window open. That's not a regression (edit: found it already filed at #1146).
Aside from all of that, this seems to be working as expected with both floating and tiled workspaces, as well as with floating windows on tiled workspaces, including with workspaces on multiple displays.
Replaces #1687, and includes #1698.
Unlike the previous solution, this doesn't have issues on multiple outputs, but I'm having trouble getting the behavior right when moving a window out of a stack...