Throw toast when app is installed #1456
Conversation
Co-authored-by: Cassidy James Blaede <[email protected]>
Co-authored-by: Cassidy James Blaede <[email protected]>
|
Screenshot from list view
|
|
I will look into hiding the toast on the app's own app info view. |
src/Application.vala
Outdated
| var toast = main_window.toast; | ||
| toast.title = _("“%s” has been installed").printf (package.get_name ()); | ||
|
|
||
| toast.send_notification (); |
There was a problem hiding this comment.
It would be better to handle sending the toast and saving the package reference inside MainWindow, instead of trying to expose last_installed_package in Application and then accessing it back in MainWindow.
Simply declare a private AppCenterCore.Package? last_installed_package member and a public void send_installed_toast (AppCenterCore.Package package) method in MainWindow and manage sending the toast there instead.
There was a problem hiding this comment.
I now also need selected_package in AppCenter.App to check if a Toast should be sent.
There was a problem hiding this comment.
I mean sure, but that didn't resolve my issue. The toast is still half managed in Application and MainWindow at the same time. And there's also no need to store selected_package globaly inside Application. Notice how MainWindow actually calls show_package and return_clicked where this property could be set within MainWindow as well.
There was a problem hiding this comment.
Thanks for your feedback. I have refactored that except for selected_package.
The show_package call in MainWindow is not the same as in View.
There was a problem hiding this comment.
I refactored it using a signal.
@donadigo thanks for being persistent 😃 The code looks much cleaner now 👍
There was a problem hiding this comment.
Again, UX-wise this meets my approval. 😄 But I'd like to see a re-review from @elementary/desktop-developers or @donadigo.
| try { | ||
| last_installed_package.launch (); | ||
| } catch (Error e) { | ||
| warning ("Failed to launch %s: %s".printf (last_installed_package.get_name (), e.message)); |
There was a problem hiding this comment.
Hm, I wonder if this should throw a dialog. If a user clicked the open button and then nothing happened, that seems bad. Thoughts @cassidyjames?
There was a problem hiding this comment.
For reference: This code is mostly a copy from AbstractAppContainer.launch_package_app()
There was a problem hiding this comment.
@danrabbit I'm indifferent. It probably makes sense to do what we do in other instances in AppCenter, and then we can file an issue to throw a dialog on failure.
Co-authored-by: Daniel Foré <[email protected]>
Closes #1455