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

Auto resizing floating window on startup #146

Merged
merged 8 commits into from
Apr 9, 2020

Conversation

eriove
Copy link
Contributor

@eriove eriove commented Apr 1, 2020

This pull request implements the auto resize in #140. It solves the problem I'm having for the existing styles, but might not help if someone creates a completely different style. But I don't see that as a problem, since that's a regular occurrence when styles are changed,

I also added an example in the TestApp which does the same thing we do in our NavigationService in our real application (our navigation service mimics MVVM Cross' navigation service but for dockable windows instead of full screen "windows").

This is the look when calling OnNewFloatingWindow before the change
image
and this is after the change:
image

@@ -248,7 +250,9 @@ protected virtual IntPtr FilterMessage(IntPtr hwnd, int msg, IntPtr wParam, IntP
handled = true;
}
}
break;
UpdateWindowsSizeBasedOnMinSize();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the first location where the style was loaded and the grid and border size could be retreived

source/Components/AvalonDock/VisualTreeHelperExtensions.cs Outdated Show resolved Hide resolved
@eriove eriove marked this pull request as ready for review April 3, 2020 07:11
@Dirkster99
Copy link
Owner

Dirkster99 commented Apr 3, 2020

I have reviewed your changes so far. Here are my thoughts on this:

  1. Please do not change the XCeed copyright notice as it refers to the past of this project. I might add another notice along the lines of:
    'Copyright (C) 2011-2020 by dirkster99 et al...' but if and when I'd rather do this for all files at once ...

  2. Please add comments to the new methods that you are adding in Extensions.c etc...

  3. I am surprissed you are changing the LayoutFloatingWindowControl because this will also affect the DocumentFloatingWindowControl, right?
    I don't think its useful to have a auto-resizing floating document control, do you agree?

Would it not be more useful to have the Auto-Resizing of the title bar in the LayoutFloatingWindowControl and the rest of it (configurable at construction time) in a deriving class (LayoutAnchorableFloatingWindowControl)?

  1. Am I right when I assume that your current implementation will always Auto-Resize the complete Floating Window?
    For the title bar thats of course always fine, but I think it should be configurable for auto-resizing the rest of the LayoutAnchorable. Should we not introduce some kind of configuration towards making Auto-Resizing unavailable by default but configurable for those who want it?

Maybe this could be done in the style of the LayoutAnchorableFloatingWindowControl by adding a property like 'IsAutoResingContent'?

@eriove
Copy link
Contributor Author

eriove commented Apr 6, 2020

I've reverted the copyright notice.

I've added comments, note that the extension methods are internal. My guess is that many projects already have these methods implemented so I didn't want to expose them since it is likely to cause compile time errors if they happen to have the same name.

3 and 4.
The control is only resized at first render (when floating). If more controls are docked in the same floating window, they will not trigger a resize. Note that FrameworkElement.MinHeight has a default value of 0. This means that unless the user has explicitly set the MinHeight or MinWidth the behavior will not be changed by this pull request.

My assumption was that when these properties are set the control is unusable if the size is smaller and that it's better to resize any floating window that needs the space. But on the other hand, a control that needs a certain size probably shouldn't be in a DocumentFloatingWindowControl to start with.

I'm happy to move the size change to LayoutFloatingWindowControl and add a switch to keep it turned off as a default. In that case I would suggest to make the _totalMargin a public property instead so that callers can easily add their own call to resize the document control if they need it.

Regarding the title bar, it seem to me that it already adjust its size to the content and that no changes are needed?

I have a couple of ideas of how the current implementation might fail so I will test more during the day.

I have the feeling that more of this could be done with styles but that I lack the experience to see how, let me know if you see a less convoluted way of solving this than what I've done.

@eriove
Copy link
Contributor Author

eriove commented Apr 6, 2020

Found a possible issue. I can honestly not tell if it is a problem or not. If the right toolbox window (in TestApp) is un-docked with "Tool Window 1" active the min height will be picked up and used.
toolwindow1

If is un-docked with "Tool window 2" active it wont get resized.
toolwindow2

What do you think? I can think of three possible options here:

  1. The current one, the size is adjusted based on what's in view when the window is undocked.
  2. The window is resized based on the largest minimum size in any of the tabs.
  3. The window change size when the tab is switched.

I prefer option one, since it would look a bit strange if the window suddenly got larger while showing a small control in the top left corner. Option 3 could potentially be annoying if the user has arranged his/her windows and quickly want to check something in another tab.

A fourth option would be to add an option for the different alternatives, but that's probably better to add when/if its needed.

@Dirkster99
Copy link
Owner

I am actually surprised that your changes work without changing styles(?)

There are styles for both floating control types (Anchorable, Document):

I usally debug XAML styles like these by setting some number or property that makes it easy to recognize the change (eg.: set Background = "Red" instead of a binding statement and you'll often get a highlighted element). It does help of course to know some basic XAML because you may set BorderBrush="Red" but it won't be visible unless you set BorderThickness="3" (its zero by default).

This is basically how I highlighted some controls visually in the Wiki section (eg.: LayoutAnchorableControl).

I could do something similar for the LayoutAnchorableFloatingWindow (depicting its parts with different colors) if this would be helpful for you.

@eriove
Copy link
Contributor Author

eriove commented Apr 6, 2020

That's a really neat trick for debugging. I've used the Live Visual Tree and the WPF Visualizer together with the styles you linked to find what to search for with the LINQ queries. It would really help if you did the same thing for the LayoutAnchorableFloatingWindow .

I started with the queries of the visual tree at runtime when I tried to do the same thing from the calling code without having to redefine the complete style.

I'll make some changes to put the setting of the LayoutAnchorableFloatingWindow in the style instead.

@Dirkster99
Copy link
Owner

Dirkster99 commented Apr 6, 2020

I've added a Visual Parts section to better visualize the parts defined in the Generics.xaml file.

Looking at the style I've realized that you might be interested in the items highlighted below:
Untitled

  • Line 1004 and Line 1022 can be used to change the height of the caption (both should have the same value)

  • Line 1020 indicates a Margin - I understand that you want to change this or make it adjustable(?)

  • I am also seeing now that the LayoutAnchorableControl is bound via ContentPresenter in the style, which means that your approach via VisualTree Helper for adjusting the Window size is propably the best way to do it in this situation - we should just find a way to make it configurable and switch it off by default

@eriove
Copy link
Contributor Author

eriove commented Apr 7, 2020

I've now added a property for enabling and disabling the resizing and exposed the content size and total margins as dependency properties so that they can be used in styles in the future. As a default the behavior will not change with these changes.

Let me know if I should make any other changes.

@Dirkster99
Copy link
Owner

Dirkster99 commented Apr 9, 2020

This looks good from a technical point of view - the switch is
LayoutFloatingWindowControl.SetWindowSizeWhenOpened

  • We should provide sample code (eg App.xaml lines below) on how users can switch this on or off.

  • We should rename the 'New floating window' towards something like 'Floating window with initial usercontrol size' and leave some explanatory text in a TextBlock in the LayoutAnchorable, instead of the Timer TextBox:

TextBlock
'This content can initially be sized to the content of a containing user control when dragging the LayoutAnchorable into floating mode using the SetWindowSizeWhenOpened property (see App.xaml of this demo app).'

We can set SetWindowSizeWhenOpened in Generic.xaml and that works as expected but we should use the following sample code in App.xaml and refer to it from the LayoutAnchorable so people who come along testing this understand the test case without having to have a Ph.D :-) or fidel with the Gerneric.xaml:

<Application.Resources>
  <ResourceDictionary
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:avalonDockControls="clr-namespace:AvalonDock.Controls;assembly=AvalonDock">
    <Style
      x:Key="{x:Type avalonDockControls:LayoutAnchorableFloatingWindowControl}"
      BasedOn="{StaticResource {x:Type avalonDockControls:LayoutAnchorableFloatingWindowControl}}"
      TargetType="{x:Type avalonDockControls:LayoutAnchorableFloatingWindowControl}">
      <Setter Property="SetWindowSizeWhenOpened" Value="False" />
    </Style>
  </ResourceDictionary>
</Application.Resources>

@Dirkster99
Copy link
Owner

On the other hand I can just merge this and add the demo customization as described above - thanks a lot for your patience and this cool contribution :-)

@Dirkster99 Dirkster99 merged commit c48ed5d into Dirkster99:master Apr 9, 2020
@eriove
Copy link
Contributor Author

eriove commented Apr 9, 2020

Thanks for merging, and thank you for pushing me a bit. I should have tried the things you mentioned earlier in the week. I will make a new pull requests with some updates.

I've just noticed that the resources of the docking manager doesn't propagate to the floating windows. Will continue a bit tonight and then I'll revisit this next week (Sweden is not working Friday or Monday), templates, resources and styles have been my weakest .NET skills so this is a good way to learn how WPF works.

Dirkster99 added a commit that referenced this pull request Apr 9, 2020
Auto resizing floating window feature from PR #146
@Dirkster99
Copy link
Owner

Hi enove,

I think its OK now, I've added the missing bits in the TestApp in the last commit. I should have known how to do this but my WPF skills have gotten a little bit slow as I was forced to take a bit of a break for the last 6 months due to private issues - but feel free to add on this if you like.

Friday and Monday is also off in Germany so I guess you have a happy Easter in Sweden :-)

Cheers Drk

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.

None yet

2 participants