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.

Sign up for GitHub

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

Expander Fixes #4394

Merged
merged 23 commits into from
Mar 26, 2021
Merged

Expander Fixes #4394

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1223fb1
Rename 'Expanding' to 'Expanded'
robloo Mar 2, 2021
363c732
Ensure Expanded/Collapsed fire after state is applied
robloo Mar 2, 2021
17c6d85
Remove MaxWidth set in the default style
robloo Mar 2, 2021
1f80b68
Make the expander properly size to content
robloo Mar 2, 2021
66bf68f
Remove Stretch content alignment (use Center default)
robloo Mar 2, 2021
0ad7bc5
Add Center VerticalAlignment
robloo Mar 2, 2021
ee209e1
Remove Min/MaxWidth from intermediate controls
robloo Mar 2, 2021
0c58a25
Update Expander test page
robloo Mar 2, 2021
25e8c59
Add fixed-height Expander example
robloo Mar 2, 2021
585b15c
Stretch header content
robloo Mar 2, 2021
21c73eb
Standardize resource names
robloo Mar 2, 2021
f2d4e00
The header VerticalContentAlignment must be center
robloo Mar 2, 2021
77554a7
Revert "Rename 'Expanding' to 'Expanded'"
robloo Mar 12, 2021
6a72d39
Revert "Ensure Expanded/Collapsed fire after state is applied"
robloo Mar 12, 2021
4739faf
Merge branch 'master' into robloo-expander-fixes
robloo Mar 12, 2021
210ca0c
Padding now applies to the Content area
robloo Mar 12, 2021
f081da4
Reorg and rename expander theme resources
robloo Mar 12, 2021
2a45b77
Work-around DP precedence issue
robloo Mar 13, 2021
9e9a810
Add resources to control header content alignment
robloo Mar 13, 2021
6543952
Fix Expander event ordering
robloo Mar 13, 2021
cf8bcef
Fix some Static not Theme resources
robloo Mar 13, 2021
ece7611
Remove some resource redirections
robloo Mar 16, 2021
53d69f4
Remove unnecessary margins
robloo Mar 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions dev/Expander/Expander.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ void Expander::OnContentSizeChanged(const winrt::IInspectable&, const winrt::Siz
}


void Expander::RaiseExpandingEvent(const winrt::Expander& container)
void Expander::RaiseExpandedEvent(const winrt::Expander& container)
{
m_expandingEventSource(*this, nullptr);
m_expandedEventSource(*this, nullptr);
}

void Expander::RaiseCollapsedEvent(const winrt::Expander& container)
Expand All @@ -103,15 +103,16 @@ void Expander::RaiseCollapsedEvent(const winrt::Expander& container)

void Expander::OnIsExpandedPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& /*args*/)
{
UpdateExpandState(true);

if (IsExpanded())
{
RaiseExpandingEvent(*this);
RaiseExpandedEvent(*this);
}
else
{
RaiseCollapsedEvent(*this);
}
UpdateExpandState(true);
}

void Expander::OnExpandDirectionPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& /*args*/)
Expand Down
2 changes: 1 addition & 1 deletion dev/Expander/Expander.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Expander :
void OnApplyTemplate();


void RaiseExpandingEvent(const winrt::Expander& container);
void RaiseExpandedEvent(const winrt::Expander& container);
void RaiseCollapsedEvent(const winrt::Expander& container);

void OnIsExpandedPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args);
Expand Down
4 changes: 2 additions & 2 deletions dev/Expander/Expander.idl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ unsealed runtimeclass Expander : Windows.UI.Xaml.Controls.ContentControl
[MUX_PROPERTY_CHANGED_CALLBACK(TRUE)]
ExpandDirection ExpandDirection{ get; set; };

event Windows.Foundation.TypedEventHandler<Expander, ExpanderExpandingEventArgs> Expanding;
event Windows.Foundation.TypedEventHandler<Expander, ExpanderExpandedEventArgs> Expanded;
robloo marked this conversation as resolved.
Show resolved Hide resolved
event Windows.Foundation.TypedEventHandler<Expander, ExpanderCollapsedEventArgs> Collapsed;

[MUX_PROPERTY_NEEDS_DP_FIELD]
Expand All @@ -52,7 +52,7 @@ enum ExpandDirection
[MUX_PUBLIC]
[default_interface]
[webhosthidden]
runtimeclass ExpanderExpandingEventArgs
runtimeclass ExpanderExpandedEventArgs
{
}

Expand Down
16 changes: 6 additions & 10 deletions dev/Expander/Expander.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,12 @@
<Setter Property="Background" Value="{ThemeResource ExpanderBackground}" />
<contract7Present:Setter Property="BackgroundSizing" Value="InnerBorderEdge" />
<Setter Property="MinWidth" Value="{ThemeResource FlyoutThemeMinWidth}" />
<Setter Property="MaxWidth" Value="{ThemeResource FlyoutThemeMaxWidth}" />
<Setter Property="MinHeight" Value="{StaticResource ExpanderMinHeight}" />
<Setter Property="BorderThickness" Value="{ThemeResource ExpanderBorderThickness}" />
<Setter Property="BorderBrush" Value="{ThemeResource ExpanderBorderBrush}" />
<Setter Property="Padding" Value="{StaticResource ExpanderHeaderPadding}" />
robloo marked this conversation as resolved.
Show resolved Hide resolved
<Setter Property="HorizontalAlignment" Value="Stretch" />
<Setter Property="HorizontalContentAlignment" Value="Left" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this HorizontalContentAlignment no longer needed? Looking at this change it looks like the template never actually used this before. Is this just deleting unused lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#4394 (comment)

  1. The default actually should be center per Expander Fixes #4394 (comment)
  2. I followed convention of the Button template
  3. When the default value of 'Center' is required, the property is simply omitted. This is better as all controls will fallback to the same default so if once in 1000 years the default was actually changed you wouldn't need to go and update all the templates (single source of truth). The rule of thumb: only change what you need to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tashatitova Did I misunderstand about the HorizontalContentAlignment? Were you only referring to VerticalContentAlignment in that comment referenced above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tashatitova Can you confirm about HorizontalContentAlignment? The more I think about it, I think this was my misunderstanding and you were only talking about VerticalContentAlignment. I would like to make this change and get this PR merged ASAP.

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 just re-read it and your last comment emphasizes vertical. I'm going to make the change and add back HorizontalContentAlignment. Horizontal seems like it should be Left so @StephenLPeters was right to point this out.

<Setter Property="VerticalContentAlignment" Value="Center" />
<Setter Property="HorizontalAlignment" Value="Left" />
<Setter Property="VerticalAlignment" Value="Center" />
<contract7Present:Setter Property="CornerRadius" Value="{ThemeResource ControlCornerRadius}" />
<Setter Property="Template">
robloo marked this conversation as resolved.
Show resolved Hide resolved
<Setter.Value>
Expand Down Expand Up @@ -133,30 +131,28 @@
BorderBrush="{TemplateBinding BorderBrush}"
BorderThickness="{TemplateBinding BorderThickness}"
MinHeight="{TemplateBinding MinHeight}"
MinWidth="{TemplateBinding MinWidth}"
MaxWidth="{TemplateBinding MaxWidth}"
contract7Present:CornerRadius="{TemplateBinding CornerRadius}"
IsEnabled="{TemplateBinding IsEnabled}"
Padding="{TemplateBinding Padding}"
Style="{StaticResource ExpanderHeaderDownStyle}"
Content="{TemplateBinding Header}"
ContentTemplate="{TemplateBinding HeaderTemplate}"
ContentTemplateSelector="{TemplateBinding HeaderTemplateSelector}"
HorizontalAlignment="{TemplateBinding HorizontalAlignment}"
HorizontalAlignment="Stretch"
tashatitova marked this conversation as resolved.
Show resolved Hide resolved
IsChecked="{Binding Path=IsExpanded, Mode=TwoWay, RelativeSource={RelativeSource TemplatedParent}}" />
<!-- The clip is a composition clip applied in code -->
<Border x:Name="ExpanderContentClip" Grid.Row="1">
<Border
x:Name="ExpanderContent"
x:Name="ExpanderContent"
Visibility="Collapsed"
Background="{ThemeResource ExpanderDropDownBackground}"
robloo marked this conversation as resolved.
Show resolved Hide resolved
contract7Present:BackgroundSizing="{TemplateBinding BackgroundSizing}"
contract7Present:CornerRadius="{Binding CornerRadius, RelativeSource={RelativeSource TemplatedParent}, Converter={StaticResource BottomCornerRadiusFilterConverter}}"
BorderBrush="{ThemeResource ExpanderDropDownBorderBrush}"
BorderThickness="{StaticResource ExpanderDropdownDownBorderThickness}"
MinHeight="{TemplateBinding MinHeight}"
MinWidth="{TemplateBinding MinWidth}"
MaxWidth="{TemplateBinding MaxWidth}"
HorizontalAlignment="Stretch"
tashatitova marked this conversation as resolved.
Show resolved Hide resolved
VerticalAlignment="Stretch"
Padding="{StaticResource ExpanderContentPadding}">
<ContentPresenter
Content="{TemplateBinding Content}"
Expand Down
19 changes: 14 additions & 5 deletions dev/Expander/TestUI/ExpanderPage.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d">

<Grid Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
<ScrollViewer Background="{ThemeResource ApplicationPageBackgroundThemeBrush}" HorizontalScrollBarVisibility="Disabled" VerticalScrollBarVisibility="Auto">

<StackPanel Orientation="Vertical" HorizontalAlignment="Left">
<StackPanel.ChildrenTransitions>
Expand All @@ -29,19 +29,19 @@
<Button AutomationProperties.AutomationId="ExpandedExpanderContent">Content</Button>
</controls:Expander>

<controls:Expander AutomationProperties.AutomationId="CollapsedExpander" AutomationProperties.Name="Expander2" IsExpanded="False" Margin="12">
<controls:Expander AutomationProperties.AutomationId="CollapsedExpander" AutomationProperties.Name="Expander2" IsExpanded="False" Margin="12" Width="{ThemeResource FlyoutThemeMaxWidth}">
<controls:Expander.Header>
<TextBlock Text="This expander is collapsed by default. This is a long title to test the header width. ext should be wrapping here." TextWrapping="Wrap" Margin="0,12,0,13"/>
</controls:Expander.Header>
<TextBlock AutomationProperties.AutomationId="CollapsedExpanderContent">Content</TextBlock>
</controls:Expander>

<controls:Expander AutomationProperties.Name="ExpanderWithButtons" IsExpanded="False" Margin="12">
<controls:Expander AutomationProperties.Name="ExpanderWithButtons" IsExpanded="False" Margin="12" MaxWidth="{ThemeResource FlyoutThemeMaxWidth}">
<controls:Expander.Header>
<ToggleButton>This is a toggle button in the header</ToggleButton>
</controls:Expander.Header>
<StackPanel>
<TextBlock TextWrapping="Wrap">Content. This is long content to test wrapping on the content section, this content should be wrapping.</TextBlock>
<TextBlock TextWrapping="Wrap">Content. This is long content to test wrapping on the content section, this content should be wrapping after the control expands to max width.</TextBlock>
</StackPanel>
</controls:Expander>

Expand Down Expand Up @@ -73,7 +73,16 @@
</StackPanel>
</controls:Expander>
</local:TestControl>

<controls:Expander AutomationProperties.Name="ExpanderStretched" ExpandDirection="Down" IsExpanded="True" Margin="12" HorizontalAlignment="Stretch" HorizontalContentAlignment="Right">
<controls:Expander.Header>
<TextBlock Margin="0,0,0,1">This expander stretches along with its content</TextBlock>
</controls:Expander.Header>
<StackPanel HorizontalAlignment="Right">
<TextBlock>Right Content</TextBlock>
</StackPanel>
</controls:Expander>
</StackPanel>

</Grid>
</ScrollViewer>
</local:TestPage>
10 changes: 5 additions & 5 deletions dev/Generated/Expander.properties.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ GlobalDependencyProperty ExpanderProperties::s_TemplateSettingsProperty{ nullptr

ExpanderProperties::ExpanderProperties()
: m_collapsedEventSource{static_cast<Expander*>(this)}
, m_expandingEventSource{static_cast<Expander*>(this)}
, m_expandedEventSource{static_cast<Expander*>(this)}
{
EnsureProperties();
}
Expand Down Expand Up @@ -211,12 +211,12 @@ void ExpanderProperties::Collapsed(winrt::event_token const& token)
m_collapsedEventSource.remove(token);
}

winrt::event_token ExpanderProperties::Expanding(winrt::TypedEventHandler<winrt::Expander, winrt::ExpanderExpandingEventArgs> const& value)
winrt::event_token ExpanderProperties::Expanded(winrt::TypedEventHandler<winrt::Expander, winrt::ExpanderExpandedEventArgs> const& value)
{
return m_expandingEventSource.add(value);
return m_expandedEventSource.add(value);
}

void ExpanderProperties::Expanding(winrt::event_token const& token)
void ExpanderProperties::Expanded(winrt::event_token const& token)
{
m_expandingEventSource.remove(token);
m_expandedEventSource.remove(token);
}
6 changes: 3 additions & 3 deletions dev/Generated/Expander.properties.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ class ExpanderProperties

winrt::event_token Collapsed(winrt::TypedEventHandler<winrt::Expander, winrt::ExpanderCollapsedEventArgs> const& value);
void Collapsed(winrt::event_token const& token);
winrt::event_token Expanding(winrt::TypedEventHandler<winrt::Expander, winrt::ExpanderExpandingEventArgs> const& value);
void Expanding(winrt::event_token const& token);
winrt::event_token Expanded(winrt::TypedEventHandler<winrt::Expander, winrt::ExpanderExpandedEventArgs> const& value);
void Expanded(winrt::event_token const& token);

event_source<winrt::TypedEventHandler<winrt::Expander, winrt::ExpanderCollapsedEventArgs>> m_collapsedEventSource;
event_source<winrt::TypedEventHandler<winrt::Expander, winrt::ExpanderExpandingEventArgs>> m_expandingEventSource;
event_source<winrt::TypedEventHandler<winrt::Expander, winrt::ExpanderExpandedEventArgs>> m_expandedEventSource;

static void EnsureProperties();
static void ClearProperties();
Expand Down