search

agzam / spacehammer

Hammerspoon config inspired by Spacemacs
MIT License
556 stars 70 forks source link

Modal: Catch rapid keypresses faster #144

Closed jaidetree closed 2 years ago

jaidetree commented 2 years ago

@agzam discovered if you press the bindings to activate the modal too fast, it causes the modal to flicker and perform oddly.

Reproduction

  1. Press cmd+space or opt+space multiple times as fast as possible
  2. Try opening the window modal, then pressing space to return to the main modal

Expected

The main modal closes, the window modal opens

Actual

The main modal closes, opens (appears to flicker), then the window modal opens

Grazfather commented 2 years ago

effect-handler properly enforces ordering (cleanup called before a new transition).

What should happen when activate-modal is called when the modal is already up?

** Warning:statemachi: In send for action activate from state idle ; Hit prefix
** Warning:statemachi: Calling effect handler
** Warning: modal.fnl: Showing modal
** Warning:statemachi: In send for action activate from state active ; Hit prefix again, already active, this calls active->submenu. This probably isn't correct.
** Warning:statemachi: Calling cleanup
** Warning: modal.fnl: Closing modal
** Warning:statemachi: Calling effect handler
** Warning: modal.fnl: Showing modal
** Warning:statemachi: In send for action activate from state submenu ; Hit W
** Warning:statemachi: Calling cleanup
** Warning: modal.fnl: Closing modal
** Warning:statemachi: Calling effect handler
** Warning: modal.fnl: Showing modal
** Warning:statemachi: In send for action start-timeout from state submenu ; Called from bind-item when Space was hit ** Warning: modal.fnl: Timeout
** Warning:statemachi: Calling cleanup
** Warning: modal.fnl: Closing modal
** Warning:statemachi: Calling effect handler
** Warning: modal.fnl: Showing modal
** Warning:statemachi: In send for action deactivate from state submenu ; Timeout
** Warning:statemachi: Calling cleanup
** Warning: modal.fnl: Closing modal
** Warning:statemachi: Calling effect handler

So I think we have two problems:

  1. activate when the modal is up and state is active re-shows the modal but also transitions state to submenu when it shouldn't. It should probably recognize it's opening the top modal and return the old state and not fire an effect in that case.
  2. add-timeout-transition is called when the submenu is opened. This seems to happen as part of the effect handler:

show-modal-menu -> bind-menu-keys -> bind-item -> select-trigger -> create-action-trigger -> start-modal-timeout -> (send :start-timeout).

Notice that the window menu doesn't timeout until we get rid of the 'first' one. I have to think about the semantics here to get this working as expected.

Grazfather commented 2 years ago

I can repro this without going fast:

  1. Hit prefix. Notice the main menu with app-specific items
  2. Hit it again. Notice the app-specific stuff is gone
  3. Hit w. Window menu
  4. Hit space. Window menu flickers.

Is this the same issue, or should we file another ticket?

jaidetree commented 2 years ago

Was able to reproduce, seems related