feat: add auto start on startup option to user settings #1564
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds an auto start on startup option to user settings, allowing users to configure Session to launch automatically when their computer starts. On Linux systems where auto start is unsupported by Electron, toggling the setting displays an informational modal explaining the limitation.
- Implements auto start functionality with platform-specific handling for macOS, Windows, and Linux
- Adds UI toggle in preferences settings with conditional availability based on platform
- Creates informational modal for Linux users explaining system-level startup configuration
Reviewed Changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ts/state/ducks/modalDialog.tsx | Adds optional hideOkayButton property to LocalizedPopupDialogState |
| ts/mains/main_node.ts | Implements IPC handlers for getting/setting auto start functionality with platform-specific logic |
| ts/data/settings-key.ts | Adds settingsAutoStart key for storing auto start preference |
| ts/components/dialog/user-settings/pages/PreferencesSettingsPage.tsx | Adds auto start toggle UI with Linux platform detection and modal integration |
| ts/components/dialog/user-settings/components/SettingsToggleBasic.tsx | Enhances toggle component to support unavailable state with custom modal |
| ts/components/dialog/LocalizedPopupDialog.tsx | Adds conditional rendering for okay button based on hideOkayButton prop |
| preload.js | Exposes auto start getter/setter functions to renderer process |
| _locales/en/messages.json | Adds localization strings for auto start feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Aerilym
force-pushed
the
feat/settings_auto_start_option
branch
from
6ac2717 to
bc6edfa
Compare
| "settingsCannotChangeDesktop": "Cannot Update Setting", | ||
| "settingsStartCategoryDesktop": "Startup", | ||
| "launchOnStartDesktop": "Launch on Startup", | ||
| "launchOnStartDescriptionDesktop": "Launch Session automatically when your computer starts up.", | ||
| "launchOnStartupDisabledDesktop": "This setting is managed by your system on Linux. To have Session start automatically, please add it to your startup applications in your system settings.", |
There was a problem hiding this comment.
avoid commiting those, we want those to come from crowdin only.
The flow should be
- commit all but those changes, even if that means the CI will fail
- wait for the strings to be added to crowdin
- merge the crowdin PR to your branch (you get the new strings)
- the CI should now be fixed
| // make sure to write it here too, as this is the value used on the UI to mark the toggle as true/false | ||
| await window.setSettingValue(SettingsKey.settingsAutoStart, newValue); | ||
| await window.setAutoStartEnabled(newValue); |
There was a problem hiding this comment.
should we make those 2 calls only one, and call one from the other? I feel like we are going to forget at some point.
I guess we should keep window.setAutoStartEnabled here, and call
window.setSettingValue in it, once we get that the change was made (and actually maybe do a get, instead of assuming the change worked?
Not sure if that's possible
There was a problem hiding this comment.
Actually, I think we should remove SettingsKey.settingsAutoStart and always rely on the value from getLoginItemSettings.
It would be nice to not duplicate the settings itself and having to keep track of it.
ts/mains/main_node.ts
Outdated
| userConfig.set('autoStart', newValue); | ||
|
|
||
| // Set the login item settings based on the platform | ||
| if (process.platform === 'darwin') { |
There was a problem hiding this comment.
why not use isLinux, isWindows etc?
ts/mains/main_node.ts
Outdated
| // Linux and other platforms - basic support | ||
| app.setLoginItemSettings({ | ||
| openAtLogin: newValue, | ||
| }); |
There was a problem hiding this comment.
we don't support that right? Maybe throw?
There was a problem hiding this comment.
yeah sorry this was meant to be gone, i included it to test it, can confirm it doesnt do anything on ubuntu
ts/mains/main_node.ts
Outdated
|
|
||
| ipc.on('set-auto-start-enabled', (event, newValue) => { | ||
| try { | ||
| userConfig.set('autoStart', newValue); |
There was a problem hiding this comment.
do we need to set/save this here? the setLoginItemSettings is not enough?
There was a problem hiding this comment.
yeah we can remove
| const appFolder = path.dirname(process.execPath); | ||
| const ourExeName = path.basename(process.execPath); | ||
| const stubLauncher = path.resolve(appFolder, '..', ourExeName); | ||
|
|
||
| app.setLoginItemSettings({ | ||
| openAtLogin: newValue, | ||
| path: stubLauncher, | ||
| args: [], | ||
| }); |
There was a problem hiding this comment.
Yep, works on windows
ts/mains/main_node.ts
Outdated
| const configVal = userConfig.get('autoStart'); | ||
|
|
||
| if (configVal !== undefined) { | ||
| event.sender.send('get-auto-start-enabled-response', configVal); | ||
| return; | ||
| } | ||
|
|
||
| const loginSettings = app.getLoginItemSettings(); | ||
| const isEnabled = loginSettings.openAtLogin; | ||
|
|
||
| userConfig.set('autoStart', isEnabled); |
There was a problem hiding this comment.
same comment, if autoStart is not required to be set and loginSettings.openAtLogin can be our source of truth, we should only rely on it
Adds auto start option to user settings. As linux auto start is unsupported by electron, toggling the setting will show a modal explaining it doesnt work on linux.
resolves #1413