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

Toolbar position is not saved or restored #8680

Closed
pmiossec opened this issue Dec 16, 2020 · 6 comments · Fixed by #8719
Closed

Toolbar position is not saved or restored #8680

pmiossec opened this issue Dec 16, 2020 · 6 comments · Fixed by #8719

Comments

@pmiossec
Copy link
Member

pmiossec commented Dec 16, 2020

Current behaviour

Since #8572, I always end up with this toolbar at startup:
image

Even if I move the toolbars. Even when only on instance is opened.

Even with #8679 fix (to only save the toolbar settings)

Expected behaviour

I can configure the toolbar position and it is saved (or restored) well.

Steps to reproduce

  1. Get a dev version of GE (portable from CI or your custom version) and launch it
  2. Change toolbar setup
  3. Restart
  4. See toolbar position configuration is the same as initally

Screenshots

See above.

Did this work in previous version of GitExtensions

Never since the feature was introduced...

Environment

  • Git Extensions 33.33.33
  • Build 3245408
  • Git 2.28.0.windows.1
  • Microsoft Windows NT 10.0.17134.0
  • .NET Framework 4.8.4261.0
  • DPI 192dpi (200% scaling)

Diagnostics

The main goal to opening this issue is:

How can I debug it easily?
Where the configuration data are stored?

@mstv
Copy link
Member

mstv commented Dec 16, 2020

I cannot reproduce exactly this but:

  • When positioned as in the issue screenshot, the original layout is restored. So I have kind of the opposite behavior.
  • Perhaps it depends on the scaling factor. (I will not touch this setting of 100% on this machine!)
  • (When moving the toolbars, they jump around in an unexpected way.)
  • When positioned stacked vertically (just fooling about), the lower toolbar is moving to the right with every start. After 3rd:
    grafik

@pmiossec
Copy link
Member Author

* Perhaps it depends on the scaling factor. (I will not touch this setting of 100% on this machine!)

I will give it a try to see if that changes the behavior.

* (When moving the toolbars, they jump around in an unexpected way.)

The same. And it is a pain to successfully arrange them...

@RussKie RussKie self-assigned this Dec 16, 2020
@RussKie RussKie added this to the 4.0 milestone Dec 16, 2020
@RussKie

This comment has been minimized.

@RussKie
Copy link
Member

RussKie commented Dec 17, 2020

This is very gnarly one... I've spent several hours trying different thigs, but so far I couldn't get it working reliably. Reading and then setting the same location moves a toolbar laterally.

RussKie added a commit to RussKie/gitextensions that referenced this issue Jan 3, 2021
Due to discovered layout and placement instability of toolbars
reverting most of the functionality added in gitextensions#8572.

Closes gitextensions#8680
@ghost ghost added the 🚧 status: in progress Issues which have associated PRs label Jan 3, 2021
@RussKie
Copy link
Member

RussKie commented Jan 3, 2021

The toolbar gets pushed by something:
image

It is wild, and requires a deeper investigation. It's plausible there's something in our codebase that's causing it, similar to the weird behaviours observed in #8698 (comment).

RussKie added a commit to RussKie/gitextensions that referenced this issue Jan 7, 2021
Due to discovered layout and placement instability of toolbars
reverting most of the functionality added in gitextensions#8572.

Closes gitextensions#8680
RussKie added a commit to RussKie/gitextensions that referenced this issue Jan 18, 2021
Due to discovered layout and placement instability of toolbars
reverting most of the functionality added in gitextensions#8572.

Closes gitextensions#8680
@ghost ghost removed the 🚧 status: in progress Issues which have associated PRs label Jan 18, 2021
@RussKie RussKie removed this from the 3.5 milestone Mar 7, 2021
@RussKie
Copy link
Member

RussKie commented Sep 20, 2021

I've debugged the Windows Forms source, and it looks like the implementation isn't working correctly when a toolstrip panel has non-zero padding.

The following workaround appears to yield the expected results:

        public override void OnLoad(EventArgs e)
        {
            // Restore the toolbars layout
            ToolStripManager.LoadSettings(this, "toolsettings");

            // Apply the padding
            this.toolPanel.TopToolStripPanel.Padding = new System.Windows.Forms.Padding(4, 0, 4, 0);
        }

        protected override void OnFormClosing(FormClosingEventArgs e)
        {
            // Reset padding to zero before saving
            this.toolPanel.TopToolStripPanel.Padding = new System.Windows.Forms.Padding(0, 0, 0, 0);

            // Persist the toolbars layout
            ToolStripManager.SaveSettings(this, "toolsettings");
        }

RussKie added a commit to RussKie/gitextensions that referenced this issue Jan 16, 2022
Reintroduce ability to move toolstrips (as proposed in gitextensions#8572).

This implementation is heavily influenced by the original implementation
ToolStripSettingsManager in Windows Forms, however we only track visibility
and location of each individual toolstrip. We're also only saving specified
toolstrips as opposed to saving all known toolstrips (which in various
situations results in terminal failures to restore toolstrips information).

There are two key changes:

1. Restore toolstrip locations after the form's geometry has been restored.
   This ensures the toolstrips can be placed at the same coords as they
   were saved.

2. Hosting ToolStripPanel's padding must be set to 0 before toolstrips'
   locations can be restored/saved. This mitigates dotnet/winforms#4449

Fixes gitextensions#8680
Reverts gitextensions#8719
RussKie added a commit to RussKie/gitextensions that referenced this issue Jan 16, 2022
Reintroduce ability to move toolstrips (as proposed in gitextensions#8572).

This implementation is heavily influenced by the original implementation
ToolStripSettingsManager in Windows Forms, however we only track visibility
and location of each individual toolstrip. We're also only saving specified
toolstrips as opposed to saving all known toolstrips (which in various
situations results in terminal failures to restore toolstrips information).

There are two key changes:

1. Restore toolstrip locations after the form's geometry has been restored.
   This ensures the toolstrips can be placed at the same coords as they
   were saved.

2. Hosting ToolStripPanel's padding must be set to 0 before toolstrips'
   locations can be restored/saved. This mitigates dotnet/winforms#4449

Fixes gitextensions#8680
Reverts gitextensions#8719
RussKie added a commit to RussKie/gitextensions that referenced this issue Jan 16, 2022
Reintroduce ability to move toolstrips (as proposed in gitextensions#8572).

This implementation is heavily influenced by the original implementation
ToolStripSettingsManager in Windows Forms, however we only track visibility
and location of each individual toolstrip. We're also only saving specified
toolstrips as opposed to saving all known toolstrips (which in various
situations results in terminal failures to restore toolstrips information).

There are two key changes:

1. Restore toolstrip locations after the form's geometry has been restored.
   This ensures the toolstrips can be placed at the same coords as they
   were saved.

2. Hosting ToolStripPanel's padding must be set to 0 before toolstrips'
   locations can be restored/saved. This mitigates dotnet/winforms#4449

Fixes gitextensions#8680
Reverts gitextensions#8719
@ghost ghost added the 🚧 status: in progress Issues which have associated PRs label Jan 16, 2022
@ghost ghost removed the 🚧 status: in progress Issues which have associated PRs label Aug 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants