(fix) - O3-1169: Make Implementer Tools Dark Themed #960
Conversation
kb019
changed the title
|
@kb019 great job. Looks good to me. @denniskigen that documentation test fail again. |
|
|
Nice design implementations @kb019 👏🏿 |
Thank you @trevor-james-nangosha |
|
Hi @vasharma05 , @brandones , @denniskigen. Could you please review this pr. Thank you. |
|
@kb019 , nice work on this, thank you. Sorry I didn't specify this in the ticket, but it would be great if we can accomplish this using Carbon Themes rather than all this custom CSS. Right now all this CSS looks like it would add perhaps more maintenance burden than it is worth. Do you think you could try to get that to work? Again, thanks for your work, and sorry for not including this in the ticket description. |
|
Hi @brandones , Certainly! I understand your preference for using Carbon Themes over custom CSS for easier maintenance. I'll make the necessary adjustments to implement the changes using Carbon Themes as per your request. Thank you for providing clear guidance . |
|
Hi @brandones , I have tried to make the necessary changes by using carbon themes. Could you please review the pr and let me know if any changes are needed. |
| 'text-primary': #f5f5f5, | ||
| 'text-secondary': #bcbaba, | ||
| 'layer-selected-inverse': #696969, | ||
| 'text-inverse': #f5f5f5, |
There was a problem hiding this comment.
Any reason we shouldn't just use 'text-primary' and eliminate 'text-inverse'?
There was a problem hiding this comment.
Hi @brandones , The "text-inverse" is for the text color of the switch buttons when they are selected and the "text-primary" is for the color of the text when they are hovered. Yes, we can use "text-primary" but then the text color of the selected switch buttons should be explicitly specified as 'text-primary'. I can make that change if needed.
|
I think this looks great, @kb019 . Really nice job. I'm interested if @denniskigen or @ibacher have thoughts, since this is the first time we've really messed with themes at all. |
|
@kb019 Can you make the switch still green when it is on? I'm not sure the white really reads. |
Thank you @brandones. |
I have made the change @brandones. Thank you. |
There was a problem hiding this comment.
It seems like you're running a markdown link checker in a Docker container. This tool checks for broken links in your markdown files. The output shows that it has checked several files and found one dead link in the README.md file. The dead link is "packages/apps/esm-translations" → Status: 400. This means that the link to packages/apps/esm-translations is not working, returning a HTTP status code of 400. You should check this link and fix it if necessary.
|
Ping @denniskigen , could I get your input on this? It looks good to me but I want to run it by you. |
|
@kb019 Feel free to ignore what @mewanDimalsha wrote; it has nothing to do with your change. I have mixed feelings about the use of ChatGPT to generate comments... |
Sure, will ping him on this. |
|
I'm happy to have the implementer tools have a dark theme. It's a pretty isolated part of the app that is really just for administrators, so it's a sensible place to experiment with this. It would perhaps be nicer if we were using colors from one of Carbon's dark themes directly rather than a set of custom settings. The colors here are not far off Carbon's G90 theme, so maybe at least starting from that would make sense. |
|
|
||
| .darkTheme { | ||
| @include theme.theme(themes.$g90); | ||
| } |
There was a problem hiding this comment.
Hi @ibacher , as per your suggestion removed the custom themes and used only the g90 theme from carbon. I have recorded the changes in the below video link . Could you please look into it and let me know if you want any changes.
OpenMRS - Google Chrome 2024-04-04 14-48-42.webm
Tagging @brandones , @denniskigen .
There was a problem hiding this comment.
Hi @ibacher, Could you share your thoughts on this?
Requirements
feat,fix, orchore, among others). See existing PR titles for inspiration.For changes to apps
If applicable
Summary
To Make the implementer tools dark themed. Also changed some files from .css to .scss extension.
Screenshots
OpenMRS.-.Google.Chrome.2024-03-24.16-27-36.webm
Related Issue
https://openmrs.atlassian.net/browse/O3-1169