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

Send Wasabi to Tray on Closing #6518

Merged
merged 11 commits into from
Oct 18, 2021
Merged

Conversation

molnard
Copy link
Contributor

@molnard molnard commented Oct 13, 2021

Closes: #4790, #4798
Related: #4591

@molnard molnard marked this pull request as ready for review October 13, 2021 11:52
adamPetho
adamPetho previously approved these changes Oct 13, 2021
Copy link
Contributor

@adamPetho adamPetho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK on Win10, macOS Big Sufur 11.5.2 and Ubuntu 20.04 🎉

Copy link
Contributor

@MaxHillebrand MaxHillebrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested 26fe35b on Qubes 4 Debian 11 with i3wm

when pressing Power+shift+q [i3wm shortcut for window close], I get this debug, and GUI keeps running.
[1] DEBUG HideShowBehavior:OnAttached (52) Closing event, cancellation of the close is set to: 'False'.

if I press Ctrl+c in the terminal that runs dotnet, then Wasabi shutsdown gracefully.


So far, shutting down GUI & keeping running in background does not work for i3wm.
[i know, not supported, not needed, just fyi.]

@adamPetho
Copy link
Contributor

when pressing Power+shift+q [i3wm shortcut for window close], I get this debug, and GUI keeps running.

Yep, we are aware of that issue. It's reproducible on master too. Only in Debug mode tho.

yahiheb
yahiheb previously approved these changes Oct 13, 2021
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK on Win10

@MaxHillebrand
Copy link
Contributor

MaxHillebrand commented Oct 13, 2021

fust fyi, this does work in i3wm in general. For example, with slack or telegram. Power+Shift+q shuts down the GUI, but the tray icon persists. When clicking on the tray icon, a new window pops up super quickly.

It would be pretty cool to get that in Wasabi too actually. 🖤

@Szpoti
Copy link
Contributor

Szpoti commented Oct 13, 2021

tACK on Win10 and Ubuntu 18.04.
Just an intersting NIT on Win10, if I open up the tray menu, right-click on Wasabi, then click out to the desktop, only Wasabi's menu gets hidden, the tray menu stays open for a short time

Step1

Step2

@molnard
Copy link
Contributor Author

molnard commented Oct 13, 2021

[1] DEBUG HideShowBehavior:OnAttached (52) Closing event, cancellation of the close is set to: 'False'.

@MaxHillebrand That means there was no cancellation on the quit procedure - so basically the logic there let the software quit, it is not prevented - so my conclusion is that this is not related to this PR.

Can you check it on master? It is important to separate otherwise this PR tries to solve more problem that it should.

@molnard molnard mentioned this pull request Oct 13, 2021
24 tasks
@molnard molnard dismissed stale reviews from yahiheb and adamPetho via d9111f6 October 13, 2021 15:11
yahiheb
yahiheb previously approved these changes Oct 13, 2021
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

@yahiheb
Copy link
Collaborator

yahiheb commented Oct 14, 2021

Should the default behavior be True? That is not the expected behavior so IMO maybe not.

@molnard
Copy link
Contributor Author

molnard commented Oct 14, 2021

Most of the software behaves like that. @danwalmsley what do you think? Should we Hide on close by default? Keep in mind that we are aiming at users who will never change the settings.

Co-authored-by: adamPetho <45069029+adamPetho@users.noreply.github.com>
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than this #6518 (comment), tACK

@nopara73
Copy link
Contributor

cACK for default, very good.

@danwalmsley
Copy link

@molnard as discussed on Slack... my view is that when the user closes the window it absolutely should move to taskbar by default, and that this behavior is actually fundamental to the new WabiSabi and auto coinjoin feature. Users expect certain types of apps to stay running in the background, i.e. Slack, Telegram, many crypto wallets also, its normal and understood behavior.

We can use the technique where the first time someone does it, you popup a notice something like.

"Hey, im still running in the background!"

but IMO users will just understand it.

Copy link
Contributor

@adamPetho adamPetho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ShowCommand fires twice, but it can be fixed easily in another PR and it doesn't mess up anything, so tACK.

@molnard molnard merged commit a7ba08a into WalletWasabi:master Oct 18, 2021
@molnard molnard deleted the hideonclose2 branch October 18, 2021 10:30
@molnard molnard mentioned this pull request Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants