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

[VDG] Temporarily disabling the running in background on macOS #11595

Merged

Conversation

wieslawsoltes
Copy link
Collaborator

Temporary fix until we can #8151 available in Avalonia.

Based on comment: #8151 (comment)

Add static property in App class called EnableFeatureHideOnClose to disable HideOnClose functionality (from UI and app state manager).

Copy link
Collaborator

@turbolay turbolay left a comment

Choose a reason for hiding this comment

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

tACK. I simply tested the "feature", not thoroughly reviewed the code.

Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

I am wondering about the backward compatibility... what will happen with those users who had the Start in the background feature enabled by default?

ApplicationStateManager L35, the initial state will be closed so the client will just close immediately? (I don't have mac, cannot test it)

@turbolay
Copy link
Collaborator

ApplicationStateManager L35, the initial state will be closed so the client will just close immediately? (I don't have mac, cannot test it)

Yes, I tested to enable the feature on master, then checkout and run this PR, and it was disabled. Then checkout back master and the feature was enabled again.

But because you mentioned it, I tried the other way around. Disable on master, go to the PR, and go back on master. Back on master, feature activates itself, even if it was not enabled in the first place. So this might be an UX issue: Users willingly disabled the feature but then out of a sudden it gots reactivated. We can also see it as a nice way to notify them in the future that the feature is fixed.

@wieslawsoltes
Copy link
Collaborator Author

I am wondering about the backward compatibility... what will happen with those users who had the Start in the background feature enabled by default?

ApplicationStateManager L35, the initial state will be closed so the client will just close immediately? (I don't have mac, cannot test it)

I guess when we reenable the future we should handle that scenario on the first run.

Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

Is the "Hide" from tray removed too?

@@ -20,6 +21,14 @@ public class App : Application
private readonly Func<Task>? _backendInitialiseAsync;
private ApplicationStateManager? _applicationStateManager;

public static bool EnableFeatureHideOnClose { get; private set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this name should be EnableFeatureHide because it should affect every hide related thing, and not just the HideOnClose

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pushed changes

WalletWasabi.Fluent/ApplicationStateManager.cs Outdated Show resolved Hide resolved
@wieslawsoltes
Copy link
Collaborator Author

Is the "Hide" from tray removed too?

@soosr this one is tricky as it does not have IsVisible property

@wieslawsoltes
Copy link
Collaborator Author

Is the "Hide" from tray removed too?

@soosr Found a way to do this a878ab1

Copy link
Collaborator

@soosr soosr left a comment

Choose a reason for hiding this comment

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

utACK

@soosr
Copy link
Collaborator

soosr commented Oct 4, 2023

@wieslawsoltes Could you fix CF?
@turbolay Could you do a final test?

@wieslawsoltes
Copy link
Collaborator Author

@wieslawsoltes Could you fix CF? @turbolay Could you do a final test?

@soosr I already fixed, CF has wrong commit

Copy link
Collaborator

@turbolay turbolay left a comment

Choose a reason for hiding this comment

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

tACK

@soosr soosr merged commit 41f3f39 into WalletWasabi:master Oct 4, 2023
6 of 7 checks passed
@wieslawsoltes wieslawsoltes deleted the vdg/DisableHideOnCloseMacOSX branch October 5, 2023 19:49
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.

None yet

3 participants