Themes selection

[bartbutenaers]

When people use e.g. a dark Node-RED theme, the Blockly editor colors don't really match with that theme:

Recently a series of Blockly themes plugins have been released.

In the blockly-themes branch of this repository, I have already implemented some changes to support those themes in this node:

1. The package.json file downloads all theme plugins as dependencies:

   "@blockly/theme-dark": "^6.0.1",
   "@blockly/theme-highcontrast": "^5.0.1",
   "@blockly/theme-tritanopia": "^5.0.1",
   "@blockly/theme-deuteranopia": "^5.0.1",
   "@blockly/theme-modern": "^5.0.1",
   

2. The config node has a dropdown to select one of the available themes:

   

   The default theme is "classic".

3. The blockly.config file has been updated to:

   • Get the selected theme name from the config node.
   • Load the corresponding blockly plugin as ES6 module from the backend
   • Pass the blockly theme instance as a parameter to the Blockly.inject function

But unfortunately the workspace view layout is messed up then.
If anybody wants to help solving it, that would be appreciated very much!!!

[ralphwetzel]

Hi @bartbutenaers!

There's good news for you and those using your great node. 😄
Theme support is possible! Watch this:

I found two reasons why the workspace is messed up with your current setup:

• Importing the blockly-theme modules runs into an error. It seems to be systematic & can't be compensated for. 👎
• When you recreate the workspace, searchElement is the only object that's not disposed upfront. This ends into an error stating that a shortcut is already registered - breaking what follows.

The second bullet is for you to figure out. You'll find those portions commented.
I've solved the first for you - with an ugly, but successful hack!

Expect a PR soon!

[bartbutenaers]

Really cool!
You are my hero for today ;-)

[bartbutenaers]

I will keep this issue open until the second bullet is solved.

[bartbutenaers]

@ralphwetzel: Could you do me one more favor. Which steps do I need to follow to reproduce the error? I have seen recently some issues on the Blockly forum about disposing some plugins. So perhaps something is wrong about that yet.

[ralphwetzel]

It's enough to remove the comment tags from lines 221 - 229:

node-red-contrib-blockly/blockly.html

Lines 216 to 229 in c622d5b

	// Ralph:
	// The way searchElement is implemented creates trouble beginning with the second invocation of onload:
	// "shortcut_registry.ts:55 Uncaught (in promise) Error: Shortcut named "startSearch" already exists."
	// Thus I've put comment tags in front of the following section!
	// // Add a 'Search' category at the end, which is linked automatically by Blockly to the toolbox-search plugin.
	// const searchElement = document.createElementNS("http://www.w3.org/1999/xhtml", "category");
	// searchElement.setAttribute("name", "Search");
	// searchElement.setAttribute("kind", "search");
	// // When the search input field is clicked, a blue background appears.
	// // I tried to apply as color the same color as the background color (grey), but it doesn't help.
	// // The same happens in the official plugin demo (https://google.github.io/blockly-samples/plugins/toolbox-search/test/index.html)
	// searchElement.setAttribute("colour", "#B2B2B2"); // Same color as the background
	// node.toolboxXmlDocument.documentElement.appendChild(searchElement);

Re-enabling searchElement then triggers the following error on my system:

[ralphwetzel]

I just did some further tests.
Looks like this issue is related to the fact that we load the themes immediately before the definition of searchElement. I'm not sure why - but moving

node-red-contrib-blockly/blockly.html

Line 214 in c622d5b

await load_themes();

after searchElement definition doesn't trigger the error. I'll send you another PR.

[cymplecy]

Minor issue - field is blank on exisiting node

[bartbutenaers]

@cymplecy: i will add tonight a statement to put a default value "classic" to this field when it is undefined. Similar to the offset ´0´ like last week.

[ralphwetzel]

I have the impression that implementing the theme loader with an async/await pattern caused this issue.
As I'm not convinced that the "fix" proposed in my last PR is fully reliable (it might be for this case, but none knows what happens in the future!), I've re-written the loader to run as a sync process. This might look a bit "old fashioned", but follows the way other resources are loaded, too.

[bartbutenaers]

Minor issue - field is blank on exisiting node

@cymplecy, that should be fixed now in the github version.
If you have some time to test it and also test the theme mechanism, that would be fine!

I've re-written the loader to run as a sync process

Really really appreciated! I would have to put a lot of other stuff on pauze, if you wouldn't have solved all of this...

BTW you have also updated the position of the expand button from 'in' the tabsheet to 'above' the tabsheet:

Is that the intention, or does it perhaps looks different on your system?
At the time being I had added it on top of the tabsheet, to save space to be able to show the Blockly workspace as large as possible. Moreover that way it was more clear that only the first tabsheet could be expanded...

[ralphwetzel]

😆
My intention was to move the expand button from 'under' to 'on' the tab. And it sits there perfectly - on my system (NR 3.1, MacOS):

Chrome:

Safari:

Firefox:

EDIT: ... and on a MacOS system running NR 4.0-dev I get what you get: 🎉

I'll try to figure out what's happening...

[bartbutenaers]

Ah it was originally under your tabsheet.
I wasn't aware of that...
Now I understand the reason behind your change 😀

[cymplecy]

@bartbutenaers
Theme is defaulting to classic if not previously set in the config :)

[bartbutenaers]

@ralphwetzel:
I can confirm that the expand button is now also correctly positioned inside the tabsheet, on my Windows 10 portable.
Kudos!!!

@cymplecy: Am I correct that everything is solved from the above discussion. If so, you may close this issue. Thanks!

[bartbutenaers]

@cymplecy; if you can also check my last question, so that we can finalize our release... Thanks!

[cymplecy]

Ignore missing theme selector report if you see it-PEBKAC here :)