-
Notifications
You must be signed in to change notification settings - Fork 595
Teacher Tool Theme Fixes #10481
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
Teacher Tool Theme Fixes #10481
Conversation
…button appearance.
I think we should introduce bespoke color vars for toast now, and plan to move the Toast component into react-common "soon" (perhaps as part of the current microbit work). |
but it's something we could certainly do later! |
Had a quick chat offline: concern here was around differentiation between color vars (red/yellow/green) and status (error/warning/success). We should probably have different theme colors for both, even if they sometimes duplicate each other. For example, micro:bit green is quite dark. It'd be nice for the success color to be a bit brighter. But I will do that in a separate change. |
I was originally thinking we could just use the colors-green, colors-yellow, and colors-red variables for this, but as was noted in #10481, those may be a bit too generic to work well for these scenarios. In the teacher tool, for example, the microbit green color is a little too dark to work well as a "success" color. Separating them will lead to some duplication in certain themes, but it's nice to have the flexibility. As is the case for all theme colors, any themes that do not specify a value in their json file will default to the value in base-theme.less. For now, I've only used these new colors in the teacher tool. We can swap them out in other areas as needed.
In microsoft/pxt#10481, I have swapped out the themepack with our new theme variables, so we can remove the themepack file from microbit. The one thing we needed to keep was the font variable, so I've moved that into style.less instead. I've also added a color that was missed in the original theme file. This already existed in arcade themes (and the base theme).
This fixes some issues with the teacher tool UI after themes were introduced. While doing this, I decided to fully swap out the themepack with our new theme variables, just to keep things consistent across the apps. It should also make things "just work" if we introduce other themes for teacher tool (though I haven't done that in this change).
Since I didn't want to introduce new theme colors, I have had to make some minor adjustments to how things look (like the beta tag & toasts), but I tried to stay relatively true to the original designs.
Upload Target: https://makecode.microbit.org/app/2c6d961517b971a19375a8090903860676c8e0f7-fa4ad57a6d--eval