feat: implement desktop popup ui toggle#598
Conversation
| factoryReset: () => { | ||
| return ipcRenderer.invoke('factory-reset'); | ||
| }, | ||
| toggleDesktopPopup: async (isEnabled: boolean) => { |
There was a problem hiding this comment.
Dialogs are triggered from main process.
The reason behind exposing it on the renderer is that depending on the selected yes or no answer, we either trigger redux action. It would also be possible to fire an event from main to renderer to execute a redux action, but that would mean a full round trip which we don't want to do.
3fb7570 to
c65df16
Compare
packages/app/src/app/desktop-app.ts
Outdated
|
|
||
| ipcMain.handle('toggle-desktop-popup', (_event, isEnabled) => { | ||
| if (cfg().enableDesktopPopup && isEnabled) { | ||
| this.enableDesktopPopup(); |
There was a problem hiding this comment.
The enableDesktopPopup is an async method. We should add a catch to handle any error, otherwise we will have unhandledPromiseRejections and the App will crash.
There was a problem hiding this comment.
Ok if we follow up, this method is used as async just because of createApprovalWindow. But neither createMainWindow nor createApprovalWindow needs to be async. So theoretically there will never be an unhandledPromiseRejections
Ok, BrowserWindow.loadFile is actually async.
packages/app/src/app/desktop-app.ts
Outdated
| defaultValue: false, | ||
| }); | ||
| if (cfg().enableDesktopPopup && isDesktopPopupEnabled) { | ||
| this.enableDesktopPopup(); |
There was a problem hiding this comment.
Same as before, this is an async method. We should wait for the Promise to resolve (and maybe add a try/catch block around so that any issues from enabling the desktop popup do not prevent the App from initialise)
bdbea58 to
1a7dec5
Compare
c41c2b0 to
0f22385
Compare
1ac9e03 to
f16d159
Compare
Overview
This PR implements desktop popup ui toggle.