Skip to content

Conversation

@michael-hawker
Copy link
Member

Tied to Experiment #129 #216

Switch SettingsExpander to use ItemsRepeater and inherit from Control instead of ItemsControl

This is the WinUI approach TabView/NavigationView uses and allows us to specify the entire Container in the DataTemplate:

        <labs:SettingsExpander Description="The SettingsExpander can use ItemsSource to define its Items."
                               Header="Settings Expander with ItemsSource"
                               ItemsSource="{x:Bind MyDataSet}">
            <labs:SettingsExpander.ItemTemplate>
                <DataTemplate x:DataType="local:MyDataModel">
                    <labs:SettingsCard Description="{x:Bind Info}"
                                       Header="{x:Bind Name}">
                        <HyperlinkButton Content="{x:Bind LinkDescription}"
                                         NavigateUri="{x:Bind Url}" />
                    </labs:SettingsCard>
                </DataTemplate>
            </labs:SettingsExpander.ItemTemplate>
        </labs:SettingsExpander>

This is needed for the ItemSource scenario of SettingsExpander to work properly. Otherwise ItemsControl only sees the data item and sticks the container in another container, like this:

image

With this change the desired behavior works properly now:

image

This PR also split out the samples for more delineation in the future and room for a full complete settings page sample.

Notes:

  • We could also inherit from ContentControl instead of Control like NavigationView, but this was a simpler first step.

  • Some ItemsControl properties/methods are still missing, but initial approach seems to work and resolve issues with our same desired API surface.

  • New issue: Expander is shifting position when expanding for some reason now? (don't think it did that before)

@michael-hawker michael-hawker added the experiment 🧪 Used to track issues that are experiments (or their linked discussions) label Oct 21, 2022
element.ReadLocalValue(FrameworkElement.StyleProperty) == DependencyProperty.UnsetValue)
{
// TODO: Get item from args.Index?
element.Style = ItemContainerStyleSelector.SelectStyle(null, element);
Copy link
Member Author

Choose a reason for hiding this comment

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

Wonder if DataContext is set already here, should check, as that'd be just easier?

@michael-hawker
Copy link
Member Author

michael-hawker commented Oct 24, 2022

Random build failure again:

"D:\a\Labs-Windows\Labs-Windows\Toolkit.Labs.All.sln" (default target) (1:2) ->
"D:\a\Labs-Windows\Labs-Windows\platforms\CommunityToolkit.Labs.Uwp\CommunityToolkit.Labs.Uwp.csproj" (default target) (10:6) ->
(BuildNativePackage target) -> 
  C:\Users\runneradmin\.nuget\packages\microsoft.net.native.compiler\2.2.10-rel-29722-00\tools\Microsoft.NetNative.targets(805,5): error : Error: NUTC300F:Internal Compiler Error: Native compilation failed due to out of memory error [D:\a\Labs-Windows\Labs-Windows\platforms\CommunityToolkit.Labs.Uwp\CommunityToolkit.Labs.Uwp.csproj]
  C:\Users\runneradmin\.nuget\packages\microsoft.net.native.compiler\2.2.10-rel-29722-00\tools\Microsoft.NetNative.targets(805,5): error : ILT0005: 'C:\Users\runneradmin\.nuget\packages\runtime.win10-x64.microsoft.net.native.compiler\2.2.10-rel-29722-00\tools\x64\ilc\Tools\nutc_driver.exe @"D:\a\Labs-Windows\Labs-Windows\platforms\CommunityToolkit.Labs.Uwp\obj\x64\Release\ilc\intermediate\MDIL\CommunityToolkit.Labs.Uwp.rsp"' returned exit code 1 [D:\a\Labs-Windows\Labs-Windows\platforms\CommunityToolkit.Labs.Uwp\CommunityToolkit.Labs.Uwp.csproj]

Some of this was going to be resolved by the work I was trying to do on warnings for the long path of #211 as well.

We could also see if the latest version is better? https://www.nuget.org/packages/Microsoft.Net.Native.Compiler/2.2.12-rel-31116-00 - @Sergio0694 what controls that, do you know?

@niels9001
Copy link
Collaborator

@michael-hawker So the jumping issue was gone when I removed the ScrollViewer in the Expander samples. The ToolkitSampleRenderer already has a ScrollViewer for these situations:

SettingsExpanderScrollBug

But it's not there when using a ScrollViewer on main - so there might be some issue still. Will check if I can find that :)

Overall, I think we need to re-XAML the sample renderer layout to avoid this quirks and think about how we want to deal with an overrideable MaxHeight and all that!

@Sergio0694
Copy link
Member

Sergio0694 commented Oct 25, 2022

@michael-hawker The .NET Native toolchain is a transitive dependency from Microsoft.NETCore.UniversalWindowsPlatform. You'll want to update to the latest and also ensure you're using the 64 bit binder. For that, set <OutOfProcPDB>true</OutOfProcPDB> in your .csproj file. Not guaranteed to fix this, but it might help.

@michael-hawker
Copy link
Member Author

Using ItemsRepeater blocked by this WinUI issue: microsoft/microsoft-ui-xaml#3842

@michael-hawker michael-hawker force-pushed the llama/settingsexamples branch 3 times, most recently from 30429ae to c3fc324 Compare October 31, 2022 21:12
… instead of ItemsControl

This is the WinUI approach TabView/NavigationView uses and allows us to specify the entire Container in the DataTemplate.
This is needed for the ItemSource scenario of SettingsExpander to work properly.
We could also inherit from ContentControl instead of Control like NavigationView, but this was a simpler first step.
Some ItemsControl properties/methods are still missing, but initial approach seems to work and resolve issues with our same desired API surface.
New issue: Expander is shifting position when expanding for some reason now? (don't think it did that before)
Unsure if intended behavior, but seems like should file a WinUI issue.
Think this isn't going to work well when the window gets resized and how we handle that vs. layout cycle...
ItemsRepeater doesn't respect HorizontalAlignment and tries to stretch to take all available space, see: microsoft/microsoft-ui-xaml#3842
…ntrol

We can do everything back in the base implementation inherting from ItemsControl now that we understand what to do.
Copy link
Collaborator

@niels9001 niels9001 left a comment

Choose a reason for hiding this comment

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

Tested, jumping is gone :). Great work!

@niels9001 niels9001 merged commit 12890d4 into main Nov 1, 2022
@delete-merged-branch delete-merged-branch bot deleted the llama/settingsexamples branch November 1, 2022 09:28
@michael-hawker michael-hawker linked an issue Nov 2, 2022 that may be closed by this pull request
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

experiment 🧪 Used to track issues that are experiments (or their linked discussions)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🧪 [Experiment] SettingsCard & SettingsExpander

3 participants