Add prompt to warn before quitting the application

[germain-gg]

Fixes element-hq/element-web#16620

Related pull requests

• Add user settings for warn before exit matrix-org/matrix-react-sdk#5793
• Add user settings for warn before exit element-web#16781

The final copy is:

• Message: Are you sure you want to quit?
• No: Cancel
• Yes: Close Element

We originally wanted to implement a different UI, however due to the nature of JavaScript and how the keyup event is triggered, we had to choose this approach. View the original ticket if you are interested in the full context

I also wanted to add a checkbox letting the user opt-out from that warning dialog. This has proven quite difficult as we do have to use dialog.showMessageBoxSync and that this option seems to be only available with dialog.showMessageBox despite what the documentation says (I opened a ticket electron/electron#28394)

Lastly, there are no references to i18n in the element-desktop repo, and the menu items do not get translated either. To not drastically increase the scope of this ticket I intentionally left that out

[t3chguy]

Should this apply to all quits, e.g explicit ones via mouse click on context menu or just Cmd + Q ones? Chrome on Macos does not warn when using context menu

[t3chguy]

Also https://www.electronjs.org/docs/api/browser-window#event-close suggests that that it'll fire when the window is closed, so when minimizing to Dock. This sounds like clicking the red traffic light on the window will yield the prompt asking if you are sure you want to quit but clicking Yes will not actually Quit

[germain-gg]

I have tested the different scenario, and I can confirm that items below all trigger the modal to be displayed

• CommandOrControl+Q
• Context Menu > Quit Element
• Clicking the red traffic light

But after you mentioned this, I had a closer look, and I do believe that clicking on the red traffic light on Mac should hide the window rather than quitting.
I have just changed the code, switching the two if statement in the close event callback and it does work that way now

[turt2live]

this side lgtm

[t3chguy]

Context Menu > Quit Element

this triggering this warning doesn't feel right given it does not match behaviour in the app we are basing the behaviour off of and is generally an explicit enough interaction that it shouldn't need confirmation.
Equally this prompt would probably trigger on SIGTERM

[germain-gg]

Context Menu > Quit Element

this triggering this warning doesn't feel right given it does not match behaviour in the app we are basing the behaviour off of and is generally an explicit enough interaction that it shouldn't need confirmation.
Equally this prompt would probably trigger on SIGTERM

I have seen different programs treating this differently, from the behaviour I implemented to the one you're describing.
I'm not so opiniated one way or another. If we wanted to implement your suggestion we would use globalShortcuts

Will clarify that with the design team and move this forward with the most appropriate experience for Element