Add OS dark mode support #2868
Add OS dark mode support #2868
Conversation
|
Hey @develohpanda, did you get a chance to look at this already? |
Hi @flostellbrink! Unfortunately not yet 😞 I've been a little pressed for time given the awesome number of PRs and pushing for a big release last week. I'll be sure to run through it in the coming or the following week! 🤗 |
|
|
There was a problem hiding this comment.
This is an awesome PR and I'm pretty excited to get it in! 🎉
Generally it looks good to me. I'd like to propose a few small changes to the Themes tab interface, in order to allow a user to use any theme for any color scheme, instead of us separating light and dark themes.
I'll get some feedback from our designer @erickrico around a nicer interface in the Themes tab, for selecting the light/dark theme when auto-detection is enabled. That will require some refactoring so I haven't reviewed the files I anticipate to change.
|
Hi @flostellbrink! Firstly, I've just tested this on macOS and it seems to detect the color scheme as expected 👏🏽 I had a chat with our PM and designer, and we think it might be good to follow the VS Code approach for now. This would involve not splitting the theme listing into light and dark, but instead allowing for light/dark selection through two drop-downs, which show all themes and default to those in app config. Most of the work you have done is still applicable, there will just be a few changes in the themes tab itself. Here's a quick mock-up just with the HTML, pulling elements from the General settings tab.
|
|
@develohpanda Thanks for the feedback and for finding a nice UI solution! Gonna get back to you on the weekend :) |
|
Heey, sorry for the delay. Also got the new look in: Hope this does the trick. Happy Holidays! |
There was a problem hiding this comment.
Awesome! I've just got some minor comments, namely to convert the object maps to switch/if-else statements.
I was doing some testing locally and noticed that when the "use OS color scheme" box is checked, clicking on the themes above doesn't do anything but still changes the selection. We could disable the click handler entirely, and not show the selected 'default' theme, essentially making it read-only. What would you expect to happen, or do you have any other ideas for that case?
Happy belated holidays to you too! Sorry for the delay, I was on vacation. 😄
packages/insomnia-app/app/ui/components/modals/settings-modal.js
Outdated
Show resolved
Hide resolved
|
Heey @develohpanda! Sorry for the delay. You're right, its confusing to change the default theme when it is not enabled. Gonna fix those conflicts real quick. |
Default light and dark themes can still be changed. For now its studio-light and default for core, and studio-dark and studio-light for designer.
The detection of dark scheme is based on the background color at the moment. This seems to work pretty well, but is not an ideal solution. I think themes should at least get to override this.
This adds a checkbox to the theme settings that determines whether we use the OS color scheme. If we don't (default) everything stays the same as before. If we do, themes are rendered in two groups. One for the light themes and one for the dark themes. They can be chosen independently. None of this overrides the default theme choice.
Themes are still aligned by adding negative margin. A bit of a hack, open for suggestions.
This makes sure that we don't override the user's choice.
Co-authored-by: Opender Singh <opender94@gmail.com>
Co-authored-by: Opender Singh <opender94@gmail.com>
Co-authored-by: Opender Singh <opender94@gmail.com>
Do not use object literal lookups to make code more readable
|
Okay, had to rebase, it was getting a bit messy. Quite a lot has happened since last time. Looks like things are still working though. |
packages/insomnia-app/app/ui/components/modals/settings-modal.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Opender Singh <opender94@gmail.com>
Co-authored-by: Opender Singh <opender94@gmail.com>
Replace :not(:disabled) with :enabled. Not sure what I was thinking there.
There was a problem hiding this comment.
Can confirm this worked on Linux (Kubuntu).
My only qualm is that the UI isn't crystal clear to me at first as to what it's selecting (and also what is currently selected). For example, it'd be nice if we could get a preview next to each of these (like exists above) for each dropdown. Then, that would give the user some indication of what theme is currently active (which we have above), e.g.
There was a problem hiding this comment.
also, is it an option to instead use https://www.electronjs.org/docs/api/native-theme. It seems better to rely on an electron javascript API than a css media query. At least with the javascript API we can be notified by the type system if anything ever changes - while with CSS we just have to hope it never breaks.
There was a problem hiding this comment.
On second thought, I see that a lot was done in this PR and what was implemented does work quite well from my testing. Although my suggestion about the preview would be nice we can always iterate - I don't want to hold this up at this juncture. I came and just immediately looked at the code and the code alone without looking at any of the discussion, my b. haha
|
I think it might be worth investigating use of the Electron API mentioned above - if not trivial, we can stick with the CSS query for now. |
|
Deploy preview for insomnia-storybook ready! Built with commit 077ab32 |
|
Thanks for bringing up the API option @dimitropoulos! I just read the electron guide on dark mode. That makes it sound like the nativeTheme API is more useful for other use cases like syncing an applications dark mode preferences back into the OS components. For just updating our theme they recommend this:
And while I absolutely agree that having a strongly typed API for this would be nice, I think there is also something to be said for just using standard web apis when possible. Either way, I don't think I can justify spending time on implementing that change at the moment. |
|
I most definitely understand. Thanks for looking into it!! |
Just discovered insomnia and I love it, especially the themes and plugins!
One thing I missed was the theme updating when I switch my OS to dark mode.
So I went ahead and added that. Maybe you'll find it useful too.
Not sure how well this works outside of Windows 10, but I am guessing electron should make it work.
Figuring out what is a light and what is a dark theme is just based on the background color.
I know that's not ideal, but it seems to work pretty well.
I think there should be some way for themes to override that if they want.
Please feel free to change the default themes, I just picked them arbitrarily.
Closes #3154