-
Notifications
You must be signed in to change notification settings - Fork 328
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
feature - add dark mode toggle to context menu [WIP] #96
feature - add dark mode toggle to context menu [WIP] #96
Conversation
@@ -186,6 +186,18 @@ app.on('ready', () => { | |||
}, | |||
]; | |||
|
|||
const darkModeToggle = { | |||
label: 'Toggle Dark Mode', |
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.
should be easy to add keybinding here
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.
Pushed a change just now that I think takes care of this. Is that keybinding acceptable?
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.
Hmm, so just recompiled and it looks like CommandorControl+Shift+L
accelerator is only working while the context menu is open and in-focus. I think there's either an issue with this specific shortcut combo, or another corresponding change needs to be made elsewhere.
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.
sure thing. @seanoliver can take a look
click: () => { | ||
store.set('darkMode', !store.get('darkMode', false)); | ||
setTimeout(() => { | ||
window.reload(); |
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.
@eshack94 Does the toggle only work with a refresh?
When we reload the window we automatically start a new empty chat thread and reset the panes as well, so ideally those remain separate actions if possible.
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.
I've only tested with the window reload so far. I can try to figure out another way to handle it, but it might take some trial-and-error and research. Do you have any ideas on alternative ways to handle it?
|
||
// Add the .dark class to the body | ||
document.body.classList.add('dark'); | ||
static toggleDarkMode(isDarkMode) { |
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.
This won't work for Claude, Bing, and a few others that don't support the system dark mode color property.
But for those providers we can just move the custom dark mode CSS we'd put together into toggleDarkMode at their level. I can take a stab at that now while I look at the shortcut thing.
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.
Makes sense. Thank you for the help!
|
||
// Get the initial value of isDarkMode from the store | ||
ipcRenderer.invoke('getStoreValue', 'isDarkMode').then((isDarkMode) => { | ||
enabledProviders.forEach((provider) => provider.toggleDarkMode(isDarkMode)); |
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.
We should probably run this over all providers rather than just enabled ones so they don't get out of sync when people add/remove providers.
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.
Good idea, totally agree. We want the state for dark/light to stay in sync across all providers.
So far, I've only added changes to
index.js
,interface.js
, andproviders/openai.js
. I've tested with ChatGPT and the current implementation works well for this provider. Other providers have not yet been added.You can pull down my feature branch and build via
npm run make
and test with the changes from this PR to verify.Stuff I could use some pointers/help on:
It seems that there is a somewhat different implementation for dark mode for each provider. Thus, I'll need to figure out the required changes for each. I could use some help with this part if someone wants to lend a hand or provide some guidance.
Within my browser's DevTools, where would I look to find out the provider-specific changes that get applied when a user toggles on dark mode within the browser?
I'm also open to suggestions for how to improve this!
TODO:
providers/bard.js
providers/bing.js
providers/claude.js
providers/claude2.js
providers/huggingchat.js
providers/oobabooga.js
providers/perplexity.js
providers/phind.js
providers/smol.js
Related open GH issue: #95