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

Restore WindowState when activating #10119

Merged

Conversation

gerhardol
Copy link
Member

@gerhardol gerhardol commented Jul 31, 2022

Fixes #9752

Proposed changes

This is a fix, probably not the correct solution, added for review.

Occasionally when restoring GE from minimized, the app fail to stay open.

One drawback with the fix is that rightclick to close on the minature in the taskbar requires that the app is not minimized. OK to select the cross.

Test methodology

Manual

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).


✒️ I contribute this code under The Developer Certificate of Origin.

@ghost ghost assigned gerhardol Jul 31, 2022
@RussKie
Copy link
Member

RussKie commented Aug 1, 2022

This does feel like a hack...

@gerhardol
Copy link
Member Author

This does feel like a hack...

It certainly is (both WIP and draft), I could not find the core issue. I assume this is something in .NET.

But this is so annoying, if there is no other solution I want to include this (will use it in my private builds).

@RussKie
Copy link
Member

RussKie commented Aug 4, 2022 via email

@gerhardol
Copy link
Member Author

If this is addressing the issue - let's take it

For me it is and I am using it.
But it is a hack, I have not found any other reports with similar issues. I hope that someone with WinForms knowledge could investigate a little about this...

For the 4.0 alpha, at least a trace printout should be added. It may be good to find how many users that experience the problem.

@gerhardol gerhardol added this to the 4.0.0 milestone Aug 7, 2022
@gerhardol gerhardol force-pushed the feature/i9752-restore-from-taskbar branch from 0785859 to 1e4a5c3 Compare August 8, 2022 20:16
@gerhardol gerhardol changed the title WIP Restore WindowState when activating Restore WindowState when activating Aug 8, 2022
@gerhardol gerhardol marked this pull request as ready for review August 8, 2022 20:16
@gerhardol
Copy link
Member Author

If this is addressing the issue - let's take it

I added a Trace printout and a comment (as well as commit message update).
I suggest we include this in next alpha/beta.

@gerhardol
Copy link
Member Author

When this "correction" kicks in, GE is restored also if it was maximized prior to the minimize. It is a little unexpected.
This could maybe be improved by using _windowPositionManager similar to RestorePosition(), but I want to keep this simple and still believe that there is a better solution...

@mstv
Copy link
Member

mstv commented Aug 21, 2022

When this "correction" kicks in, GE is restored also if it was maximized prior to the minimize. It is a little unexpected.

more than unexpected -- and occurs several times a day in contrast to #9752, which I cannot reproduce.

Though when trying your steps to reproduce, I noticed that the window positions are not stored correctly when closed minimized, at least the maximized state.
Perhaps, we should not store them at all if minimized (and keep the previously stored positions).

@gerhardol
Copy link
Member Author

more than unexpected -- and occurs several times a day in contrast to #9752, which I cannot reproduce.

Then you may have some part of the problem, but Minimized is changed later?

Though when trying your steps to reproduce, I noticed that the window positions are not stored correctly when closed minimized, at least the maximized state. Perhaps, we should not store them at all if minimized (and keep the previously stored positions).

seem reasonable

@gerhardol
Copy link
Member Author

Maybe the state can be saved when minimizing, I have not found the win call for this

Though when trying your steps to reproduce, I noticed that the window positions are not stored correctly when closed minimized, at least the maximized state. Perhaps, we should not store them at all if minimized (and keep the previously stored positions).

I do not see this. When saving in Minimized or Maximized state, the RestoreBounds are used, which are OK for me.

gerhardol and others added 2 commits September 4, 2022 10:01
Changed behavior from .NET4 to .NET5, application requires explicit
"restore" in Taskbar.
This is a raw workaround with the minor annoyance that closing in taskbar
requires application activation.
Changed behavior from .NET4 to .NET5, application requires explicit
"restore" in Taskbar.
This is a raw workaround with the minor annoyance that closing in taskbar
requires application activation.

The behavior is called by setting WorkaroundRestoreFromMinimize
that must be manually set in GitExtensions.settings
@gerhardol gerhardol force-pushed the feature/i9752-restore-from-taskbar branch from 1e4a5c3 to 900ef40 Compare September 4, 2022 08:52
@gerhardol
Copy link
Member Author

more than unexpected -- and occurs several times a day in contrast to #9752, which I cannot reproduce.

I did not find a reasonable way to get the status before it was minimized.
The behavior was therefore changed to require a setting to be acitivated.
The setting currently requires manually editing of the settings file

%APPDATA%\GitExtensions\GitExtensions\GitExtensions.settings
(e.g. C:\Users\user\AppData\Roaming\GitExtensions\GitExtensions\GitExtensions.settings

  <item>
    <key>
      <string>WorkaroundRestoreFromMinimize</string>
    </key>
    <value>
      <string>true</string>
    </value>
  </item>

Can we merge with this to get a new alpha/beta/rc soon?

@gerhardol gerhardol merged commit 21c293d into gitextensions:master Sep 6, 2022
@gerhardol gerhardol deleted the feature/i9752-restore-from-taskbar branch September 6, 2022 19:56
@ghost ghost modified the milestones: 4.0.0, vNext Sep 6, 2022
gerhardol added a commit that referenced this pull request Sep 6, 2022
Changed behavior from .NET4 to .NET5, application requires explicit
"restore" in Taskbar.
This is a raw workaround with the minor annoyance that closing in taskbar
requires application activation.

The behavior is called by setting WorkaroundRestoreFromMinimize
that must be manually set in GitExtensions.settings

(cherry picked from commit 21c293d)
@gerhardol gerhardol modified the milestones: vNext, 4.0.0 Sep 6, 2022
gerhardol added a commit to gerhardol/gitextensions that referenced this pull request Apr 1, 2023
Add an experimental setting to not run git-ls-files when the
window is minimized on activation, as async commands when
minimized seem to block the app from being restored.

Rename configuration added in gitextensions#10119 to
deactivate at upgrade as this setting may give a negative
experience after gitextensions#10802 mostly fixes the problem.
gerhardol added a commit to gerhardol/gitextensions that referenced this pull request Apr 1, 2023
Add an experimental setting to not run git-ls-files when the
window is minimized on activation, as async commands when
minimized seem to block the app from being restored.

Rename configuration added in gitextensions#10119 to
deactivate at upgrade as this setting may give a negative
experience after gitextensions#10802 mostly fixes the problem.
gerhardol added a commit that referenced this pull request Apr 6, 2023
Add an experimental setting to not run git-ls-files when the
window is minimized on activation, as async commands when
minimized seem to block the app from being restored.

Rename configuration added in #10119 to
deactivate at upgrade as this setting may give a negative
experience after #10802 mostly fixes the problem.
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.

Cannot bring GE to front
3 participants