Page MenuHomePhabricator

[web] Add modals for updating the desktop app
ClosedPublic

Authored by michal on Jan 20 2023, 4:18 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 27, 3:45 PM
Unknown Object (File)
Wed, Apr 17, 9:48 AM
Unknown Object (File)
Wed, Apr 17, 9:48 AM
Unknown Object (File)
Wed, Apr 17, 9:47 AM
Unknown Object (File)
Wed, Apr 17, 9:47 AM
Unknown Object (File)
Wed, Apr 17, 9:47 AM
Unknown Object (File)
Wed, Apr 17, 9:42 AM
Unknown Object (File)
Tue, Apr 9, 7:08 AM
Subscribers

Details

Summary

Adds modals for updating the desktop app. There are two cases:

  • The user is using the first version of the app. We check if the version string exists and if not, we prompt the user to download manually a new (self-updating) version. This is shown on refresh.
  • If the user is on the self-updating version, after downloading the new version we ask if they want to restart the app

(both of them only show up when it's the desktop app not the web app)

More info here: https://linear.app/comm/issue/ENG-2661/

Test Plan
  • Check if the user is prompted to download the new version if there is no version string
  • Check if the user is prompted to restart after a new version is available and the update is applied correctly

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat requested changes to this revision.Jan 20 2023, 10:41 AM

Some questions

web/modals/update-modal.react.js
46 ↗(On Diff #21142)

We should add a comment explaining that this is only for the first upgrade

50 ↗(On Diff #21142)

How do we avoid showing this if the user has downloaded the new version?

74 ↗(On Diff #21142)

Is it possible for multiple handlers to be set if pushModal is changed? Should we return a cleanup function from this effect to remove the old handler when that happens?

This revision now requires changes to proceed.Jan 20 2023, 10:41 AM

Add comment

web/modals/update-modal.react.js
50 ↗(On Diff #21142)

The version property isn't available on old versions. So in the newer versions, the second part of the condition will return.

74 ↗(On Diff #21142)

This already happens, you can see the cleanup function in D6322 in preload.js:17.

I don't see the comment. Please add the comment before landing

This revision is now accepted and ready to land.Jan 23 2023, 10:34 AM