#3070 Stretch the content of the expander header

[skendrot]

Fixes #3070

PR Type

What kind of change does this PR introduce?

• Style Change

What is the current behavior?

The Expander Header is left aligned

What is the new behavior?

The expander header will stretch to fit it's space. BREAKING CHANGE

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository instructions. Link:
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

This is a breaking *style change. Content that was left aligned will now stretch. This could change how an application looks

Other information

[ghost]

Thanks for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[michael-hawker]

Thanks @skendrot. It'll be a breaking change if their content is aligned differently, eh?

Trying to think if there'd be an easy way to expose these bound from the Expander itself somehow, so it's more configurable, then we could at least leave the default the same...

[michael-hawker]

@skendrot think you missed my question. Could we somehow TemplateBind this property to the Expander's HorizontalContentAlignment property so we could keep the default, but make this an option and not have it be a breaking change?

[michael-hawker]

@skendrot thoughts on removing the line in the Style all together as the default for HorizontalContentAlignment should be Left already. Then we just add a TemplateBinding to the ToggleButton where it's used:

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/090e9f7bfd478b578e61f2b93c6156dca9678cee/Microsoft.Toolkit.Uwp.UI.Controls/Expander/Expander.xaml#L335-L344

<ToggleButton x:Name="PART_ExpanderToggleButton" 
               MinWidth="40" 
               MinHeight="40" 
               AutomationProperties.Name="Expand" 
               Content="{TemplateBinding Header}" 
               ContentTemplate="{TemplateBinding HeaderTemplate}"
               HorizontalContentAlignment="{TemplateBinding HorizontalContentAlignment}" 
               Foreground="{TemplateBinding Foreground}" 
               IsChecked="{Binding IsExpanded, Mode=TwoWay, RelativeSource={RelativeSource TemplatedParent}}" 
               Style="{TemplateBinding HeaderStyle}" 
               TabIndex="0" /> 

This should allow a developer to just do:

      <controls:Expander HorizontalContentAlignment="Stretch"> ...

[skendrot]

@michael-hawker Should the ToggleButton bind to the HorizontalContentAlignment? Or HorizontalAlignment. The ToggleButton isn't considered to be the Content of the control. Maybe instead Change to bind to the HorizontalAlignment?

[skendrot]

Thinking about it more. Maybe it does makes sense for the content of the toggle button to align with the HorizontalContentAlignment of the expander itself.

[michael-hawker]

@skendrot I did notice we do use those properties as well here on the content area.

So thinking more, it does make sense to pass the HorizontalAlignment to the toggle for the expander 'header' and then keep the HorizontalContentAlignment as the content area.

[skendrot]

@michael-hawker what more is needed here?

[michael-hawker]

@skendrot sorry I meant to get to this the other day, but I've had to reset both my machines this week and slowed me down. I discussed with some others on my team about this problem, the consensus was that we should just introduce a new property to deal with this HeaderHorizontalContentAlignment so that we can not cause a breaking change and have more fined control over it (and be able to use the HorizontalContentAlignment for the actual content).

If you'd like to add the new property, let me know.

[skendrot]

Why would we need a new property? I say it's a breaking change, but in reality it's probably not going to affect anyone

[michael-hawker]

@skendrot there's cases I've used an Expander with a Grid to size/place icons next to stuff where this would break it, so we wanted to be cautious. Also the HorizontalContentAlignment would indicate it should adjust the content of the expander, not the Expander's header, so we figured it made sense to separate these out.

[skendrot]

@michael-hawker I don't see a need for a property for just aligning the header content. That's easily achievable with a quick style.

    <controls:Expander>
        <controls:Expander.HeaderStyle>
            <Style TargetType="ToggleButton">
                <Setter Property="HorizontalAlignment" Value="Stretch" />
            </Style>
        </controls:Expander.HeaderStyle>
    </controls:Expander>

[michael-hawker]

@skendrot that almost does it if it didn't have to replace the template with our chevron in it too at the same time. WinUI Ref

[michael-hawker]

Going to move this out for now, we can try and better address with more concrete template changes in 7.0

[michael-hawker]

Saw this same bug was carried over to the new WinUI version in the works so filed an issue there: microsoft/microsoft-ui-xaml#4248

For now let's just merge this change in and make a doc note, and we can figure out an alternative later if needed or we'll be deprecating this control soon in favor of the WinUI one.

[michael-hawker]

@RosarioPulella mind just cherry-picking this commit on top of main so @skendrot can get credit and we'll just have a cleaner history rather than trying to resolve the merge based on this older branch? Thanks!

[michael-hawker]

Done in #3769 from cherry-picked commit. Closing this one.