Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plugin-cross-tab-copy-paste #2058

Open
1 task done
bartbutenaers opened this issue Nov 3, 2023 · 3 comments
Open
1 task done

plugin-cross-tab-copy-paste #2058

bartbutenaers opened this issue Nov 3, 2023 · 3 comments
Assignees
Labels
type: bug Something isn't working

Comments

@bartbutenaers
Copy link

bartbutenaers commented Nov 3, 2023

Check for duplicates

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

Component

plugin-cross-tab-copy-paste

Description

Hi everybody,

I have used the plugin-cross-tab-copy-paste plugin for the first time, and it works very well. Very useful plugin!

However in our application we create a new workspace when we show the old workspace in full screen. When we do that now, the following exceptions is raised. All my other plugins have no issue, but they all have a dispose function that does the cleanup of all stuff. However the plugin-cross-tab-copy-paste plugin does 't offer such a function...

Thanks!
Bart

Reproduction steps

  1. Call ´Blockly.inject´
  2. Initialize the plugin:
    node.copyPastePlugin = new CrossTabCopyPaste();
    node.copyPastePlugin.init(copyPastePluginOptions, function() {
       //console.log('Use this error callback to handle TypeError while pasting');
    });
    
  3. For all other plugins I call now their dispose function, but this plugin doesn' t have it...
  4. Call ´Blockly.inject´ again. Now an exception occurs

Stack trace

Uncaught Error: Menu item with ID "blockCopyToStorage" is already registered.
    at ContextMenuRegistry$$module$build$src$core$contextmenu_registry.register (contextmenu_registry.ts:45:13)
    at t.blockCopyToStorageContextMenu (index.js:90:5)
    at t.init (index.js:33:12)
    at createWorkspace (<anonymous>:455:34)
    at Object.open (<anonymous>:918:29)
    at n (red.min.js?v=:19:44660)
    at red.min.js?v=:19:45794

Screenshots

No response

@bartbutenaers bartbutenaers added triage type: bug Something isn't working labels Nov 3, 2023
@alicialics
Copy link
Contributor

alicialics commented Nov 4, 2023

does it work if you call Blockly.ContextMenuRegistry.registry.unregister('blockCopyToStorage'); in place of plugin.dispose()

@bartbutenaers
Copy link
Author

Hi @alicialics,

Thanks for joining (again)!

  1. In some circumstances I arrive at the cleanup part before the plugins have been initialized, and then your statement results into this:

    Uncaught Error: Menu item with ID "blockCopyToStorage" not found.
        at ContextMenuRegistry$$module$build$src$core$contextmenu_registry.unregister (contextmenu_registry.ts:58:17)
        at cleanup_plugins (<anonymous>:549:50)
     at createWorkspace (<anonymous>:369:9)
     at lastScriptToLoad.onload (<anonymous>:217:13)
    

    Which can be resolved by checking first if the context menu item has already been registered.

  2. Beside the blockCopyToStorage menu item, there is also a blockPasteFromStorage menu item. Which needs the same treatement.

As a result, the following code snippet seems to do the job for me:

if (Blockly.ContextMenuRegistry.registry.getItem("blockCopyToStorage")) {
   Blockly.ContextMenuRegistry.registry.unregister('blockCopyToStorage');
}

if (Blockly.ContextMenuRegistry.registry.getItem("blockPasteFromStorage")) {
   Blockly.ContextMenuRegistry.registry.unregister('blockPasteFromStorage');
}

So a dispose function with that would be sufficient...

BTW I am not familiar with Typescript. Is there perhaps somewhere a description of how to contribute something like this (e.g. how to generate the javascript...)?

Enjoy your weekend!
Bart

@alicialics
Copy link
Contributor

alicialics commented Nov 4, 2023

Thanks Bart! =)

The typescript declarations are fairly straightforward I think, you can a new function declaration here:

dispose(): void;

I took a look at CrossTabCopyPaste and suspect that it doesn't have dispose because having one meant that it might need to cleanup these shortcuts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants