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

Fixes the ability to restart with a message #191

Merged
merged 8 commits into from
Jun 28, 2021

Conversation

dalyIsaac
Copy link
Member

@dalyIsaac dalyIsaac commented Feb 22, 2021

This fixes the following issues related to waking from sleep:

  • Missing window handle
  • Object disposed
  • Display settings changed
  • TextBlockMessage didn't display buttons

Missing window handle

When the OS wakes from sleep, sometimes the BarForm will try to redraw the window, even though the window handle has not yet been created (presumably it was destroyed as part of going to sleep):
Exception thrown when the window handle has not yet been created

Object disposed

Sometimes after waking from sleep, BarForm has been disposed of.
System.ObjectDisposedException

Display settings changed

Sometimes when the OS wakes from sleep, it thinks that the display settings have changed, which triggers ConfigContext::HandleDisplaySettingsChanged https://github.com/rickbutton/workspacer/blob/d9da742fb684eb3db38e770b39c24b46e8c1193e/src/workspacer/ConfigContext.cs#L52

HandleDisplaySettingsChanged sends the action LauncherAction.RestartWithMessage to the workspace watcher.
https://github.com/rickbutton/workspacer/blob/d9da742fb684eb3db38e770b39c24b46e8c1193e/src/workspacer/ConfigContext.cs#L223-L233

The workspace watcher should create a TextBlockMessage:
https://github.com/rickbutton/workspacer/blob/cd334dca261cba1181783a8e26a9350e97f5aa3a/src/workspacer.Watcher/Program.cs#L58-L62

https://github.com/rickbutton/workspacer/blob/cd334dca261cba1181783a8e26a9350e97f5aa3a/src/workspacer.Watcher/Program.cs#L132-L140

The TextBlockMessage is not opened as HandleDisplaySettingsChanged calls CleanupAndExit immediately after sending LauncherAction.Restart.

CleanupAndExit calls Environment.Exit, which also kills the workspace watcher.

https://github.com/rickbutton/workspacer/blob/e0e3c0607082926d7dca8c5d41325c057f6017ba/src/workspacer/ConfigContext.cs#L193-L198

Removing Environment.Exit does not prevent the watcher from being shutdown when the user wants to quit - that is handled by LauncherAction.Quit:

https://github.com/rickbutton/workspacer/blob/e0e3c0607082926d7dca8c5d41325c057f6017ba/src/workspacer/ConfigContext.cs#L169-L178

Application.Exit should suffice for stopping all threads in the main workspacer process, according to the Application.Exit documentation.

TextBlockMessage didn't display buttons

The buttons for TextBlockMessage would only be displayed after going full screen. Updating the dimensions and locations of controls in TextBlockMessage resolves this.

P.S.

This may also fix #37.

@josteink
Copy link
Member

This seems like a really well meant and well researched issue, but I really don't have anywhere to test these changes (I don't have a laptop running Windows anywhere).

I'd appreciate it if someone else could test these changes.

Cc: @N1x0 @rickbutton

@dalyIsaac
Copy link
Member Author

A quick way to test HandleDisplaySettingsChanged is to open the Display page in the Windows Settings app. If you have multiple monitors, you can slightly change a monitor's position and press
Apply". That triggers HandleDisplaySettingsChanged.

p.s. @josteink thanks for reviewing all these PRs even though you don't use Windows anymore 🙏.

@josteink
Copy link
Member

PRs haven’t been merged in months and Rick haven’t been answering emails either.

Right now it seems a fork is needed to keep workspacer going forwards.

I’m discussing this in a issue here: #211. Feel free to let me know your thoughts.

Copy link
Contributor

@N1x0 N1x0 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@sitiom sitiom left a comment

Choose a reason for hiding this comment

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

I put a comment on the empty try-catch block. Otherwise, LGTM!

@sitiom sitiom merged commit bf36598 into workspacer:master Jun 28, 2021
@dalyIsaac dalyIsaac deleted the wake-from-sleep branch June 28, 2021 08:02
@dalyIsaac dalyIsaac added this to the 0.9.11 milestone Jun 29, 2021
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.

On restart, something went wrong and permanently hid some of my windows :(
4 participants