search

miromannino / miro-windows-manager

Intuitive and clever mechanism for moving windows using only arrows, even resizing windows by thirds or quarters! For OSX
378 stars 39 forks source link

Use `hs.hotkey.modal` for modals, a bugfix, and some code cleanup #9

Open matthewfallshaw opened 5 years ago

matthewfallshaw commented 5 years ago

Bugfix:

Refactoring:

Change:

@miromannino , Thank you again for your stellar work on this Spoon. I like your keyboard interface for Move & Resize Modes much more than I liked the one I created in my last pull request.

If you don't like the addition of jumping to full width/height after tapping opposite directions in Move & Resize Modes, remove ; growFullyModals[move]:enter() from line 564, ; growFullyModals[mapR[resize]]:enter() from line 583, then remove lines 584, 580 and 565.

miromannino commented 5 years ago

Hello Matt! I checked this but with the modal the window can't be moved unless you repeatedly press the arrows. Also I noticed in your changed the window move and resize are bound to just arrows, but people use also WASD or HJKL as arrows.

I'll review changes and bugfix, but I don't think I can merge this as is

matthewfallshaw commented 5 years ago

Those are both excellent points, and I think I've just fixed them both. I can now tap or hold hjkl as my move keys for both move mode and resize mode. Sorry for the oversight.

rjhilgefort commented 5 years ago

@matthewfallshaw Maybe I installed your branch incorrectly, but I'm not seeing the move and resize keys behaving. I'm holding hyper+v and then using my "arrow" keys, but it's behaving as if I'm not holding v at all- it just acts like I'm holding hyper and whatever arrow key and moves to that corner/side. I think it's more likely that I haven't correctly picked up your code, but I don't have time to troubleshoot at the moment. I'll play around next chance I get, but I just wanted to let you know in case it's not my error. Here's my config:

local hyper = {"ctrl", "alt", "cmd"}
local hyperShift = {"ctrl", "alt", "cmd", "shift"}

hs.loadSpoon("MiroWindowsManager")

hs.window.animationDuration = 0.1
spoon.MiroWindowsManager:bindHotkeys({
    up         = {hyper, "k"},
    right      = {hyper, "l"},
    down       = {hyper, "j"},
    left       = {hyper, "h"},
    fullscreen = {hyper, "return"},
    center     = {hyper, "c"},
    move       = {hyper, "v"},
    resize     = {hyper, "d"}
})
rjhilgefort commented 5 years ago

... aaaaannndd I hadn't pulled since switching my submodule over to your fork. Sorry for the noise on this thread. I've tested your changes and they work great for me! This seems to solve the issue I was seeing in the previous PR!

rjhilgefort commented 5 years ago

@miromannino I'm still on this PR/fork for the functionality in it. I can't speak to the Lua code, but the functionality would be a great addition. Please consider merging or at least helping @matthewfallshaw make changes that you would like to see. Thanks!

miromannino commented 5 years ago

Ok I'll review this soon , the last time there were bugs and I never had a chance to see @matthewfallshaw fixes, sorry for that! Yes I would like to merge if everything is stable. For now @rjhilgefort please tell me if you find any bug so we can address it before merging to main

rjhilgefort commented 5 years ago

@miromannino Thank you!! This Spoon you've created is one of my favorite utilities on my Mac and I greatly appreciate the time you spend on it. Sorry to be a bother!

I've been running this fork for something like 2 months now and I haven't seen anything to mention. I'll continue to keep an eye out.

miromannino commented 5 years ago

I know I actually cannot think to work without it. :) Ok thank you for the feedback, I'll merge soon! Thank you again @matthewfallshaw for the new features and ideas!

jacoscaz commented 4 years ago

@miromannino this really seems worth merging in. Doing so would also provide something akin to what #15 is about, removing the need to merge that one.

jacoscaz commented 4 years ago

I'm currently on this branch and everything seems smooth as butter, at least from a cursory look.

miromannino commented 4 years ago

Perfect! I am glad to know! I will then definitely merge! I just need to review @matthewfallshaw code before submitting. Thanks again @matthewfallshaw again for changes!

jacoscaz commented 3 years ago

@miromannino here I am for my yearly "thank you" message for your window manager and my personal +1 for this PR (which I've been using for one year now without encountering any glitch or issue). Do you still plan on merging this?