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

Hotkey to cycle to the next power schema #32

Conversation

objecttothis
Copy link

  • Refactored variables to fix typos
  • Added function to PowerManager to cycle to the next power schema
  • Added Configuration variables for the Cycle to the next schema hotkey toggle and status
  • Added app string to explain the setting
  • Added business logic to register and unregister the hotkey when enabled or disabled
  • Added business logic to register and unregister the hotkey from the xml config file
  • Added business logic to fire cycling to the next power schema on hotkey press
  • Removed unneeded whitespace in the code
  • Removed unused "using" statements

- Refactored variables to fix typos
- Added function to PowerManager to cycle to the next power schema
- Added Configuration variables for the Cycle to the next schema hotkey toggle and status
- Added app string to explain the setting
- Added business logic to register and unregister the hotkey when enabled or disabled
- Added business logic to register and unregister the hotkey from the xml config file
- Added business logic to fire cycling to the next power schema on hotkey press
- Removed unneeded whitespace in the code
- Removed unused "using" statements
newHotKey.HotKeyFired += (this.MainWindow as MainWindow).ToggleWindowVisibility;

if (isCycleNextSchemaSwitch)
newHotKey.HotKeyFired += CycleNextPowerSchema;
Copy link
Author

@objecttothis objecttothis May 29, 2021

Choose a reason for hiding this comment

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

@petrroll The hotkey press is correctly switching the schema to the next power schema. The problem is that it doesn't show you what schema it switched you to. What I would like to do is modify MainWindow.ToggleWindowVisibility() to sort of act as a toast notification and disappear on its own after, say, 5 seconds. I can call `newHotKey.HotKeyFired += (this.MainWindow as MainWindow).ToggleWindowVisibility; as it currently is, and it shows the new schema, but it doesn't hide the window unless the user manually clicks somewhere on the screen. I played around with adding an optional timeout parameter but Visual Studio complained about that. I also couldn't figure out how to trigger the HideWithAnimation() function after the timeout period. I'm open to ideas about the best way to implement some sort of indicator notification of what power schema it just got changed to and then letting it disappear... if there is somewhere in the code where you are already doing this, please let me know.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, I vaguely remember the fly-out logic was somehow buggy w.r.t. to timeout parameters. The best way to approach it might be to use winrt API for proper notifications. I.e. instead of creating your own toast-like notification just use native Windows Notification API.

Not sure how much identity the current packaging schema provides and what's the state of invoking notifications without it, but remember that https://github.com/File-New-Project/EarTrumpet was doing something with winRT APIs so it might be a good place to start. Or you might wait until project reunion comes with notifications API (won't need app identity at all): https://github.com/microsoft/ProjectReunion

@petrroll
Copy link
Owner

Hi!
First off, amazing work! Happy to see that my code is still useful after all this time. And super-happy to see this high-quality contribution. Unfortunately, I'll not be merging the PR at this moment, mainly for two reasons.

  1. If I were to create a new version, there are many things that should also be updated. Among other things the fly-out logic deserves an upgrade, ideally to whatever ear-trumpet is currently using, the tray-icon/menu logic should be updated, support for light theme should be added, move to .NET Core should happen, move to the modern support for packaging apps should happen, ... .

  2. Even if I were ok creating a new version without 1) my whole deploy pipeline is not working right now. And for reasons outlined in Not in active development (since Sep2017) #23 I can't justify spending time on it. There's a chance I might have some time during summer to properly update this project as vacations are unlikely to happen (thanks covid) but can't and don't want to promise anything.

Given I won't be merging this change (despite it being super cool and good quality) into master. If someone wants to build their own version with your update they can just just clone your fork and build it locally.

Alternatively, I'd be happy to create a separate branch for this feature on this repo and you can merge into that one. I'll remember to keep it in sync with master if I ever come back to developing this.

Or, I'm also perfectly fine with you creating proper fork and even releasing it to Windows store.

I hope you understand. :)

@objecttothis
Copy link
Author

Hi!
First off, amazing work! Happy to see that my code is still useful after all this time. And super-happy to see this high-quality contribution. Unfortunately, I'll not be merging the PR at this moment, mainly for two reasons.

1. If I were to create a new version, there are many things that should also be updated. Among other things the fly-out logic deserves an upgrade, ideally to whatever ear-trumpet is currently using, the tray-icon/menu logic should be updated, support for light theme should be added, move to .NET Core should happen, move to the modern support for packaging apps should happen, ... .

I completely understand. With my own work, I'm always finding things that I wish I could figure out how to clone myself over and work on.

2. Even if I were ok creating a new version without 1) my whole deploy pipeline is not working right now. And for reasons outlined in #23 I can't justify spending time on it. There's a chance I might have some time during summer to properly update this project as vacations are unlikely to happen (thanks covid) but can't and don't want to promise anything.

I understand. I saw that it had not been in development for some time, so I knew there was a chance that it would be an exercise in learning. I was feeling inspired to code on a non-work project at the time.

Given I won't be merging this change (despite it being super cool and good quality) into master. If someone wants to build their own version with your update they can just just clone your fork and build it locally.

Alternatively, I'd be happy to create a separate branch for this feature on this repo and you can merge into that one. I'll remember to keep it in sync with master if I ever come back to developing this.

I think at this point, I'd recommend creating a branch on this repo and merging this into that branch. That way, if you or someone else ever picks the project back up, they can use the functionality if they want...

Or, I'm also perfectly fine with you creating proper fork and even releasing it to Windows store.

I hope you understand. :)

Thanks for taking the time to look at the PR.

@petrroll petrroll changed the base branch from master to contribPRAfterActiveDev/cycleNextSchemaHotkey June 1, 2021 20:48
@petrroll petrroll merged commit 9acf931 into petrroll:contribPRAfterActiveDev/cycleNextSchemaHotkey Jun 1, 2021
@petrroll
Copy link
Owner

petrroll commented Jun 1, 2021

I think at this point, I'd recommend creating a branch on this repo and merging this into that branch. That way, if you or someone else ever picks the project back up, they can use the functionality if they want...

Did just that :)

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.

2 participants