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

Added the ability to cycle between monitors #163

Merged
merged 2 commits into from Feb 19, 2021
Merged

Added the ability to cycle between monitors #163

merged 2 commits into from Feb 19, 2021

Conversation

OldKros
Copy link
Contributor

@OldKros OldKros commented Dec 22, 2020

I found the swapping between monitors with dedicated keybinds for each monitor rather annoying and instead wanted one keybind to go to the next monitor (or another to go to the previous) however that ability was not implemented. It was for the workspaces but not for monitors which I thought was a bit weird, so here we are.

Copy link
Contributor

@N1x0 N1x0 left a comment

Choose a reason for hiding this comment

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

Hi @OldKros, nice PR!

I'm doing some testing at the moment and it seems to work well and quite frankly I feel like this could not only extend the current way of handling monitor interaction but actually replace in part, at least for focusing monitors. For moving windows to other monitors I believe it makes sense to retain the original approach as well. Just my two cents, it depends on what @rickbutton thinks in the end.

What's missing is adding these actions to the KeybindManager as default subscriptions. E.g.

            Subscribe(mod | KeyModifiers.LShift, Keys.Right
                () => _context.Workspaces.SwitchFocusToNextMonitor(), "Focus next monitor");

            Subscribe(mod | KeyModifiers.LShift, Keys.Left
                () => _context.Workspaces.SwitchFocusToPreviousMonitor(), "Focus previous monitor");

            Subscribe(mod | KeyModifiers.Control, Keys.Right
                () => _context.Workspaces.MoveFocusedWindowToNextMonitor(), "move focused window to next monitor");

            Subscribe(mod | KeyModifiers.Control, Keys.Left
                () => _context.Workspaces.MoveFocusedWindowToPreviousMonitor(), "move focused window to previous monitor");

Potentially some more minor cleanups but other than that a great addition IMO!

@josteink
Copy link
Member

Yeah this looks good to me to.

Sorry about taking so long time getting this reviewed. Consider it merged!

@josteink josteink merged commit 0f56fcf into workspacer:master Feb 19, 2021
dalyIsaac added a commit to dalyIsaac/workspacer that referenced this pull request Feb 19, 2021
Added the ability to cycle between monitors (workspacer#163)
dlundgaard pushed a commit to dlundgaard/workspacer that referenced this pull request Feb 21, 2021
…t with those recently defined in workspacer#163

added `int GetWorkspaceIndex(IWorkspace workspace)` to the `IWorkspaceContainer` as suggested by @dalyIsaac
@OldKros OldKros deleted the cyclemonitors branch February 24, 2021 16:07
@dalyIsaac dalyIsaac added this to the 0.9.11 milestone Jun 29, 2021
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.

None yet

4 participants