TreeViewItem doesn't update IsSelected in SelectionMode=Multiple when clicking on Checkbox or content

limefrogyank commented 5 years ago

Describe the bug When you have a TreeView using ItemsSource binding and set to SelectionMode="Multiple", the IsSelected property is not changed when clicking on the checkbox. Also, the checkbox is not changed when clicking on the item, either.

Steps to reproduce the bug

Steps to reproduce the behavior:

Clone XamlControlsGallery and update Microsoft.UI.Xaml to latest stable release (v2.0.181018003.1)
Go to TreeViewPage.xaml under ControlPages.
Go to third TreeView sample (A TreeView with DataBinding Using ItemSource)
Add SelectionMode="Multiple" to TreeView
Add IsSelected="{x:Bind IsSelected,Mode=TwoWay}" to the TreeViewItem
Add Debug.WriteLine($"Selected changed: {value}"); to the set statement for IsSelected in the code-behind file. Include using statement for Debug.
Run sample, go to TreeView page, and try clicking on the checkboxes, then the item contents instead.

Expected behavior The debug output should show Selected changed: true when the checkbox is checked and Selected changed: false when unchecked. Also, the checkbox should be checked/unchecked when the content of the TreeViewItem is toggled.

Screenshots

Version Info

NuGet package version: Microsoft.UI.Xaml v2.0.181018003.1

Windows 10 version:

[x] Insider Build (1809)
[ ] October 2018 Update (17763)
[ ] April 2018 Update (17134)
[ ] Fall Creators Update (16299)
[ ] Creators Update (15063)
[ ] Anniversary Update (14393)

Device form factor:

[x] Desktop
[ ] Mobile
[ ] Xbox
[ ] Surface Hub
[ ] IoT

Additional context

limefrogyank commented 5 years ago

I tried some hacky workarounds to see if I could use Multiple mode, but everytime I changed my IsSelected property on my viewmodel to True, the other nodes would immediately all be set to False

jevansaks commented 5 years ago

Thanks for the report! Our team is on holiday break right now but we'll take a look when we return!

Felix-Dev commented 4 years ago

I took a quick look at this and the culprit here seems to be the call to ListView's::PrepareContainerForItemOverride call here. When we data bind a collection to the TreeView (using ItemsSource), set the TreeView to SelectionMode::Multiple and set a (two-way) bound IsSelected property in our bound data collection items, this call to ListView's PrepareContainerForItemOverride() sets the IsSelected property of the bound data item to false.

This happens because we set the SelectionMode of the ListView to None in multi-selection mode so there is no concept of "selecting an item" for the ListView. The ListView will thus set the IsSelected property of the ItemContainer to false which is then reported back to the bound item.

As such, I wrote a quick guard against this call when we are using data binding in SelectionMode::Multiple:

// Don't call ListView's base method when we are using data binding and are in SelectionMode::Multiple
if (!IsContentMode() || !m_isMultiselectEnabled)
{
    __super::PrepareContainerForItemOverride(element, item);
}

With additional logic to handle checkbox selection and updating the data-bound IsSelected properties, using IsSelected property binding to select treeview items now works partly. The issue I'm currently facing is this one:

Suppose we have the following data collection we bind to:

private ObservableCollection<TreeViewItemSource> PrepareItemsSource(bool expandRootNode = false)
{
    var root0 = new TreeViewItemSource() { Content = "Root.0", IsSelected = true };
    var root = new TreeViewItemSource() { Content = "Root", Children = { root0 } };

    // TestTreeViewItemsSource
    return new ObservableCollection<TreeViewItemSource>{root};
}

We have a root item (Root) with one child (Root.0). The child has its IsSelected property set to true and we use data binding like this:

<muxcontrols:TreeView 
    ItemsSource="{x:Bind TestTreeViewItemsSource}"
    SelectionMode="Multiple">
    <muxcontrols:TreeView.ItemTemplate>
        <DataTemplate x:DataType="local:TreeViewItemSource">
            <muxcontrols:TreeViewItem 
                ItemsSource="{x:Bind Children}"
                Content="{x:Bind Content}"
                IsSelected="{x:Bind IsSelected, Mode=TwoWay}"/>
        </DataTemplate>
    </muxcontrols:TreeView.ItemTemplate>
</muxcontrols:TreeView>

(Code is a slightly modified version of the MUXControlsTestApp TreeView test code.)

Important here is that root TreeViewItem is not set to be expanded (IsExpanded = false). The expected look is this:

However, the actual behavior is this one: treeview-isselected-issue

We first need to actually expand the root item for it to be marked as selected. This is the case because there is no TreeViewItem created for its child (Root.0) yet. A TreeViewItem is only created for the child once we request it to be shown (by expanding its parent). Once it is created, the registered property changed callback for its IsSelected property is called which in turn updates the selection state of its parent and its children (if any). Thus, Root will be set to selected state.

So then, the question I now have...how can we proceed from here? Seems like we currently need the TreeViewItem's children to be able to set the correct selection state on its parent yet we don't have those available when the parent is not in an expanded state initially.

tipa commented 4 years ago

I spent the whole day wrapping my head around what seems to be the same issue as this one until I finally found this issue. I am also using databinding and the selection state seems to be very unreliable - usually only one checkbox is selected even though multiple should be ticked.

Would be awesome if this could be fixed at some point - looks like the issue is already pretty old. If you need a repro project, let me know and I can make one.

<muxc:TreeView x:Name="treeView" ItemsSource="{x:Bind odFolders}" Expanding="OneDriveFolderView_Expanding"
               CanDragItems="False" CanReorderItems="False" SelectionMode="Multiple">
    <muxc:TreeView.ItemTemplate>
        <DataTemplate x:DataType="local:ODFolder">
            <muxc:TreeViewItem ItemsSource="{x:Bind Children, Mode=OneWay}" IsSelected="{x:Bind IsChosen}"
                          IsExpanded="{x:Bind IsExpanded}" Content="{x:Bind DisplayPath}" Padding="0,0,8,0"
                          HasUnrealizedChildren="{x:Bind HasUnrealizedChildren, Mode=OneWay}"/>
        </DataTemplate>
    </muxc:TreeView.ItemTemplate>
</muxc:TreeView>

kaiguo commented 4 years ago

@Felix-Dev's observation is correct, the rudimentary cause of this problem is the underlying ListView's selection mode being set to none. Skipping the "super" call will cause bunch of other unexpected behaviors. To fix the issue we probably need to either make changes in ListView to make it comply with TreeView or swap out the underlying implementation using ItemsRepeater.

The current workaround is using selection Apis without data binding.

tipa commented 4 years ago

Thanks, that workaround seems to work. I can still use databinding, but have to manually add the item to the SelectedItems list

kristianpinke commented 3 years ago

Thanks, that workaround seems to work. I can still use databinding, but have to manually add the item to the SelectedItems list

@tipa What do you mean by "I can still use databinding"? Because when I do IsSelected="{x:Bind IsChecked, Mode=TwoWay}" IsChecked does not gets updated whenever I make changes on the UI. Please don't get me wrong, I understand that I need to modify SelectedNodes to reflect UI changes from the code, but I want to have information in my ViewModel about IsSelected changes that were made on UI. Did you have any luck to have this working?

This is not the case for IsExpanded (IsExpanded="{x:Bind IsOpened, Mode=TwoWay}"). That works as expected.

@kaiguo Sample project where the issue occurs.

MrDeej commented 2 years ago

This is still a issue 17.02.2022. Please fix!

MrDeej commented 2 years ago

It is annoing that I can make a workaround to databind the threeview this easy:

<parentTypes:VfaPage
    x:Class="WinUI_Vfa_Div.Pages.Leverandor.AnalyseHv.LeverandorAnalyseHv"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:WinUI_Vfa_Div.Pages.Leverandor.AnalyseHv"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    xmlns:parentTypes="using:WinUI_Vfa_Div.ParentTypes"
    mc:Ignorable="d"
    Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
    <parentTypes:VfaPage.DataContext>
        <local:LeverandorAnalyseHvVm x:Name="ViewModel"/>
    </parentTypes:VfaPage.DataContext>
    <Grid>
        <TreeView x:Name="TreeView" Grid.Row="1" ItemsSource="{x:Bind ViewModel.DtoKategoriTre,Mode=OneWay}"  >
            <TreeView.ItemTemplate>
                <DataTemplate x:DataType="local:DtoKategoriTre">
                    <TreeViewItem  ItemsSource="{x:Bind UnderKategorier}" 
                                   >
                        <CheckBox IsChecked="{x:Bind ErValgt,Mode=TwoWay,UpdateSourceTrigger=PropertyChanged}"

                                  Content="{x:Bind KategoriNavn}"/>
                    </TreeViewItem>
                </DataTemplate>
            </TreeView.ItemTemplate>
        </TreeView>

    </Grid>
</parentTypes:VfaPage>

public class DtoKategoriTre : ParentTypes.StandardPropertyChanged
  {
      private bool? erValgt;

      public bool? ErValgt
      {
          get => erValgt;
          set
          {
              if (value == erValgt)
                  return;

              erValgt = value;
              NotifyPropertyChanged();

              if (UnderKategorier != null && value != null)
              {
                  foreach (var rad in UnderKategorier)
                      rad.ErValgt = value;
              }

              if (DtoParent is { UnderKategorier: { } })
              {
                  if (DtoParent.UnderKategorier.All(a => a.ErValgt != null && (bool)a.ErValgt))
                      DtoParent.ErValgt = true;
                  else if (DtoParent.UnderKategorier.All(a => a.ErValgt != null && !(bool)a.ErValgt))
                      DtoParent.ErValgt = false;
                  else
                      DtoParent.ErValgt = null;
              }

          }
      }

      public string KategoriNavn { get; set; } = default!;

      public int KategoriCode { get; set; } = default!;

      public DtoKategoriTre[]? UnderKategorier { get; set; }

      public DtoKategoriTre? DtoParent { get; set; }
  }

public class LeverandorAnalyseHvVm : ParentTypes.StandardVm
{

    private DtoKategoriTre[]? dtoKategoriTre;

    public DtoKategoriTre[]? DtoKategoriTre
    {
        get => dtoKategoriTre;
        set
        {
            if (value == dtoKategoriTre)
                return;

            dtoKategoriTre = value;
            NotifyPropertyChanged();
        }
    }

}


public static async Task<DtoKategoriTre[]> GetKategoriTre(CancellationToken cancellationToken)
{
    var req = new getKategorierRequest();

    var rep = await MainWindow.MainWindow.ProductClientHelper.GetClient().GetDtoCategoriesAsync(req, null, null, cancellationToken);

    var defaultCodesExcluded = new[] { 990, 80, 70, 900 };
    var a = (from main in rep.MsgMains
             join sub in rep.MsgSubs on main.Code equals sub.MainCategoryCode
                 into joinedSub
             select new DtoKategoriTre()
             {
                 KategoriNavn = main.Name,
                 KategoriCode = main.Code,
                 ErValgt = !defaultCodesExcluded.Contains(main.Code),
                 UnderKategorier = joinedSub.Select(a => new DtoKategoriTre()
                 {
                     ErValgt = !defaultCodesExcluded.Contains(main.Code),
                     KategoriNavn = a.Name,
                     KategoriCode = a.Code
                 }).ToArray()
             }).ToArray();

    foreach (var rad in a)
    {
        if (rad.UnderKategorier != null)
            foreach (var rad2 in rad.UnderKategorier)
                rad2.DtoParent = rad;
    }

    return a;

}

w-ahmad commented 2 years ago

This issue is causing a big roadblock for my ongoing project... please fix it

lukedukeus commented 1 year ago

How is this still an issue after four years? please fix it.

bogdan-patraucean commented 1 year ago

This is ridiculous.

lukedukeus commented 1 year ago

Here is the workaround I came up with:

I have a class Entity.cs that my make up the nodes of my treeview:

public abstract class Entity : ObservableRecipient
    {
        private bool _selected = false;
        public bool Selected { get { return _selected; } set { SetProperty(ref _selected, value); } }

        private string _name;
        public virtual string Name { get { return _name; } set { SetProperty(ref _name, value); } }

        public int ID { get; set; }
        public virtual ObservableCollection<Entity> Children { get; set; } = new ObservableCollection<Entity>();
    }

Then in View.xaml:

<TreeView   
      x:Name="EntitiesTreeView"
      ItemsSource="{x:Bind ViewModel.Entities, Mode=OneWay}"
  />

View.xaml.cs:

    public sealed partial class EntitiesTreeViewComponent : UserControl
    {
        public EntitiesTreeViewViewModel ViewModel { get; }

        public EntitiesTreeViewComponent()
        {
            ViewModel = App.GetService<EntitiesTreeViewViewModel>();

            this.InitializeComponent();

            DispatcherQueue dispatcherQueue = DispatcherQueue.GetForCurrentThread();
            Thread CheckingThread = new Thread(() => CheckThread(dispatcherQueue));
            CheckingThread.Start();
        }

        private async void CheckThread(DispatcherQueue dispatcherQueue)
        {
            if (dispatcherQueue != null)
            {
                while (true)
                {
                    await Task.Run(async () =>
                    {
                        await dispatcherQueue.EnqueueAsync(() =>
                        {
                            if (EntitiesTreeView.SelectedItems != null)
                            {
                                Entity root = ((ObservableCollection<Entity>)EntitiesTreeView.ItemsSource).FirstOrDefault();

                                if (root != null)
                                {
                                    List<Entity> entities = EntitiesTreeView.SelectedItems.Cast<Entity>().ToList();
                                    SetItemSelected(root, entities);

                                    ViewModel.HasSelected = entities.Count > 0;
                                }
                            }

                        });

                    });

                    await Task.Delay(100);

                }
            }
        }

        private void SetItemSelected(Entity root, List<Entity> selectedItems)
        {
            if (selectedItems.Count > 0)
            {
                root.Selected = selectedItems.Where(x => x.ID == root.ID && x.Name == root.Name).Any();

                List<Entity> rest = selectedItems.Where(x => x != root).ToList();

                foreach (Entity child in root.Children)
                {
                    SetItemSelected(child, rest);
                }
            }
        }
    }

ViewModel.cs:

    public class EntitiesTreeViewViewModel : ObservableRecipient
    {
        public ObservableCollection<Entity> Entities { get; set; } = new ObservableCollection<Entity>();

        private bool _hasSelected;
        public bool HasSelected { get { return _hasSelected; } set { SetProperty(ref _hasSelected, value); } }
    }

This spins up a thread that constantly checks what items are selected, and updates a selected prop on what you are binding to. It's inefficient and destroys the purpose of using MVVM, but the best I could come up with, because the selection events also don't work / aren't comprehensive.