microsoft-ui-xaml icon indicating copy to clipboard operation
microsoft-ui-xaml copied to clipboard

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

Open limefrogyank opened this issue 6 years ago • 21 comments

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:

  1. Clone XamlControlsGallery and update Microsoft.UI.Xaml to latest stable release (v2.0.181018003.1)
  2. Go to TreeViewPage.xaml under ControlPages.
  3. Go to third TreeView sample (A TreeView with DataBinding Using ItemSource)
  4. Add SelectionMode="Multiple" to TreeView
  5. Add IsSelected="{x:Bind IsSelected,Mode=TwoWay}" to the TreeViewItem
  6. Add Debug.WriteLine($"Selected changed: {value}"); to the set statement for IsSelected in the code-behind file. Include using statement for Debug.
  7. 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 avatar Dec 20 '18 21:12 limefrogyank

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

limefrogyank avatar Dec 20 '18 22:12 limefrogyank

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

jevansaks avatar Dec 24 '18 04:12 jevansaks

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: image

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.

Felix-Dev avatar Apr 27 '20 16:04 Felix-Dev

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>

tipa avatar May 12 '20 19:05 tipa

@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.

kaiguo avatar May 12 '20 22:05 kaiguo

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

tipa avatar May 13 '20 06:05 tipa

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.

kristianpinke avatar Apr 09 '21 12:04 kristianpinke

This is still a issue 17.02.2022. Please fix!

MrDeej avatar Feb 17 '22 08:02 MrDeej

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;

}

MrDeej avatar Feb 17 '22 09:02 MrDeej

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

w-ahmad avatar Feb 22 '22 00:02 w-ahmad

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

lukedukeus avatar Oct 03 '22 23:10 lukedukeus

This is ridiculous.

bogdan-patraucean avatar Oct 24 '22 20:10 bogdan-patraucean

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.

lukedukeus avatar Oct 24 '22 23:10 lukedukeus

Anyone fix this?

banbeviet avatar Nov 22 '22 09:11 banbeviet

Here's a workaround sample app just in case.

AndrewKeepCoding avatar Mar 22 '23 05:03 AndrewKeepCoding

Bump, is anyone looking into this issue or has it gone stale? @jevansaks

sitecompass avatar May 12 '23 00:05 sitecompass

Same here, broken, but with UWP almost done, not surprising.

jhwheuer avatar Aug 10 '23 10:08 jhwheuer

Using WinUI3. Same issue... is this not a key component to a working tree view using MVVM? any status update?

SoggyBottomBoy avatar Oct 15 '23 07:10 SoggyBottomBoy

Focus might work treeViewItem.Focus();

BilalBaydur avatar Nov 24 '23 11:11 BilalBaydur

In addition, SelectionChanged is not fired when SelectedItems is changed in multiple selection mode.

weitzhandler avatar Feb 26 '24 05:02 weitzhandler

This component is so bad, I have actually done my own, based on ListView, to keep things moving. Avoid TreeView.

jhwheuer avatar Feb 26 '24 07:02 jhwheuer

WinUI 3 now has a SelectionChanged event we can use. So, at least there's that.

bogdan-patraucean avatar Apr 05 '24 18:04 bogdan-patraucean