Skip to content
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

[Design] Notify that there is an update (and maybe make a distinction between recommended and required update) #102

Open
msbrogli opened this issue Apr 7, 2020 · 7 comments
Assignees

Comments

@msbrogli
Copy link
Member

msbrogli commented Apr 7, 2020

Verification

We will use the GitHub API to find the latest signed release of a repository (see the docs here) that is not a draft and not a prerelease.

The following block in the body of the releases will be used to indicate extra headers, such as the type of release (i.e. recommended, required, or security).

```headers
Type: Required
Show-Security-Alert: True
```

For now, only the type header will be available. The keys of the headers will be case insensitive.

Scheduler

This verification should be executed after the wallet is started, and then automatically every X minutes.

UX

When a new version is detected, we should show an alert bar saying "A new version of Hathor Wallet is available. Click here for more information". When clicked, it should open a NewVersionAvailable modal.

We should also show that a new version is available in the About menu and in the Settings screen.

The NewVersionAvailableModal should have the title "An update is available" and an explanation of the new version. It should have two buttons: "Close" and "Download it", and the latter should open the html_url given by the GitHub API.

The NewVersionAvailableModal may also have a scrollable text with border and white background showing the change logs of all versions between current version and the latest version.

Example of explanation:

"""
Hathor Wallet v0.15.0 for macOS, Windows, and Linux has been released. We recommend you to update your wallet as soon as possible.

You should always update your wallet to have access to all new features, improvements, and fixes.

(if it's been a long time) Your wallet is outdated, and it may stop working anytime. We highly recommend you to update it right now.
"""

@msbrogli msbrogli self-assigned this Apr 7, 2020
@pedroferreira1
Copy link
Member

I like this design and if we decide to do this we could think about implementing the electron autoUpdater (https://www.electronjs.org/docs/api/auto-updater). This would be the best, in my opinion. For what I've searched it does not work on Linux, so we would need a secondary solution anyway.

Only one thing, we don't have a 'About menu' as described in the design.

@jansegre
Copy link
Member

jansegre commented Apr 7, 2020

I understood what it means to include the headers in the body after reading the Github docs, but the format isn't too clear for me.

I think we should consider multiple new versions. In that case we should join all changelogs and properly calculate the "required" and "security-alert" properties (if any update in the chain has a property, the whole chain will have that property, i.e. [0.4.1 (no props), 0.4.2 (security), 0.5.0 (required), 0.5.1 (no props)] should collapse to 0.5.1 (security and required, and the whole changelog joined).

Also we may want to allow skipping the notification, in that case the wallet would have to remember that option and possibly flush it after an update.

@msbrogli
Copy link
Member Author

msbrogli commented Apr 8, 2020

I like this design and if we decide to do this we could think about implementing the electron autoUpdater (https://www.electronjs.org/docs/api/auto-updater). This would be the best, in my opinion. For what I've searched it does not work on Linux, so we would need a secondary solution anyway.

@pedroferreira1 I didn't understand whether the autoUpdater is really installing the new wallet or just updating the javascript inside the container. If it updates the javascript, I think we shouldn't do it.

Only one thing, we don't have a 'About menu' as described in the design.

@pedroferreira1 There is one in the app. Maybe it is automatically generated.

@msbrogli
Copy link
Member Author

msbrogli commented Apr 8, 2020

I understood what it means to include the headers in the body after reading the Github docs, but the format isn't too clear for me.

@jansegre The format is like HTTP headers, with keys and values separated by a semicolon.

I think we should consider multiple new versions. In that case we should join all changelogs and properly calculate the "required" and "security-alert" properties (if any update in the chain has a property, the whole chain will have that property, i.e. [0.4.1 (no props), 0.4.2 (security), 0.5.0 (required), 0.5.1 (no props)] should collapse to 0.5.1 (security and required, and the whole changelog joined).

@jansegre I wasn't thinking about showing the changelog in the NewVersionAvailableModal, but we could.

Also we may want to allow skipping the notification, in that case the wallet would have to remember that option and possibly flush it after an update.

@jansegre We can save which version should be skipped. So, it would notify when new versions are released. What do you think?

@jansegre
Copy link
Member

Just noticed that this is a duplicate of #26, which I'll close because this one already has a proposal and more discussion.

@pedroferreira1
Copy link
Member

pedroferreira1 commented Apr 21, 2020

@pedroferreira1 I didn't understand whether the autoUpdater is really installing the new wallet or just updating the javascript inside the container. If it updates the javascript, I think we shouldn't do it.

I think it installs the new wallet but we need to test this. We should study how other apps do this with electron. I think it's better than just start doing it. Did you search for other electron apps to see how they do it?

@pedroferreira1 There is one in the app. Maybe it is automatically generated.

That's true, I forgot about that one. It's not automatically generated, we control that, so we can show the information there.

@jansegre
Copy link
Member

I think we should show a changelog from the newest version to the currently running version. I'm not sure if this is the intention. Otherwise I like the design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants