Enable Dark Theme Support #28
Conversation
bitnimble
left a comment
bitnimble
left a comment
There was a problem hiding this comment.
Thanks for the PR! pretty much lgtm - I've unfortunately just reinstalled Windows and I'm doing some travelling at the moment as well, so my reviews may be a little slower than usual. I also have to get my dev environment set up from scratch again, so I don't have the repo checked out and running locally etc, so some of my review comments will ask to just double check things on your end if I can't check it easily myself.
This PR lgtm, just have one thing to check (not critical)
src/ui/base/text/text.tsx
Outdated
| type TextStyle = 'regular' | 'title' | 'monospace' | 'code'; | ||
| type TextWeight = 'regular' | 'semibold' | 'bold' | 'extrabold' | 'black'; | ||
| type TextColor = 'black' | 'red' | 'white' | 'grey' | 'purple'; | ||
| type TextColor = 'black' | 'red' | 'white' | 'grey' | 'purple' | 'auto'; |
There was a problem hiding this comment.
is there anything using 'black' right now that would need to be updated to 'auto'? and can 'black' just be removed now?
There was a problem hiding this comment.
I added it at first and then kept it, although black and auto are not being used I thing may be useful to leave it for future updates. Auto is the default contrast color of the light/dark theme, and black and white can be used to force color on specific ui elements that require it
There was a problem hiding this comment.
yep, what I'm saying is that the current 'black' prop might be currently used somewhere with the assumption that the site is in light mode - those would need to be changed to auto now, if they exist.
if no usage of the black prop exists, then we should just remove it, since they most likely should be using auto instead, I think
2 participants
This MR adds a simple dark theme option to the application. Changes include:
All existing functionality is preserved. No breaking changes expected. Please review the theme switcher behavior on different pages.
related issue: #25