High contrast: Icons for block categories now have colors in Minecraft

[srietkerk]

All of our other targets still have the colors on the icons for the block categories, so we want Minecraft to match that.

Most of the categories are super straightforward because you can just change the icon color via code. However, the Agent category is a bit tricky because the icon that we're using doesn't have an associated icon in Semantic; we're using a static png. Because of this, changing the color of the Agent icon via code isn't really possible. That's where the new Agent_icon_orange file comes in. I just alternate between the two files to make the color changes work. I wasn't sure if referencing files across the repos like this would cause problems in production, so we might have to find another solution, but it works locally. One thing to note, though, is that when you first switch to high contrast mode, it takes a second for the agent icon to load in, so there is definitely room for improvements.

Will get an upload target on Monday.

Closes: https://github.com/microsoft/pxt-minecraft/issues/2062

[srietkerk]

UPDATE

I've decided to go the CSS route so it's a bit easier to interpret going forward. There are still a couple things that I want to fiddle with, but I wanted to push up my current changes.

The following has changed on this first pass:

1. Hover and not hovering for inverted toolbox is no longer handled in JS, but is handled by the :hover property in CSS
2. Styling for inverted and colored toolboxes is moved to toolbox.less
3. CSS variables made for block meta color, the fade color that applies to toolbox categories for inverted toolbox hover, and the border properties that we use for our colored toolbox. Looking at it now, I will likely just move this fully into CSS and pass in the --block-meta-color variable for it's color property.
4. I removed the logic for flipping the border side when the editor changes between right to left and left to right orientation for colored toolboxes. From what I can tell, we sort this out programmatically. When adding the coloredToolbox styling to CSS, I noticed that the styling in CSS that specified border-left automatically turned into border-right.

What I want to change still:

1. The icon image style. Right now, we are using a style element to apply more specific styling to blocks categories that use images as their icons. I think a majority of this styling can just live in CSS, and in the case of the backgroundImage url, I'll use a CSS variable as was done with the meta color.

Things that need clarification:

1. The animation stuff that was in the styling. Thanks to the git blame, I was able to find this PR: Toolbox visibility: animate drawers, hover effect on flyout #6774, showing what the animation stuff should be giving us. This is broken as far as I can tell. I'm wondering if this is something that I should open a bug for or if I should just get rid of the animation logic.

[jwunderl]

Awesome, nice clean up! Thanks for listening to me ramble on this for a bit & then fixing everything!

left a few comments since you with minor things since you mentioned still wanting to clean up a little more, Also, in case it's helpful, here's a pointer to the place I can remember using a css variable for an image url , I remember it being a little finicky / having to play with it but that was probably just cause I was doing a datauri + css variable instead of just a url https://github.com/microsoft/pxt/pull/9329/files#diff-7047859ba8a434609145f1795eb0ee90ad1991dcdb4e22fc986bc181f2a55862R350

[thsparks]

Hover and not hovering for inverted toolbox is no longer handled in JS, but is handled by the :hover property in CSS

I might be missing something obvious, but I couldn't find where this is happening. Can you point me to it?

The animation stuff that was in the styling. Thanks to the git blame, I was able to find this PR: #6774, showing what the animation stuff should be giving us. This is broken as far as I can tell. I'm wondering if this is something that I should open a bug for or if I should just get rid of the animation logic.

Personally, I'd recommend just opening a bug for now. No need to remove the code; it could be helpful for whoever ends up debugging.

[srietkerk]

I might be missing something obvious, but I couldn't find where this is happening. Can you point me to it?

Yeah, no problem,

pxt/theme/toolbox.less

Line 285 in 8aa2687

div.blocklyTreeRow:hover {

[thsparks]

Personally, I'd recommend just opening a bug for now. No need to remove the code; it could be helpful for whoever ends up debugging.

Unless it's causing a crash or something, that is. In that case, I'd say we should remove it and reference the commit that removed it in the bug. If it's just not doing anything then I think it's fine to leave it in.

[srietkerk]

Personally, I'd recommend just opening a bug for now. No need to remove the code; it could be helpful for whoever ends up debugging.

Unless it's causing a crash or something, that is. In that case, I'd say we should remove it and reference the commit that removed it in the bug. If it's just not doing anything then I think it's fine to leave it in.

Yep, not causing any issues, just not adding anything.

[jwunderl]

Again nice job :)