LanceMcCarthy / MvpApi

An application for Microsoft MVPs to easily browse and upload contributions
MIT License
36 stars 10 forks source link

Additional Technology Areas Not Preselected in ListView #49

Closed LanceMcCarthy closed 5 years ago

LanceMcCarthy commented 6 years ago

When a contribution is loaded into the DetailsPage, the AdditionalTechnologyAreas collection is properly populated and has any additional selected areas for that item

However, in the UI, the Additional Technology Areas Flyout's ListView doesn't show the item preselected

I suspect a bug in the SelectedValues attached property I wrote to handle two-way binding the ListView's SelectedItems

UI https://github.com/LanceMcCarthy/MvpApi/blob/88731d9edbb9635e6c441bc90a7c5fb4e37b44a7/MvpApi.Uwp/Views/ContributionDetailPage.xaml#L88-L114

ViewModel https://github.com/LanceMcCarthy/MvpApi/blob/multiple-categories/MvpApi.Uwp/ViewModels/ContributionDetailViewModel.cs

Attached Property Extension https://github.com/LanceMcCarthy/MvpApi/blob/multiple-categories/MvpApi.Uwp/Extensions/ListViewExtensions.cs

LanceMcCarthy commented 6 years ago

Fixed by completely rewriting the approach as a behavior, with additional checks for changes on both sides.

public class SelectedItemsBindingBehavior : Behavior<ListViewBase>
    {
        private bool selectionChangedInProgress;

        protected override void OnAttached()
        {
            base.OnAttached();
            AssociatedObject.SelectionChanged += OnListViewSelectionChanged;
        }

        protected override void OnDetaching()
        {
            base.OnDetaching();
            AssociatedObject.SelectionChanged -= OnListViewSelectionChanged;
        }

        public static readonly DependencyProperty SelectedItemsProperty = DependencyProperty.Register(
            "SelectedItems",
            typeof(IList),
            typeof(SelectedItemsBindingBehavior),
            new PropertyMetadata(new ObservableCollection<object>(), PropertyChangedCallback));

        public IList SelectedItems
        {
            get => (IList)GetValue(SelectedItemsProperty);
            set => SetValue(SelectedItemsProperty, value);
        }

        private static void PropertyChangedCallback(DependencyObject o, DependencyPropertyChangedEventArgs args)
        {
            if (o is SelectedItemsBindingBehavior behavior)
            {
                void Handler(object s, NotifyCollectionChangedEventArgs e) => OnBoundSelectedItemsChanged(behavior, e);

                if (args.OldValue is INotifyCollectionChanged objects)
                {
                    objects.CollectionChanged -= Handler;

                    behavior.AssociatedObject.SelectedItems.Clear();
                }

                if (args.NewValue is INotifyCollectionChanged collection)
                {
                    foreach (var item in behavior.SelectedItems)
                    {
                        if (!behavior.AssociatedObject.SelectedItems.Contains(item))
                        {
                            behavior.AssociatedObject.SelectedItems.Add(item);
                        }
                    }

                    collection.CollectionChanged += Handler;
                }
            }
        }

        /// <summary>
        /// Handles selection changes in the ListView
        /// </summary>
        /// <param name="sender"></param>
        /// <param name="e"></param>
        private void OnListViewSelectionChanged(object sender, SelectionChangedEventArgs e)
        {
            if (selectionChangedInProgress)
                return;

            selectionChangedInProgress = true;

            foreach (var item in e.RemovedItems)
            {
                if (SelectedItems.Contains(item))
                {
                    SelectedItems.Remove(item);
                }
            }

            foreach (var item in e.AddedItems)
            {
                if (!SelectedItems.Contains(item))
                {
                    SelectedItems.Add(item);
                }
            }

            selectionChangedInProgress = false;
        }

        /// <summary>
        /// Handles item changes in the bound collection
        /// </summary>
        /// <param name="sender"></param>
        /// <param name="e"></param>
        private static void OnBoundSelectedItemsChanged(object sender, NotifyCollectionChangedEventArgs e)
        {
            if (sender is SelectedItemsBindingBehavior behavior)
            {
                var listSelectedItems = behavior.AssociatedObject.SelectedItems;

                if (e.OldItems != null)
                {
                    foreach (var item in e.OldItems)
                    {
                        if (listSelectedItems.Contains(item))
                        {
                            listSelectedItems.Remove(item);
                        }
                    }
                }

                if (e.NewItems != null)
                {
                    foreach (var item in e.NewItems)
                    {
                        if (!listSelectedItems.Contains(item))
                        {
                            listSelectedItems.Add(item);
                        }
                    }
                }
            }
        }
    }
LanceMcCarthy commented 6 years ago

Update: The bug still exists. The problem appears to be with using a CollectionViewSource, adding items to the SelectedItemsCollection is problematic

LanceMcCarthy commented 6 years ago

Created a new control, TechnologyAreasListView, that can handle the special syncing use case for this.

The bug appears to only be a styling problem, because the SelectedItems collection has the right number of items in it. The problem is with this specific ListView on that specific page (ContributionDetailPage)

LanceMcCarthy commented 5 years ago

Not fixable at this time. Used alternate UI design to implement same functionality