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

Add SplitView control #4174

Merged
merged 11 commits into from
Jun 30, 2020
Merged

Add SplitView control #4174

merged 11 commits into from
Jun 30, 2020

Conversation

amwx
Copy link
Contributor

@amwx amwx commented Jun 26, 2020

What does the pull request do?

Adds SplitView control from my FluentAvalonia repo with a few enhancements, primarily moving pane animation to Styles

NewSplitView

Checklist

<Style Selector="SplitView:overlay:left /template/ Panel#PART_PaneRoot">
<Setter Property="Width" Value="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.ClosedPaneWidth}" />
<!-- ColumnSpan should be 2 -->
<Setter Property="Grid.ColumnSpan" Value="1"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ColumnSpan should technically be 2 here (and also in :compactoverlay below). When I copied the code into Avalonia from my test project this broke and opening the pane would also move the content slightly, but only when the pane is on the left. Not sure why this is being handled differently...

Copy link
Member

Choose a reason for hiding this comment

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

well.. could you try explicitly setting grid.column too? just in case there's some bug in there...

<Grid Name="Container" Background="{TemplateBinding Background}">
<Grid.ColumnDefinitions>
<!-- why is this throwing a binding error? -->
<ColumnDefinition Width="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.PaneColumnGridLength}"/>
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 throwing a binding error (you can see it on load or when you change the PanePlacement).
[Binding] Error in binding to 'Avalonia.Controls.ColumnDefinition'.'Width': 'Null value in expression ''.' (ColumnDefinition #41130254)
Like my comment below, this only started happening when I moved the code from my test project into avalonia. I know and have checked TemplateSettings.PaneColumnGridLength is not null so I'm not sure why this is happening.

@jmacato
Copy link
Member

jmacato commented Jun 26, 2020

Data-yes
It's about time 😆

@amwx amwx changed the title [WIP] Add SplitView control Add SplitView control Jun 26, 2020
@amwx amwx marked this pull request as ready for review June 26, 2020 20:45
Copy link
Member

@jmacato jmacato left a comment

Choose a reason for hiding this comment

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

LGTM, just some comments :)

<Style Selector="SplitView:overlay:left /template/ Panel#PART_PaneRoot">
<Setter Property="Width" Value="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.ClosedPaneWidth}" />
<!-- ColumnSpan should be 2 -->
<Setter Property="Grid.ColumnSpan" Value="1"/>
Copy link
Member

Choose a reason for hiding this comment

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

well.. could you try explicitly setting grid.column too? just in case there's some bug in there...

@jmacato
Copy link
Member

jmacato commented Jun 27, 2020

@AvaloniaUI/core dont merge this yet.. i have an incoming PR for spline easing

/// <summary>
/// Defines constants for how the SplitView Pane should display
/// </summary>
public enum SplitViewDisplayMode
Copy link
Member

Choose a reason for hiding this comment

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

nit: I noticed that many controls is placed in root of Avalonia.Controls folder. Should we start to move controls and related classes/enums to specific sub folders?

Copy link
Member

Choose a reason for hiding this comment

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

I think that would be a good idea for more complex controls that are not single file... what about namespace too?

Copy link
Member

Choose a reason for hiding this comment

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

We should keep Avalonia.Controls, because it is convenient for developers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree here, I think the controls folder could be better organized this way. Would make it easier to find things too

amwx and others added 5 commits June 28, 2020 17:58
Co-authored-by: Max Katz <maxkatz6@outlook.com>
Update SplitView easings to use what's in UWP's template instead of QuinticEaseOut.
Copy link
Member

@danwalmsley danwalmsley left a comment

Choose a reason for hiding this comment

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

some minor nits.

@amwx
Copy link
Contributor Author

amwx commented Jun 30, 2020

@danwalmsley I believe I've addressed all you're comments
@jmacato The new spline easing looks fantastic!

@danwalmsley danwalmsley merged commit 45be929 into AvaloniaUI:master Jun 30, 2020
@ShadowDancer
Copy link
Contributor

ShadowDancer commented Jul 12, 2020

@amwx could you explain how exactly overlay mode works? When I try render content over other controls (ie. adding them to same grid row) they appear on top of each other. It all happens just by ZIndex?

@amwx
Copy link
Contributor Author

amwx commented Jul 12, 2020

@ShadowDancer In Overlay mode, the pane is supposed to display above content. Tho there is currently an issue with the SplitView in Overlay mode, because of a difference in behavior between the Avalonia source and the nuget package for some reason. So currently, Overlay mode displays like Inline.
If you can open up the ControlCatalog, the example in there works as intended for all four display modes

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.

5 participants