Fix some issues on SelectingItemsControls when items or control changes visibility

src/Avalonia.Controls.ColorPicker/ColorView/ColorView.cs

@@ -4,15 +4,13 @@
 using Avalonia.Controls.Metadata;
 using Avalonia.Controls.Primitives;
 using Avalonia.Media;
-using Avalonia.Threading;
 
 namespace Avalonia.Controls
 {
     /// <summary>
     /// Presents a color for user editing using a spectrum, palette and component sliders.
     /// </summary>
     [TemplatePart("PART_HexTextBox", typeof(TextBox))]
-    [TemplatePart("PART_TabControl", typeof(TabControl))]
     public partial class ColorView : TemplatedControl
     {
         /// <summary>
@@ -22,7 +20,6 @@ public partial class ColorView : TemplatedControl
 
         // XAML template parts
         private TextBox?    _hexTextBox;
-        private TabControl? _tabControl;
 
         protected bool _ignorePropertyChanged = false;
 
@@ -77,99 +74,11 @@ private void SetColorToHexTextBox()
         /// Derived controls may re-implement this based on their default style / control template
         /// and any specialized selection needs.
         /// </remarks>
+        // TODO-13: Remove this unused method 
+        [Obsolete("The necessary validation is now handled by the TabControl. This method will be removed in the next major release.")]
         protected virtual void ValidateSelection()
         {
-            if (_tabControl != null &&
-                _tabControl.Items != null)
-            {
-                // Determine the number of visible tab items
-                int numVisibleItems = 0;
-                foreach (var item in _tabControl.Items)
-                {
-                    if (item is Control control &&
-                        control.IsVisible)
-                    {
-                        numVisibleItems++;
-                    }
-                }
-
-                // Verify the selection
-                if (numVisibleItems > 0)
-                {
-                    object? selectedItem = null;
-
-                    if (_tabControl.SelectedItem == null &&
-                        _tabControl.ItemCount > 0)
-                    {
-                        // As a failsafe, forcefully select the first item
-                        foreach (var item in _tabControl.Items)
-                        {
-                            selectedItem = item;
-                            break;
-                        }
-                    }
-                    else
-                    {
-                        selectedItem = _tabControl.SelectedItem;
-                    }
-
-                    if (selectedItem is Control selectedControl &&
-                        selectedControl.IsVisible == false)
-                    {
-                        // Select the first visible item instead
-                        foreach (var item in _tabControl.Items)
-                        {
-                            if (item is Control control &&
-                                control.IsVisible)
-                            {
-                                selectedItem = item;
-                                break;
-                            }
-                        }
-                    }
-
-                    _tabControl.SelectedItem = selectedItem;
-                    _tabControl.IsVisible = true;
-                }
-                else
-                {
-                    // Special case when all items are hidden
-                    // If TabControl ever properly supports no selected item /
-                    // all items hidden this can be removed
-                    _tabControl.SelectedItem = null;
-                    _tabControl.IsVisible = false;
-                }
-
-                // Hide the "tab strip" if there is only one tab
-                // This allows, for example, to view only the palette
-                /*
-                var itemsPresenter = _tabControl.FindDescendantOfType<ItemsPresenter>();
-                if (itemsPresenter != null)
-                {
-                    if (numVisibleItems == 1)
-                    {
-                        itemsPresenter.IsVisible = false;
-                    }
-                    else
-                    {
-                        itemsPresenter.IsVisible = true;
-                    }
-                }
-                */
-
-                // Note that if externally the SelectedIndex is set to 4 or something
-                // outside the valid range, the TabControl will ignore it and replace it
-                // with a valid SelectedIndex. This however is not propagated back through
-                // the TwoWay binding in the control template so the SelectedIndex and
-                // SelectedIndex become out of sync.
-                //
-                // The work-around for this is done here where SelectedIndex is forcefully
-                // synchronized with whatever the TabControl property value is. This is
-                // possible since selection validation is already done by this method.
-                SetCurrentValue(SelectedIndexProperty, _tabControl.SelectedIndex);
-            }
-
-            return;
+            // Method is now obsolete and is no longer implemented
         }
 
         /// <inheritdoc/>
@@ -182,7 +91,6 @@ protected override void OnApplyTemplate(TemplateAppliedEventArgs e)
             }
 
             _hexTextBox = e.NameScope.Find<TextBox>("PART_HexTextBox");
-            _tabControl = e.NameScope.Find<TabControl>("PART_TabControl");
 
             SetColorToHexTextBox();
 
@@ -193,7 +101,6 @@ protected override void OnApplyTemplate(TemplateAppliedEventArgs e)
             }
 
             base.OnApplyTemplate(e);
-            ValidateSelection();
         }
 
         /// <inheritdoc/>
@@ -260,27 +167,6 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang
                 // (Color will be coerced automatically if HsvColor changes)
                 SetCurrentValue(HsvColorProperty,  OnCoerceHsvColor(HsvColor));
             }
-            else if (change.Property == IsColorComponentsVisibleProperty ||
-                     change.Property == IsColorPaletteVisibleProperty ||
-                     change.Property == IsColorSpectrumVisibleProperty)
-            {
-                // When the property changed notification is received here the visibility
-                // of individual tab items has not yet been updated through the bindings.
-                // Therefore, the validation is delayed until after bindings update.
-                Dispatcher.UIThread.Post(() =>
-                {
-                    ValidateSelection();
-                }, DispatcherPriority.Background);
-            }
-            else if (change.Property == SelectedIndexProperty)
-            {
-                // Again, it is necessary to wait for the SelectedIndex value to
-                // be applied to the TabControl through binding before validation occurs.
-                Dispatcher.UIThread.Post(() =>
-                {
-                    ValidateSelection();
-                }, DispatcherPriority.Background);
-            }
 
             base.OnPropertyChanged(change);
         }

src/Avalonia.Controls/Primitives/SelectingItemsControl.cs

@@ -461,7 +461,7 @@ private protected override void OnItemsViewCollectionChanged(object? sender, Not
 
             if (AlwaysSelected && SelectedIndex == -1 && ItemCount > 0)
             {
-                SelectedIndex = 0;
+                SelectedIndex = GetFirstVisibleAndEnabledIndex();
             }
         }
 
@@ -534,6 +534,11 @@ protected internal override void ContainerForItemPreparedOverride(Control contai
 
             if (Selection.AnchorIndex == index)
                 KeyboardNavigation.SetTabOnceActiveElement(this, container);
+
+            if (AlwaysSelected && index == SelectedIndex && (!container.IsVisible || !container.IsEnabled))
+            {
+                MoveSelectionToFirstVisibleAndEnabledItem();
+            }
         }
 
         /// <inheritdoc />
@@ -616,6 +621,13 @@ protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs chang
             {
                 AutoScrollToSelectedItemIfNecessary(GetAnchorIndex());
             }
+            else if (change.Property == IsVisibleProperty)
+            {
+                if (change.GetNewValue<bool>())
+                {
+                    AutoScrollToSelectedItemIfNecessary(GetAnchorIndex());
+                }
+            }
             else if (change.Property == SelectionModeProperty && _selection is object)
             {
                 var newValue = change.GetNewValue<SelectionMode>();
@@ -1035,7 +1047,7 @@ private void OnSelectionModelLostSelection(object? sender, EventArgs e)
         {
             if (AlwaysSelected && ItemsView.Count > 0)
             {
-                SelectedIndex = 0;
+                SelectedIndex = GetFirstVisibleAndEnabledIndex();
             }
         }
 
@@ -1153,6 +1165,11 @@ Presenter is object &&
                 anchorIndex >= 0 &&
                 IsAttachedToVisualTree)
             {
+                if (!IsEffectivelyVisible)
+                {
+                    return;
+                }
+
                 Dispatcher.UIThread.Post(state =>
                 {
                     ScrollIntoView((int)state!);
@@ -1205,6 +1222,54 @@ private void MarkContainerSelected(Control container, bool selected)
             }
         }
 
+        /// <summary>
+        /// Finds the first visible and enabled index in the ItemsSource.
+        /// </summary>
+        /// <returns>the index of the first visible and enabled item, or -1 if none found</returns>
+        private int GetFirstVisibleAndEnabledIndex()
+        {
+            var count = ItemCount;
+            if (count == 0)
+                return -1;
+
+            for (var i = 0; i < count; i++)
+            {
+                var container = ContainerFromIndex(i);
+                if (container is not null)
+                {
+                    if (container is { IsVisible: true, IsEnabled: true })
+                        return i;
+                }
+                else
+                {
+                    var item = ItemsView[i];
+                    if (item is Visual v)
+                    {
+                        if (v.IsVisible && (v is not Control c || c.IsEnabled))
+                            return i;
+                    }
+                    else if (item is not null)
+                    {
+                        return i;
+                    }
+                }
+            }
+
+            return -1;
+        }
+
+        /// <summary>
+        /// this method moves selection to first visible and enabled item.
+        /// </summary>
+        private void MoveSelectionToFirstVisibleAndEnabledItem()
+        {
+            var index = GetFirstVisibleAndEnabledIndex();
+            if (index != -1 && index != SelectedIndex)
+            {
+                SelectedIndex = index;
+            }
+        }
+
         private void UpdateContainerSelection()
         {
             if (Presenter?.Panel is { } panel)
@@ -1251,7 +1316,7 @@ private void InitializeSelectionModel(ISelectionModel model)
 
             if (_updateState is null && AlwaysSelected && model.Count == 0)
             {
-                model.SelectedIndex = 0;
+                model.SelectedIndex = GetFirstVisibleAndEnabledIndex();
             }
 
             UpdateContainerSelection();
@@ -1358,7 +1423,7 @@ private void EndUpdating()
 
                 if (AlwaysSelected && SelectedIndex == -1 && ItemCount > 0)
                 {
-                    SelectedIndex = 0;
+                    SelectedIndex = GetFirstVisibleAndEnabledIndex();
                 }
             }
         }

tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_AutoSelect.cs

@@ -1,9 +1,11 @@
 using System.Collections.Generic;
 using System.Collections.Specialized;
+using System.Linq;
 using Avalonia.Collections;
 using Avalonia.Controls.Presenters;
 using Avalonia.Controls.Primitives;
 using Avalonia.Controls.Templates;
+using Avalonia.Threading;
 using Avalonia.UnitTests;
 using Xunit;
 
@@ -118,6 +120,48 @@ public void Removing_Selected_First_Item_Should_Select_Next_Item()
             Assert.Equal("bar", target.SelectedItem);
         }
 
+        [Fact]
+        public void AutoScrollToSelectedItem_Should_Work_When_Becoming_Visible()
+        {
+            using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface))
+            {
+                var items = Enumerable.Range(0, 100).Select(i => $"Item {i}").ToList();
+
+                var target = new ListBox
+                {
+                    Template = new FuncControlTemplate(CreateListBoxTemplate),
+                    ItemsSource = items,
+                    ItemTemplate = new FuncDataTemplate<string>((_, _) => new TextBlock { Height = 50 }),
+                    Height = 100,
+                    ItemsPanel = new FuncTemplate<Panel?>(() => new VirtualizingStackPanel { CacheLength = 0 }),
+                    AutoScrollToSelectedItem = true,
+                    IsVisible = false
+                };
+
+                target.Width = target.Height = 100;
+                var root = new TestRoot(target);
+                root.LayoutManager.ExecuteInitialLayoutPass();
+
+                // Select item 50
+                target.SelectedIndex = 50;
+
+                // Make visible
+                target.IsVisible = true;
+                target.UpdateLayout();
+
+                // Wait for dispatcher
+                Dispatcher.UIThread.RunJobs(null, TestContext.Current.CancellationToken);
+                target.UpdateLayout();
+
+                var scrollViewer = (ScrollViewer)target.VisualChildren[0];
+                var offset = scrollViewer.Offset.Y;
+
+                // Item 50 is at 50 * 50 = 2500. 
+                // ListBox height is 100, so it should be visible if offset is between 2400 and 2500.
+                Assert.True(offset > 0, $"Expected AutoScrollToSelectedItem to scroll to item 50, but offset was {offset}");
+            }
+        }
+
         private static FuncControlTemplate Template()
         {
             return new FuncControlTemplate<SelectingItemsControl>((control, scope) =>
@@ -148,5 +192,31 @@ private class ResetOnAdd : List<string>, INotifyCollectionChanged
                     new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
             }
         }
+
+        private Control CreateListBoxTemplate(TemplatedControl parent, INameScope scope)
+        {
+            return new ScrollViewer
+            {
+                Name = "PART_ScrollViewer",
+                Template = new FuncControlTemplate(CreateScrollViewerTemplate),
+                Content = new ItemsPresenter
+                {
+                    Name = "PART_ItemsPresenter",
+                    [~ItemsPresenter.ItemsPanelProperty] =
+                        ((ListBox)parent).GetObservable(ItemsControl.ItemsPanelProperty).ToBinding(),
+                }.RegisterInNameScope(scope)
+            }.RegisterInNameScope(scope);
+        }
+        private Control CreateScrollViewerTemplate(TemplatedControl parent, INameScope scope)
+        {
+            return new ScrollContentPresenter
+            {
+                Name = "PART_ContentPresenter",
+                [~ContentPresenter.ContentProperty] =
+                    parent.GetObservable(ContentControl.ContentProperty).ToBinding(),
+            }.RegisterInNameScope(scope);
+        }
+
+
     }
 }