feat: delegate clipboard actions to core #501
Conversation
Amazing. I hadn't spotted that the unit had changed when it moved into core. |
| Events.setGroup(true); | ||
| const block = clipboard.paste(this.copyData, pasteWorkspace) as BlockSvg; | ||
| if (block) { | ||
| if (targetNode) { | ||
| this.navigation.tryToConnectBlock(targetNode, block); | ||
| } | ||
| Events.setGroup(false); | ||
| return true; | ||
| } | ||
| Events.setGroup(false); |
There was a problem hiding this comment.
This removed part doesn't have an equivalent so the behaviour has reverted to the core behaviour that doesn't connect.
I'm not sure this is a bad thing - see discussion on #480 and recent user testing feedback - but checking whether it's intended here.
There was a problem hiding this comment.
Yes, I read #480 and figured that staying consistent with the behavior in core made sense, and if we wanted to change it, we could change it in core. It should be easy to do a one-time move mode toast, I should get to the "keyboard mode" heuristic soon and we could use that.
|
Updated to v12 release and addressed Aaron's suggestion |
|
This has allowed us to implement MakeCode's cross tab cut / copy / paste shortcuts so no further changes are suggested for launch, however, there was a bit of work involved.
This is probably not that bad by itself, but made worse by the fact that the using the keyboard nav plugin is a user setting, so all of the above happens optionally with alternatives where the plugin isn't used. Not sure if there's anything that can change to make this more straightforward, but I think we'd support having the clipboard exposed in core so that the plugin can determine if paste should be enabled as this would help most users with simpler cases. |
4 participants
Works on #488
Fixes #237
Fixes #500
This PR removes all of the custom copy/paste logic and simply delegates the responsibility for copy/pasting to core, or the currently registered copy/paste functions. This simplifies the plugin, and makes it possible to use the cross-tab-copy-paste plugin as long as that plugin registers its replacement shortcuts under the same shortcut names that core uses.
The only custom logic for keyboard shortcuts is that we show/hide toasts on clipboard actions, and we paste in the main workspace instead of flyout workspaces.
We add the context menu options but those also largely use the implementations from core. We can't totally reuse the preconditionFn from core for the keyboard shortcuts, because we can't use the check if a gesture is in progress (it always is during a right click/context menu being open). We can also handle additional places of showing these options in the context menu, such as showing paste on the workspace's menu and not just a block's.
Also note that I decreased the toast time from almost 2 hours (7000 seconds) to 7 seconds.
Adopting this approach would mean that any edge cases around what to do when you try to copy a connection for example would need to be added in core, not in the plugin.
One downside to this approach is that the "paste" option appears enabled even if you have never copied anything. The core implementation does not expose this information so we can't check it, and we can't use the precondition from the keyboard shortcut because that checks if there's a gesture in progress. However, it may be possible to expose the clipboard data from core. Doing so would probably be semi-ugly (I sort of hate that the clipboard data is just held in the keyboard shortcut items file instead of being its own class).
And finally, note that until RaspberryPiFoundation/blockly#8793 is fixed, this PR doesn't handle cut behavior correctly. After cutting a block, nothing is focused, so you have to click the workspace again before you can paste or do something else with the workspace.
@microbit-matt-hillsdon I would appreciate if you could take a look and let me know if you think this approach would work with the makecode special pasting logic.