Skip to content
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

workspace-management: Implement move_to_workspace requested #238

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Dec 6, 2023

Needed for cosmic-workspaces.

Requires pop-os/cosmic-protocols#21. This dependency will need to be updated here when that is merged.

@ids1024 ids1024 requested a review from a team December 6, 2023 23:19
src/shell/mod.rs Outdated Show resolved Hide resolved
@ids1024
Copy link
Member Author

ids1024 commented Dec 12, 2023

Updated. I had to make the seat optional Shell::move_window, like my earlier version, since there's no seat associated with the operation here. Which should be fine.

The output argument to the protocol is ignored, since there is only one output per workspace, even with workspace_mode:global. The argument still makes sense to have in the protocol since cosmic-workspace-unstable-v1 allows multiple outputs per workspace, even if the cosmic-comp implementation doesn't currently do that.

Seems to be working fine, with workspace_mode set to global or output bound.

@Drakulix
Copy link
Member

Drakulix commented Dec 12, 2023

Updated. I had to make the seat optional Shell::move_window, like my earlier version, since there's no seat associated with the operation here. Which should be fine.

This will obviously cause the pointer/focus to not swap over to the new output/newly mapped window. Given we only really support one seat and the last active one probably is the one that initiated this action to be send by our toplevel-management client, we could also use state.common.last_active_seat. Or is switching the focus away from cosmic-workspaces in this case actually not desirable?

Maybe we should extend the toplevel-management request to take a nullable seat?
Or better add an explicit request to focus a given toplevel on a given seat later?

The output argument to the protocol is ignored, since there is only one output per workspace, even with workspace_mode:global. The argument still makes sense to have in the protocol since cosmic-workspace-unstable-v1 allows multiple outputs per workspace, even if the cosmic-comp implementation doesn't currently do that.

Sounds reasonable. 👍

@ids1024
Copy link
Member Author

ids1024 commented Dec 12, 2023

Or is switching the focus away from cosmic-workspaces in this case actually not desirable?

I'm using this for drag-and-drop of a window to a workspace in cosmic-workspaces. As currently implemented it makes sense not for this to focus the surface.

Having the option to focus the surface is useful, but using another request for that is reasonable enough.

@Drakulix
Copy link
Member

I'm using this for drag-and-drop of a window to a workspace in cosmic-workspaces. As currently implemented it makes sense not for this to focus the surface.

Right, then there is a reason to take an Option here, agreed.

Having the option to focus the surface is useful, but using another request for that is reasonable enough.

Good. Then we can do this later and that doesn't have to concern this PR.

@Drakulix Drakulix merged commit 6569965 into master_jammy Dec 12, 2023
1 check passed
@ids1024 ids1024 deleted the move-to-workspace branch December 12, 2023 20:43
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