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

Handle need restart after installation for winget #626

Merged
merged 4 commits into from
Jan 23, 2023

Conversation

panther7
Copy link
Contributor

@panther7 panther7 commented Jan 18, 2023

#625

Added some new texts, but question is, are we really need dialog?

@marticliment

@panther7 panther7 force-pushed the 625-need-restart branch 2 times, most recently from c204aee to fec96df Compare January 18, 2023 10:37
@marticliment
Copy link
Owner

Instead of showing a dialog, what do you think about showing only a push notification, with the globals.trayIcon.showMessage function?

@marticliment
Copy link
Owner

I think this dialogs might be a little bit intrusive, especially when bulk-updating packages

@panther7
Copy link
Contributor Author

I think this dialogs might be a little bit intrusive, especially when bulk-updating packages

I agree, but now, show "error" dialog.

Should I just rewrite it in a text message like "installed successfully"?

@marticliment
Copy link
Owner

marticliment commented Jan 19, 2023

The current behaviour is:

  1. If failed, show a message.
  2. If succeeded, throw a notification.

What I would do is change the text of that "{0} installation succeeded" for "A restart is required to finish {0} installation", as well as addding a visual indicator in the ui telling the user to restart its PC.

If you don't understand how the error message class works, (it is a little bit messy), just let me know and I'll implement this feature.

Don't worry for the visual indicator, since I am working on a base class for them, since I need to implement one for one of the chocolatey features.

@panther7 panther7 force-pushed the 625-need-restart branch 3 times, most recently from ca17bb5 to c80a21c Compare January 22, 2023 23:03
@panther7
Copy link
Contributor Author

panther7 commented Jan 22, 2023

Done, new strings:

  • {action} was successfully!
  • Restart your PC to finish installation

Code is now more "simplify".

@panther7 panther7 force-pushed the 625-need-restart branch 3 times, most recently from 3f09fe7 to a419e59 Compare January 23, 2023 10:42
@marticliment
Copy link
Owner

image
image

I have added new restart icons, so it looks visually different

@marticliment marticliment merged commit 4f146de into marticliment:main Jan 23, 2023
@panther7 panther7 deleted the 625-need-restart branch January 23, 2023 17:44
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

Successfully merging this pull request may close these issues.

2 participants