Auto-scroll prevents many blocks from being moved to drag targets near edge of visible worksapce

[cpcallen]

Check for duplicates

• I have searched for similar issues before opening a new one.

Description

The intention of using the dragger to implement keyboard moves was partially to take advantage of the existing machinery for e.g. showing insertion markers for candidate connections, and partially to ensure that a user moving blocks using the keyboard would be able to do all the same things a user using the mouse would, including dragging blocks to drag targets such as the trash or backpack, or using drag-to-duplicate procedure parameters.

Unfortunately, #451 has broken the ability to use unconstrained movement to move a block to drag targets near the edge of the visible workspace (e.g., in to the trash), for all but the smallest blocks.

Reproduction steps

1. On the test page, click on the "draw circle" block.
2. Press m.
3. Holding down the unconstrained drag modifier (⌥ or control), use the arrow keys to move the block to the trash.

Observe that, at most zoom levels, it is impossible to delete the block because the workspace scrolls before the top left corner of the block reaches the trash.

Compare with:

1. Drag a MATH_NUMBER block from the toolbox onto the workspace.
2. Press m
3. Holding down the unconstrained drag modifier (⌥ or control), use the arrow keys to move the block to the trash.
4. Once the trash can lid opens, press enter to delete the block.

Observe that it is (correctly) possible to delete small blocks.

[cpcallen]

Needs discussion.

[RoboErikG]

Opinion: I think this is working as intended. Deleting things doesn't seem like a primary use case for unconstrained moves and the delete/backspace key is pretty standard.

[cpcallen]

@RoboErikG: I agree for the trash in particular this isn't important, but I filed the bug because the issue affects potentially all drag targets near the edges of the visible workspace. I've updated the title and description to make this more clear.

There are in my view two main ways we could go here:

1. Make changes to allow blocks to be moved on to any visible drag target, while still maintaining reasonable autoscroll behaviour.
   • Part of the issue is that the "hotspot" that is used to decide when a block is on a drag target when doing a keyboard move is the upper left corner of the block—rather than the mouse pointer location—while autoscroll wil try to keep the whole block visible. Moving the hotspot to the centre of the block (or group of blocks), and having autoscroll happen only when the hotspot is not on the visible portion of the workspace would solve the problem.
   • An alternative approach would be to check for any intersection between the blocks being moved and the drag target, though this could make it difficult to avoid deleting large (groups of) blocks when on a small screen, and would be much more aggressive than the current mouse-drag drag target behaviour.
2. Decide that drag targets will not be supported by keyboard move at all.
   • This would allow us to entirely replace, for keyboard move purposes, the existing dragger with something slightly more streamlined and tailored to keyboard move use, and thereby clean up some of the kludgery going on in the Mover class. If there is still enough common code between the Dragger and hypothetical new KeyboadDragger classes it could be moved to a common subclass.

I am broadly in favour of option 1 for the same reason that I was happy with the original decision to use Dragger for mouse moves: that it provides maximal parity between the keyboard and mouse experience.

[RoboErikG]

Ah, that makes more sense. In that case, what do you think of adding the drag targets to the list of viable constrained move locations if their shouldPreventMove() method returns false? We'd still need to prevent the scrolling behavior when moving to those, but that would be a clear case.

My thoughts on the other two ideas:

1. Moving the hotspot or checking for overlap would be a bigger rewrite and would be hard to avoid problems with the way connections are checked. This could have a lot of corner cases where it's hard to predict if you'll be on a block connection or the trash.

2. I think this would be reasonable and we could advise developers to have a key for each dress target to achieve parity with mouse. I think whether we move keyboard away from mover is a separate question.

[cpcallen]

what do you think of adding the drag targets to the list of viable constrained move locations if their shouldPreventMove() method returns false? We'd still need to prevent the scrolling behavior when moving to those, but that would be a clear case.

That seems reasonable, though the particular criteria probably need to be considered carefully—e.g., the workspace-backpack plugin returns true from shouldPreventMove when the thing being dragged is a block (which is the useful case).