w-ahmad / WinUI.TableView

A light weight TableView component for WinUI3
MIT License
132 stars 9 forks source link

CellSelect throws if TableView was created on collapsed panel #35

Closed VisualAlf closed 2 months ago

VisualAlf commented 2 months ago

I need to swap two panels each having a TableView on it. So I defined one panels's Visbiilty as Collapsed. When I show this collapsed panel at runtime all seems OK, until I click a cell. Then a NPE is thrown, since obviously the ScrollViewer from the template is null. Happens first in TableView.GetColumnsInDisplay().

If you look at the VisualTree just before clicking, there sure is a Scrollviewer. However when I catch the Loaded Event on the TableView, there isn't any. So this maybe a WinUI-Problem. Anyway, Imho, the TableView. OnLoaded() should not simply return if there is no scrollviewer found.

Steps to Reproduce:

Here is a simple example:

MainWindow.xaml

<Window
    x:Class="TestTableView.MainWindow"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:TestTableView"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    xmlns:table="using:WinUI.TableView"

    mc:Ignorable="d">

    <Grid x:Name="RootGrid" Margin="32" RowSpacing="16"
          Loaded="RootGrid_Loaded" >
        <Grid.RowDefinitions >
            <RowDefinition Height="Auto" />
            <RowDefinition Height="Auto" />
            <RowDefinition Height="*" />
        </Grid.RowDefinitions>

        <TextBlock Text="Check for invisible" FontSize="18" />

        <StackPanel Orientation="Horizontal" Margin="16 0 0 0" Grid.Row="1" >
            <RadioButton x:Name="R1" Content="Show Table 1" Checked="RadioButton_Checked"/>
            <RadioButton x:Name="R2" Content="Show Table 2" Checked="RadioButton_Checked"/>
        </StackPanel>

        <StackPanel x:Name="SP1" Grid.Row="2" >
            <TextBlock Text="TableView 1" />

            <table:TableView x:Name="tableView1" AutoGenerateColumns="False"
                             SelectionMode="Single" SelectionUnit="Row" >
                <table:TableView.Columns>
                    <table:TableViewTextColumn Header="F1" Width="80" Binding="{Binding F1}" />
                    <table:TableViewTextColumn Header="F2" Width="80" Binding="{Binding F2}" />
                </table:TableView.Columns>
            </table:TableView>

        </StackPanel>

        <StackPanel x:Name="SP2" Grid.Row="2"
                    Visibility="Collapsed"
            >
            <TextBlock Text="TableView 2" />

            <table:TableView x:Name="tableView2" AutoGenerateColumns="False"
                             SelectionMode="Single" SelectionUnit="Row"
                             >
                <table:TableView.Columns>
                    <table:TableViewTextColumn Header="F2" Width="80" Binding="{Binding F2}" />
                    <table:TableViewTextColumn Header="F1" Width="80" Binding="{Binding F1}" />
                </table:TableView.Columns>
            </table:TableView>

        </StackPanel>

    </Grid>
</Window>

MainWindow.xaml.cs

public class TestItem
{
    public string F1 { get; set; } = "";
    public string F2 { get; set; } = "";
}

public sealed partial class MainWindow : Window
{
    public List<TestItem> TheList { get; set; } = [];

    public MainWindow()  { this.InitializeComponent();  }

    private void RootGrid_Loaded(object sender, RoutedEventArgs e)
    {
        List<TestItem> theList = [];
        for (int i = 1; i <= 5; i++)
            theList.Add(new TestItem { F1 = i.ToString(), F2 = (i * 10).ToString() }); 

        tableView1.ItemsSource = new ObservableCollection<TestItem>(theList);
        tableView2.ItemsSource = new ObservableCollection<TestItem>(theList);
        R1.IsChecked = true;
    }

    private void RadioButton_Checked(object sender, RoutedEventArgs e)
    {
        if (sender is not RadioButton rb || !RootGrid.IsLoaded ) return;
        if (rb.Name=="R1") {SP1.Visibility = Visibility.Visible; SP2.Visibility = Visibility.Collapsed; }
        if (rb.Name=="R2") { SP1.Visibility = Visibility.Collapsed; SP2.Visibility = Visibility.Visible; }
    }
}

Workaround:

Define both panels with Visibility="Visible". Hide tableView2 in the Loaded Event.

Expected behavior:

Obviously shouldn't throw.

Environment:

w-ahmad commented 2 months ago

🙌 Thanks, @VisualAlf! Investigating it now!

VisualAlf commented 2 months ago

@w-ahmad

According to the docs use of GetTemplateChild() should probably be restricted to OnAppyTemplate().

You can use the GetTemplateChild method inside your OnApplyTemplate override and keep a reference to the objects you need.

which may read as: outside of OnApplyTemplate() the function MAY work. And here we've found the case where it doesn't.

So, if you move the access to Scrollviewer into OnApplyTemplate() and later use only the reference to it, the bug is gone.

I had success with changing the methods:

    protected override void OnApplyTemplate()
    {
        base.OnApplyTemplate();

        _headerRow = GetTemplateChild("HeaderRow") as TableViewHeaderRow;

        // test
        if (GetTemplateChild("ScrollViewer") is ScrollViewer scrollViewer)
        {
            _scrollViewer = scrollViewer;
        }
        else
            Debugger.Break();

    }

    private void OnLoaded(object sender, RoutedEventArgs e)
    {

        //if (GetTemplateChild("ScrollViewer") is not ScrollViewer scrollViewer)
        //{
        //    return;
        //}

        //_scrollViewer = scrollViewer;
        // replace by:

        ScrollViewer scrollViewer;
        if (_scrollViewer == null)
            return;
        else
            scrollViewer = _scrollViewer;

        Canvas.SetZIndex(ItemsPanelRoot, -1);
...

You may consider -as a precaution- replace the other uses of GetTemplateChild() as well.

Thanks for your work !

w-ahmad commented 2 months ago

agreed, GetTemplateChild("ScrollViewer") should be moved to the OnApplyTemplate() method.