-
Notifications
You must be signed in to change notification settings - Fork 3k
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: silently update desktop app #40253
Changes from 13 commits
16c7b65
316552a
c556ae6
399d8f5
6b45aab
55a56b4
6b491b2
60b4d5d
dfc5f00
5d38bf4
e2a13e5
232838f
3baac62
a990d5b
202987e
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 |
---|---|---|
|
@@ -111,6 +111,7 @@ process.argv.forEach((arg) => { | |
// happens correctly. | ||
let hasUpdate = false; | ||
let downloadedVersion: string; | ||
let isSilentUpdating = false; | ||
|
||
// Note that we have to subscribe to this separately and cannot use Localize.translateLocal, | ||
// because the only way code can be shared between the main and renderer processes at runtime is via the context bridge | ||
|
@@ -127,16 +128,20 @@ const quitAndInstallWithUpdate = () => { | |
autoUpdater.quitAndInstall(); | ||
}; | ||
|
||
/** Menu Item callback to triggers an update check */ | ||
const manuallyCheckForUpdates = (menuItem: MenuItem, browserWindow?: BrowserWindow) => { | ||
/** Menu Item callback to trigger an update check */ | ||
const manuallyCheckForUpdates = (menuItem?: MenuItem, browserWindow?: BrowserWindow) => { | ||
// Disable item until the check (and download) is complete | ||
// eslint: menu item flags like enabled or visible can be dynamically toggled by mutating the object | ||
// eslint-disable-next-line no-param-reassign | ||
menuItem.enabled = false; | ||
if (menuItem) { | ||
// eslint-disable-next-line no-param-reassign -- menu item flags like enabled or visible can be dynamically toggled by mutating the object | ||
menuItem.enabled = false; | ||
} | ||
|
||
autoUpdater | ||
.checkForUpdates() | ||
.catch((error) => ({error})) | ||
.catch((error) => { | ||
isSilentUpdating = false; | ||
return {error}; | ||
}) | ||
.then((result) => { | ||
const downloadPromise = result && 'downloadPromise' in result ? result.downloadPromise : undefined; | ||
|
||
|
@@ -148,7 +153,7 @@ const manuallyCheckForUpdates = (menuItem: MenuItem, browserWindow?: BrowserWind | |
dialog.showMessageBox(browserWindow, { | ||
type: 'info', | ||
message: Localize.translate(preferredLocale, 'checkForUpdatesModal.available.title'), | ||
detail: Localize.translate(preferredLocale, 'checkForUpdatesModal.available.message'), | ||
detail: Localize.translate(preferredLocale, 'checkForUpdatesModal.available.message', {isSilentUpdating}), | ||
buttons: [Localize.translate(preferredLocale, 'checkForUpdatesModal.available.soundsGood')], | ||
}); | ||
} else if (result && 'error' in result && result.error) { | ||
|
@@ -172,6 +177,10 @@ const manuallyCheckForUpdates = (menuItem: MenuItem, browserWindow?: BrowserWind | |
return downloadPromise; | ||
}) | ||
.finally(() => { | ||
isSilentUpdating = false; | ||
if (!menuItem) { | ||
return; | ||
} | ||
// eslint-disable-next-line no-param-reassign | ||
menuItem.enabled = true; | ||
}); | ||
|
@@ -201,7 +210,7 @@ const electronUpdater = (browserWindow: BrowserWindow): PlatformSpecificUpdater | |
if (checkForUpdatesMenuItem) { | ||
checkForUpdatesMenuItem.visible = false; | ||
} | ||
if (browserWindow.isVisible()) { | ||
if (browserWindow.isVisible() && !isSilentUpdating) { | ||
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. @gijoe0295 I still think that we should show a notification to the user after updating version successfully cc @Beamanator 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. Hmm well this is why we're calling it "silent" updating I guess haha. At this point, if the user already clicked "Update" I'm not sure this would require another notification to the user -> Like on Web, we would just refresh & BAM it's updated, no extra notification 🤔 |
||
browserWindow.webContents.send(ELECTRON_EVENTS.UPDATE_DOWNLOADED, info.version); | ||
} else { | ||
quitAndInstallWithUpdate(); | ||
|
@@ -604,6 +613,15 @@ const mainWindow = (): Promise<void> => { | |
} | ||
}); | ||
|
||
// Automatically check for and install the latest version in the background | ||
ipcMain.on(ELECTRON_EVENTS.SILENT_UPDATE, () => { | ||
if (isSilentUpdating) { | ||
return; | ||
} | ||
isSilentUpdating = true; | ||
manuallyCheckForUpdates(undefined, browserWindow); | ||
}); | ||
|
||
return browserWindow; | ||
}) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
import {Linking} from 'react-native'; | ||
import CONST from '@src/CONST'; | ||
import ELECTRON_EVENTS from '@desktop/ELECTRON_EVENTS'; | ||
|
||
export default function updateApp() { | ||
Linking.openURL(CONST.APP_DOWNLOAD_LINKS.DESKTOP); | ||
window.electron.send(ELECTRON_EVENTS.SILENT_UPDATE); | ||
} |
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.
Move this line to the below code block
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.
Agreed
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.
@gijoe0295 Small change, please address it
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.
Updated!