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

Add feature to disable theme switching while certain processes are running #734

Merged
merged 6 commits into from
Jun 8, 2023

Conversation

pohuing
Copy link
Contributor

@pohuing pohuing commented Jun 5, 2023

This is a rough fix for #733 currently it can

  • show a list of processes
  • add a process to a block list
  • check for active processes to postpone switching

I'm opening this PR early before I'm done because I'm a bit lost with regards to the following topics.

What's missing as far as I can tell is

  • localization
    • This would happen eventually via Weblate?
  • A better display for excluded processes
    • Currently it uses a DataGrid, I would love something that looks similar to the Windows' settings
      image

    • Not sure how to implement that however

  • An option to remove entries
    • Related to the prior point

What I'm not sure about is the current way to display a list of active processes. I filter for processes with an active handle and a window. This rules out about 100 instances of svchost, but not everything I wouldn't consider an application the user interacts with. For instance ApplicationFrameHost, svchost, and TextInputHost below:
image
Furthermore I currently require the user to refresh a process list because

  • I'm not sure how expensive getting the entire process list is
  • Automatically reordering lists while the user is interacting with them is terrible UX
    maybe there's a better solution I'm not aware of.

Then there's the behavior of the ProcessBlockModule. Running the AutoDarkMode service seems to switch themes immediately, without waiting for the module to be initialized. If I instead start the application and have it wait for a custom time in the near future my Module seems to work fine.

I would appreciate it if someone more experience than me with .NET and WPF could help me out here. Thanks in advance!

…nning

Adds Panel in SwitchModes Page
Adds Module to postpone switching while some processes are running
Adds Config to store process block list
Automatically refresh process list on opening dropdown
Improve UI of listing for blocked tasks
Localize missing strings
@pohuing
Copy link
Contributor Author

pohuing commented Jun 6, 2023

I've figured out a way to display the block list in a prettier manner and added the option to remove entries. It's not super pretty but my lack of taste in design fits my profession 😆. The thing now looks like this:
image
Opening the ComboBox now refreshes the list of active processes, this causes some noticeable lag for about a quarter of a second, but I think is overall easier to use than having to manually refresh.
"Entfernen" is German for Remove. I believe my localization works for German and English systems.

I'm still unsure if my service module works correctly or not, I'd be more comfortable if someone could verify that before I'd consider this ready to merge. Are there tests I could expand to cover my code?

@Spiritreader
Copy link
Member

Spiritreader commented Jun 7, 2023

Hey, thanks for working on this!
Unfortunately I'm not a WPF developer either, I sort of hacked in the current look of the UI.

I think going for a function first approach similar to how you did it is totally fine for now.

If you have any questions about the backend, feel free to reach out.
I'll probably be able to get back to you tomorrow because of the holiday.

I'm not sure how expensive getting the entire process list is

In the front end it should be fine refreshing every few seconds or so while the UI section is expanded/open.
That or you hook it into the open event of the dropdown box.

Running the AutoDarkMode service seems to switch themes immediately, without waiting for the module to be initialized. If I instead start the application and have it wait for a custom time in the near future my Module seems to work fine.

That is somewhat of a design limit currently because a theme switch request is fired immediately after the application starts, which may or may not be before all modules have finished setting their postpone state.
You don't have to worry about that, I'll look into this at some point as it's more of an architectural thing.

Spiritreader added a commit that referenced this pull request Jun 8, 2023
@Spiritreader
Copy link
Member

I've added a ThemeSwitchApproaching proprty to the globalstate which you can use do determine whether a module is allowed to start running. There's an example in the GPUMonitorModule

if (!State.ThemeSwitchApproaching && !MonitoringActive)

Should be a lot more streamlined now and is governor independent.

Spiritreader added a commit that referenced this pull request Jun 8, 2023
@Spiritreader
Copy link
Member

Spiritreader commented Jun 8, 2023

In addition, there's also the necessity to register modules that hook into that mechanism via the module's enable hook

And the disable hook

public override void DisableHook()

@pohuing
Copy link
Contributor Author

pohuing commented Jun 8, 2023

I believe this feature now works as intended. Or at least it works on my end and doesn't seem to do unnecessary process checks.
I decided not include my own monitoring flag, instead just checking if I currently postpone based on the postpone reason.

I think that should be fine since I don't expect there to be a whole lot of postpones in the queue.

@pohuing
Copy link
Contributor Author

pohuing commented Jun 8, 2023

The filtering for the dropdown is still a bit weird, listing processes I wouldn't consider relevant for the feature. I'll look around how other applications handle it but this is a cosmetic issue and does not impact functionality nor usability significantly.

@Spiritreader Spiritreader merged commit b81401e into AutoDarkMode:master Jun 8, 2023
@Spiritreader
Copy link
Member

Spiritreader commented Jun 8, 2023

I decided not include my own monitoring flag, instead just checking if I currently postpone based on the postpone reason.
I think that should be fine since I don't expect there to be a whole lot of postpones in the queue.

That is correct. I usually use the monitoring flag because it's easier to read and allows for better logging, but using the postpone queue is also absolutely fine.

I believe this feature now works as intended.

Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to disable theme switching while some process is running.
2 participants