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

layout/monitor: Add next monitor to focus/move to #872

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

julianschuler
Copy link

First of all, thank your for this awesome project!

This PR adds the option to focus/move to the next monitor.
It is mainly intended for cycling between multiple monitors using a single key and wraps around to the first monitor after the last one is reached.

Currently, the order follows the order of outputs given by Smithay, please let me know if a different order is desired instead.
Also let me know if I should add a similar option to focus/move to the previous monitor.

Copy link
Owner

@YaLTeR YaLTeR left a comment

Choose a reason for hiding this comment

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

Thanks, looks reasonable.

Currently, the order follows the order of outputs given by Smithay, please let me know if a different order is desired instead.

That order is nondeterministic based on how the outputs appear or are plugged in. I think it's better to sort outputs by name like elsewhere in the code to get a stable order every time.

let outputs: Vec<_> = self.global_space.outputs().collect();
outputs.sort_unstable_by(|a, b| a.name.compare(&b.name));

Also let me know if I should add a similar option to focus/move to the previous monitor.

Yeah, let's do that.

@julianschuler
Copy link
Author

I now have added saving the output order in reposition_outputs(). As far as I've seen, this is always called after any change to the underlying outputs of global_space. I also added the options to focus/move to the previous monitor.

Please let me know if I should implement any further changes!

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