Shift box zooms to box instead of box selection

[ugurdogrusoz]

I cannot box select using Shift drag as I used to. It zooms into that area now.

[canbax]

I observed this 1-2 times but couldn't reproduce. Maybe it occurs on Mac OS more frequently.

[ugurdogrusoz]

It does happen on Windows as well. Also it is not only box selection but Shift + tap (extend selection) works like zoom as well after a while.

[canbax]

I find one way (I hope it is the only way) to reproduce this issue. When you press the control button (CTRL) and the shift button the key down event handler will be triggered continuously. If you release one of the buttons or both, the key up handler should be called.

But if you press the TAB button at the same time when you are pressing, the key-up handler is never gets called even after you release the buttons.

The TAB button will change the focus on some other HTML element. So probably due to that, key-up handlers are never get called.

https://github.com/iVis-at-Bilkent/cytoscape.js-view-utilities/commit/c3ee6a389b10cc11d7b8e7fc2a0f5de74e679509 checks if the key down handler is being called continuously. If it stops being called, after 1 second it checks if the key-up handler is called. If it is not called, it calls the key-up manually.

Also, changing cursor functionality is moved to cytoscape.js-view-utilities.

[ugurdogrusoz]

@msalihaltun Please review

[msalihaltun]

This is a weird one. If I release a button quickly enough that the timeout doesn't occur it still happens.

I have definitely seen the bug occur a few times, yet I don't recall ever using tab at the same time. But I think you must be right in that one of ctrlKeyDown or shiftKeyDown stays as true after the keys are released. So in a way, your fix should resolve the issue (without always being related to tab, maybe there are some other reasons why keyup is not called).

Also, while possibly unrelated, maybe we should handle additional keys pressed by calling disableMarqueeZoom() when a non Shift or Ctrl key is pressed (i.e keydown is called, key is not ctrl or shift, ctrlKeyDown and shiftKeyDown are both true). I think it makes sense that if keys other than Ctrl and Shift are also pressed, zoom shouldn't happen. Maybe this messes things up even further though?

[canbax]

In theory, if you release a button quickly, still the key-up event handler should be called. So before the timeout, the key-up event handler should be called manually.

Here my logic is this: If key-up is not called even if key-down is not being called for 1 second, it should have been called. So I'm calling it manually. In theory, this should work in any case.

Calling disableMarqueeZoom() on keypresses other than ctrl and shift might make things more clear but it might be risky. I think we should clearly reproduce the issue then try to fix it. Otherwise guessing and trying might introduce unexpected new bugs.

You can try printing logs on keydown and keyup, you can try changing timeout duration.

[canbax]

We observed a buggy case.

If a user presses CTRL+SHIFT+TAB and then drag using the mouse before the timeout event defined in the extension occurs, this bug happens.

We updated the extension and set the timeout delay to 750 ms. This delay cannot be shorter than 500 ms because the key down delay is like 500 ms. (If you press a button the key-down event will be fired. If the hold pressing it following key-down events will be fired after a delay of 500 ms)

I think this buggy case is very unlikely to occur. Because the user should press 3 buttons and then should drag very quickly. Other than that we couldn't find a buggy case. We will continue experimenting.