Skip to content

Handle need restart after installation for winget#626

Merged
Martí Climent (marticliment) merged 4 commits intoDevolutions:mainfrom
panther7:625-need-restart
Jan 23, 2023
Merged

Handle need restart after installation for winget#626
Martí Climent (marticliment) merged 4 commits intoDevolutions:mainfrom
panther7:625-need-restart

Conversation

@panther7
Copy link
Copy Markdown
Contributor

@panther7 Filip Mösner (panther7) commented Jan 18, 2023

#625

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

Martí Climent (@marticliment)

@panther7 Filip Mösner (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
Copy Markdown
Collaborator

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

@marticliment
Copy link
Copy Markdown
Collaborator

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

@panther7
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

Martí Climent (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 Filip Mösner (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
Copy Markdown
Contributor Author

Filip Mösner (panther7) commented Jan 22, 2023

Done, new strings:

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

Code is now more "simplify".

@panther7 Filip Mösner (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
Copy Markdown
Collaborator

image
image

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

@marticliment Martí Climent (marticliment) merged commit 4f146de into Devolutions:main Jan 23, 2023
@panther7 Filip Mösner (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

Development

Successfully merging this pull request may close these issues.

2 participants