Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Hi @tgbhy. Thanks for this useful PR! The changes look good but there are some additional steps needed before we can accept the PR:

• You'll need to agree to the CLA.
• eslint is unhappy about all the properties with underscores in their name. You'll need to disable this rule for these declarations.
• You'll also need to run npm run format to run Prettier and thereby fix the formatting.

Working on it !

This is fine now ?

Wait, it's already the case ?
Where did you see that, it's already the case of being "colourBlocks" in every themes...

Bump here

Hi @tgbhy we can't merge this because renaming the properties will break them. I'm not sure how you tested the change, but the way themes work are that they correlate a category name to a set of styles. The category names like colour_blocks are set in core, so changing them here and not in core would break the theme as there is no category name called colourBlocks. And we are not interested in changing the category names in core.

We can accept this PR after you address Christopher's comment here https://github.com/google/blockly-samples/pull/2474/files#r1929130761

Please let us know if you have further questions. Thanks for your work on this and for following up!

oh okay, I don't know why it was working perfectly for me...
I'll do the change, you can come back in less than an hour I think ;)

You will need to fix this for all of the defaultBlockStyles and categoryStyles, not just the colour_blocks.

So wait, I need to do this only for defaultBlockStyle (-> default_block_style) and categoryStyles (-> category_style) ?
After that it's finish ?

Because I really don't understand why you ask me that because it was working fine for me but whatever, just confirm me that and I'll do the changes.

You can check how a theme is supposed to look on the published plugin showcase page. Here's what the highcontrast theme is supposed to look like, for example: https://google.github.io/blockly-samples/plugins/theme-highcontrast/test/index.html

You can test your change by running npm start from the plugin's directory. When I checkout your PR and run npm start from the highcontrast plugin, here's what I see:

All of the blocks are black and the toolbox is missing the little color stripe for the category. This means your theme is broken.

You can read more about how themes work here.

The properties in the theme like defaultBlockStyle and categoryStyle are supposed to be camel case. However, those properties are objects where the keys are arbitrary values set by the owner of the blocks. If an application owner creates their own block category and decides to call it mY_SpEcIaL_BlOcKs, that's what would have to go in as the object key, so that blockly can match that name from the toolbox definition to the theme definition. The themes you're editing provide values for the default blocks and categories that Blockly ships with (like logic_blocks, math_blocks, and so on). Since Blockly uses snake case for these values, the theme has to as well.

This PR shouldn't change any object keys as part of the change. You should revert all of those kinds of changes. If the key was previously logic_blocks it needs to remain that way. If the key was previously defaultBlockStyles it needs to remain that way. Does that help?

Okay thank you, I will do all the edits right now.

And done !

Give me a sec I have an idea
I'm really sorry for this ^^'

And the work is done !
Sorry, I'll pay attention to that the next time ^^

Any news here ?

No problem. Will it be merged now, or does it need more modifications?