Propose approach to handle keyboard events in core dragging code

[cpcallen]

Figure out what we need to do to plumb keyboard events through existing dragging code in core (which all uses pointer events), and create a design doc or tracking issue for this work.

• Figure out if this entails breaking changes.
  • If so, figure out timings re: v13.

[cpcallen]

Notes about the existing keyboard move system, as it has evolved since its creation in #352, written mostly as an exercise to force myself to carefully read and understand the existing code before I propose changes to it.

KeyboardDragStrategy

• constructor: receives and saves:

  • block: BlockSvg
  • moveType: MoveType (which is Insert or Move)
    • Not used for anything in this class!
  • startPoint: RenderedConnection | null (not coordinates for workspace blocks; that's stored onMover's MoveInfo object).

• Overrides startDrag:

  • Saves list of candidate connections in geographic order, top-to-bottom, left-to-right. This is not correct; should be sorted per topBlocks then in pre-order traversal within stacks.
  • Does a this.block.moveDuringDrag(this.startLoc) (where this.startLoc has just been set to this.block.getRelativeToSurfaceXY()
    • How does this have any useful effect??
  • Ensures connection candidate for just-disconnected connection is shown (except in case of Incorrect initial candidate connection when moving top block of stack #728).
  • Displays move indicator icon + bubble.

• Overrides drag:

  • Requires event parameter, or does an early return—won't even do super call.
  • Fetches this.currentDragDirection from e.tiltX and .tiltY.
  • If an unconstrained move found a connection candidate, it updates search start location for next constrained move.
  • If a constrained move found a connection candidate, it moves block a bit to offset it from that candidate.
  • If a move did not find a connection candidate:
    • Reset search start location to null.
    • If doing a constrained move, show constrained move hint.

• Overrides endDrag:

  • Deletes candidate connection list. (Is this needed?)
  • Removes move icon.

• Overrides currCandidateIsBetter to always favour new candidate when doing constrained move.

• Overrides GetConnectionCandidate to (via .getConstrainedConnectionCandidate which calls findTraversalCandidate) choose next connection from list saved by constructor when doing a constrained move.

• Overrides shouldHealStack to always return true if block has a previous connection (even if unconnected)—i.e., will always heal stacks (but not rows of value blocks).

  • Actual healing is done by BlockDragStrategy.prototype.startDrag.

Mover

• canMove checks if navigation state is STATE.WORKSPACE and the navigation can edit the worspace, and no move is currently in progress on that workspace—and that the draggable is movable.

• startMove:

  • Patches drag strategy if draggable is a block, or adds a move bubble if it's a workspace comment.
  • Creates a Dragger (or registered custom alternative).
  • Informs workspace that a keyboard move is in progress.
  • Calls dragger.onDragStart.
  • Refocuses block being moved
    • How might it have lost focus? Is this a bug in focus manager?
  • Sets up a listener for blur events, that will terminate move if focus is lost.
    • Maybe this should be part of the action, rather than the mover itself?
      • Though note that there are two different actions that can initiate a move!
  • Sets up new shortcut, bound to all keys used by existing shortcuts except for the move shortcut itself, that will finish (not abort) the move.
    • Surely this should definitely be part of the move action??

• finishMove:

  • Does pre-drag-end cleanup (see below).
  • Calls dragger.onDragEnd.
  • Does post-drag-end cleanup (see below).

• abortMove:

  • Does pre-drag-end cleanup (see below).
  • patches dragger so that .onDragEnd will either move block back to original position or delete it, depending on whether move was initiated my m shortcut or by inserting new block.
    • For some reason the flag that controls this is on the KeyboardDragStrategy instance, even though it doesn't use it for anything!
  • Saves candidate connection neighbour in case this is an insert and we'll need to put focus somewhere sensible if it is aborted.
    • Maybe putting focus back on the toolbox would be better?
  • Sets drag strategy connection candidate to null to prevent block from connecting (so delete will only delete that one block
    • Does this correctly handle inserting multiple non-shadow blocks at the same time?
  • Calls dragger.onDragEnd.
  • If this was an insert, move focus to saved connection candidate neighbour.
    • This causes Aborting insert puts focus on normally-unreachable locations #736.
  • Does post-drag-end cleanup (see below).

• preDragEndCleanup:

  • Unregisters shortcut bound to most other shortcut keys.
  • Clears move hints (those shown by KeyboardDragStrategy.prototype.drag for constrained moves with no candidates).
    • Why does KeyboardDragStrategy not do this itself??
  • Removes blur listener added by startMove.

• postDragEndCleanup:

  • Removes and disposes the move indicator icon + bubble.
    • That might have been added either by startMove or by KeyboardDragStrategy.prototype.startDrag.
  • Unpatches draggable's drag strategy.
  • Informs workspace that keyboard move is no longer in progress.
  • Sets timeout to scroll focused element into view.
  • Sets focus on draggable.
    • Apparently this is to ensure it retains focus if it gets reattached? Not sure why this would be needed.

• moveConstrained:

  • If draggable is a comment, do an unconstrained move instead.
  • Call dragger.onDrag with fake PointerEvent that encodes up/down/left/right direction as .tiltX/.tiltY.
  • Updates mover's own .totalDelta by checking actual location of draggable (either using .relativeCoords for BlockSvgs or .getBoundingRectangle() for other draggables), since KeyboardDragStrategy will have have moved block / comment.
  • Scrolls current element into view.

• moveUnconstrained:

  • Computes delta from direction and updates totalDelta.
  • Calls dragger.onDrag with fake pointer event and totalDelta.
  • Scrolls current element into view.
  • Updates location of move indicator bubble.
    • Why?? We do this for constrained moves, and comment and mutator bubbles seem to get moved automatically when dragging.
    • Upon further testing I am pretty sure this is not needed: the MoveIcon updates the position of the bubble automatically when it moves.

[cpcallen]

OK, I am making a proposal that we handle keyboard events by completely revising the IDraggable and IDragStrategy APIs; see #739 for details.

[cpcallen]

Marking this as on hold as we need to see if the proposed approach in #739 is agreed.

If so this will definitely be a (substantial) breaking change to the custom dragging APIs, so will end up in v13 or later.