-
-
Notifications
You must be signed in to change notification settings - Fork 435
High Contrast theme updates #1265
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
Conversation
This is an excellent quick solution, @francescospissu 👍 I will try it out in action. @davegarthsimpson, could you please help review the effective code change? Thank you! |
(I have noticed that the baud dropdown does not have the right border. Not critical.)
(I have noticed that the settings and 3rd party URLs dialog have different OK/Cancel buttons orders. Not critical.)
I went through the list of changes, and they look good to me. I found a few places we might want to improve.
|
Thank you for testing it @kittaakos!
This difference is also present in the other themes:
I'll provide a fix for this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine with me for temp. workaround, nice one @francescospissu !
Since no one has complained in the past I was expecting to see the restore of the previous version of the adapted HC theme before the introduction of our latest Theme related PR that made us change how some variables are handled. But even if we have gone for this approach that emulate how vscode handle the HC theme I believe the final result is usable overall, apart from minor issues highlighted by @kittaakos . The only thing that I would change to align with the visual hierarchy of other Arduino themes is to restore the primary button entity on toolbar for verify, upload and debugger: |
These changes have been done taking into account what is reported in this issue:
About
I'll revert this behavior. |
It's looking great. All the issues I reported in #1265 (comment) were fixed. ✅ I found only this one: notification.mp4 |
Thanks for testing it @kittaakos! I fixed the notification hover status by adding a dashed outline (in Theia Blueprint it works like this): |
0d0c2c4
to
a53feb5
Compare
Thanks for testing it @AlbyIanna! I implemented your suggestions here. This is the result: |
Motivation
The High Contrast theme needs to be updated as hover status in drop-down menus and sketchbook is missing, board selector has no borders, and other design issues make the experience unpleasant to use.
Change description
Other information
This is a workaround until the version of Theia used is updated to 1.27.0, where the Theia High Contrast theme is updated and it's possible to use the Theia APIs to customize it.
Closes #1202.
Reviewer checklist