-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -186,6 +186,19 @@ app.on('ready', () => { | |
}, | ||
]; | ||
|
||
const darkModeToggle = { | ||
label: 'Toggle Dark Mode', | ||
accelerator: 'CommandorControl+Shift+L', | ||
type: 'checkbox', | ||
checked: store.get('darkMode', false), | ||
click: () => { | ||
store.set('darkMode', !store.get('darkMode', false)); | ||
setTimeout(() => { | ||
window.reload(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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? |
||
}, 100); | ||
}, | ||
}; | ||
|
||
const separator = { type: 'separator' }; | ||
|
||
const providersToggles = allProviders.map((provider) => { | ||
|
@@ -248,6 +261,7 @@ app.on('ready', () => { | |
// Return the complete context menu template | ||
return [ | ||
...menuHeader, | ||
darkModeToggle, | ||
superPromptEnterKey, // TODO: move into the customize keyboard shortcut window | ||
separator, | ||
...providersToggles, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,22 @@ window.addEventListener('DOMContentLoaded', () => { | |
updateSplitSizes(panes, splitInstance); | ||
}); | ||
|
||
/* ========================================================================== */ | ||
/* Dark Mode Listeners */ | ||
/* ========================================================================== */ | ||
|
||
// 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 commentThe 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 commentThe 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. |
||
}); | ||
|
||
// Listen for changes to the isDarkMode value in the store | ||
ipcRenderer.on('setStoreValue', (event, key, value) => { | ||
if (key === 'isDarkMode') { | ||
enabledProviders.forEach((provider) => provider.toggleDarkMode(value)); | ||
} | ||
}); | ||
|
||
/* ========================================================================== */ | ||
/* Prompt Input Listeners */ | ||
/* ========================================================================== */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,19 +63,43 @@ class OpenAi extends Provider { | |
|
||
`); | ||
setTimeout(() => { | ||
this.getWebview().executeJavaScript(` | ||
// Get the root element | ||
const root = document.querySelector(':root'); | ||
// Get the current dark mode setting from the store | ||
const isDarkMode = store.get('darkMode', false); | ||
|
||
// Set the color-scheme CSS variable | ||
root.style.setProperty('--color-scheme', 'dark'); | ||
// Toggle dark mode based on the current setting | ||
this.toggleDarkMode(isDarkMode); | ||
}, 300); | ||
}); | ||
} | ||
|
||
// 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. Thank you for the help! |
||
if (isDarkMode) { | ||
// Add code to enable dark mode | ||
this.getWebview().executeJavaScript(` | ||
// Get the root element | ||
const root = document.querySelector(':root'); | ||
|
||
`); | ||
}, 0); | ||
}); | ||
// Set the color-scheme CSS variable | ||
root.style.setProperty('--color-scheme', 'dark'); | ||
|
||
// Add the .dark class to the body | ||
document.body.classList.add('dark'); | ||
document.body.classList.remove('light'); | ||
`); | ||
} else { | ||
// Add code to disable dark mode | ||
this.getWebview().executeJavaScript(` | ||
// Get the root element | ||
const root = document.querySelector(':root'); | ||
|
||
// Remove the color-scheme CSS variable | ||
root.style.setProperty('--color-scheme', 'light'); | ||
|
||
// Remove the .dark class from the body | ||
document.body.classList.add('light'); | ||
document.body.classList.remove('dark'); | ||
`); | ||
} | ||
} | ||
|
||
static isEnabled() { | ||
|
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