feat: introduce Mover actions + experimental DragMover implementation
#352
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduce the Mover class, with shortcuts that will put a workspace into (and exit from) moving mode. While in moving mode, cursor navigation via the cursor keys is disabled.
Create an experimental implementation of unconstrained dragging using the existing Dragger class. Implement this on a new Mover subclass DragMover, to keep this experimental code (that will probably be thrown away) separate from the main Mover implementation. Known bugs: * Blocks lurch around somewhat wildly while being moved. * Moving top-level blocks works reasonably well, but attempting to move a non-top-level block will result in it being deleted unless the move is aborted.
Alt+arrows causes browser navigation on Windows. :-(
There was a bug that would often cause blocks to be deleted. This was because the dragger thought the block was on the toolbox, which is a delete area. Fix this by giving the dragger much better fake PointerEvents, so that it knows where the block actually is - good enough that you can move the block to the trash can and it will be deleted!
rachel-fenichel
requested review from
RoboErikG and
gonfunko
and removed request for
a team and
RoboErikG
Collaborator
Author
gonfunko
approved these changes
Mar 31, 2025
| cursor.setCurNode(ASTNode.createBlockNode(block)); | ||
|
|
||
| // Additional implementation goes here. | ||
| console.log('startMove'); |
Contributor
There was a problem hiding this comment.
Remove? Applies throughout these next few methods.
Collaborator
Author
There was a problem hiding this comment.
I filed #357 to track removing all of these if we don't need them. I think the point was to support multiple types of Movers in case we want to compare two different types of behaviour, but I'm not sure if we'll need to do that.
@cpcallen to confirm.
The same applies for finishMove and abortMove, which ar ethe same here but distinct in the drag mover.
Comment on lines
+259
to
+268
| abortMove(workspace: WorkspaceSvg) { | ||
| const info = this.moves.get(workspace); | ||
| if (!info) throw new Error('no move info for workspace'); | ||
|
|
||
| // Additional implementation goes here. | ||
| console.log('abortMove'); | ||
|
|
||
| this.moves.delete(workspace); | ||
| return true; | ||
| } |
Contributor
There was a problem hiding this comment.
This appears identical to finishMove(); perhaps rename to finishOrAbort(), or have this just call finishMove()?
This was referenced Mar 31, 2025
microbit-grace
pushed a commit
to microbit-matt-hillsdon/blockly-keyboard-experimentation
that referenced
this pull request
Apr 2, 2025
…tion (RaspberryPiFoundation#352) * feat: Introduce Mover class, moving mode Introduce the Mover class, with shortcuts that will put a workspace into (and exit from) moving mode. While in moving mode, cursor navigation via the cursor keys is disabled. * feat: Save block starting location and connections * feat: Experiment: unconstrained movement using Dragger Create an experimental implementation of unconstrained dragging using the existing Dragger class. Implement this on a new Mover subclass DragMover, to keep this experimental code (that will probably be thrown away) separate from the main Mover implementation. Known bugs: * Blocks lurch around somewhat wildly while being moved. * Moving top-level blocks works reasonably well, but attempting to move a non-top-level block will result in it being deleted unless the move is aborted. * feat: Add Move Block (M) context menu item * fix: Allow ctrl+arrows to also perform unconstrained moves Alt+arrows causes browser navigation on Windows. :-( * fix: Use better fake PointerEvents to prevent random deletions There was a bug that would often cause blocks to be deleted. This was because the dragger thought the block was on the toolbox, which is a delete area. Fix this by giving the dragger much better fake PointerEvents, so that it knows where the block actually is - good enough that you can move the block to the trash can and it will be deleted! * chore: address feedback from RaspberryPiFoundation#317 * chore: lint and format * fix: remove allowCollision for m * fix: RaspberryPiFoundation#356 by hiding the connection preview before reverting a drag * fix: spelling --------- Co-authored-by: Christopher Allen <cpcallen+git@google.com> feat: heuristic insert (RaspberryPiFoundation#355) * chore: simplify tryToConnectNodes We only insert blocks so remove other cases. * feat: heurstic insertion logic - use first statement or value input - in statement inputs move to the last next connection in the chain - use next connections over previous connections chore: convert webdriverio tests to TypeScript (RaspberryPiFoundation#353) * chore: convert webdriverio tests to TypeScript * chore: comment typo fix feat: allow for toolbox with multiple tab stops (RaspberryPiFoundation#358) - Switch to focusin/out for toolbox (maybe in future elsewhere) - Drop the tab index fiddling as it's not practical to get it right in the face of more than one tab stop in the toolbox, especially if that toolbox has a DOM structure not known to the plugin. Given that we're reinstating the ability to shift-tab flyout->toolbox it makes sense to allow tab to the flyout. Shift-tab to the flyout remains not possible in the auto-hide case but that's not unreasonable. This version can be made to work with MakeCode without further modifications. feat: use ts and factor out helper functions
This was referenced Apr 24, 2025
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a rebuild/rebase of Christopher's #317.
I rebased it, tested that it worked, and then addressed Aaron's feedback in his review of #317.
I left alt in place because of Christopher's note about Mac users ("For now I will make both ctrl and alt usable (ctrl won't work for macOS users who use Spaces with default shortcut keys), and add a note to #106 that we need to figure out what to do instead.") but long-term I think we should actually move it to use shift+arrow for constrained and arrow for unconstrained. That matches what other dragging-related software does, generally.
I then fixed lint errors and formatted.
I was not (yet) able to resolve the issue with aborting a drag when the insertion marker is shown.