-
Notifications
You must be signed in to change notification settings - Fork 357
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
Move QuietBackgroundProcesses UI to Windows customization #2511
Conversation
#nullable enable | ||
private DevHome.QuietBackgroundProcesses.QuietBackgroundProcessesSession? _session; | ||
private QuietBackgroundProcessesSession? _session; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nullable is enabled in this project, you might want to review these #nullable statements. Not sure if disabling it is what you want.
I'm good with these changes as-is, but @EricJohnson327 mentioned here that, "The QuiteBackgroundProcesses stuff should go in its own tools/QuietBackgroundProcesses project. All code should be written as if it wasn't an experimental feature, then the experimental feature checks just wrap it to enable/disable it. That way when it's not experimental anymore, we just remove the checks and the rest of the code stays exactly the same." I'm signing off, but giving Eric a chance to respond (I'm not sure if what he's saying is possible for WindCust anyway). Thanks @nieubank! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private readonly TimeSpan _oneSecond = new TimeSpan(0, 0, 1); | ||
private readonly IExperimentationService _experimentationService; | ||
|
||
private readonly TimeSpan _zero = new(0, 0, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit: TimeSpan.FromSeconds(1)
<ctControls:SettingsCard x:Uid="QuietBackgroundProcessesExplanation" HorizontalContentAlignment="Left" > | ||
</ctControls:SettingsCard> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<ctControls:SettingsCard x:Uid="QuietBackgroundProcessesExplanation" HorizontalContentAlignment="Left" > | |
</ctControls:SettingsCard> | |
<ctControls:SettingsCard x:Uid="QuietBackgroundProcessesExplanation" HorizontalContentAlignment="Left" /> |
Summary of the pull request
Move QuietBackgroundProcesses UI to Windows customization instead of a top-level page.
References and relevant issues
Detailed description of the pull request / Additional comments
Merge the UI project for QuietBackgroundProcesses into Customization and control the view's visibility based on the experimental features enablement status.
Validation steps performed
With the feature disabled in Settings -> Experimental features navigate to Windows customization and see that the Quiet mode card is not visible.
Enable the feature and notice that the settings card is visible upon navigating back to Windows customization.
Start a session, navigate away from the page and back and ensure the session remains active.
Close Dev Home and verify that DevHome.QuietBackgroundProcesses.ElevatedServer.exe is still running, launch, navigate back to Windows customization and stop session. Confirm that everything cleans up.
PR checklist