Skip to content

Commit bae2bde

Browse files
MrJulMrJul
committed
Keep SelectingItemsControl selection values until ItemsSource is set (#18634)
* Add failing tests for SelectedItem/SelectedIndex without an ItemsSource * Keep SelectedItem/SelectedIndex until ItemsSource is set * Add failing tests for setting SelectedValue without an ItemsSource * Keep SelectedValue until ItemsSource is set
1 parent 1dfa521 commit bae2bde

File tree

5 files changed

+177
-19
lines changed

5 files changed

+177
-19
lines changed

‎src/Avalonia.Controls/ItemCollection.cs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,10 @@ namespace Avalonia.Controls
1111
/// </summary>
1212
public class ItemCollection : ItemsSourceView, IList
1313
{
14-
// Suppress "Avoid zero-length array allocations": This is a sentinel value and must be unique.
15-
#pragma warning disable CA1825
16-
private static readonly object?[] s_uninitialized = new object?[0];
17-
#pragma warning restore CA1825
18-
1914
private Mode _mode;
2015

2116
internal ItemCollection()
22-
: base(s_uninitialized)
17+
: base(UninitializedSource)
2318
{
2419
}
2520

@@ -100,7 +95,7 @@ private IList WritableSource
10095
{
10196
if (IsReadOnly)
10297
ThrowIsItemsSource();
103-
if (Source == s_uninitialized)
98+
if (Source == UninitializedSource)
10499
SetSource(CreateDefaultCollection());
105100
return Source;
106101
}

‎src/Avalonia.Controls/ItemsSourceView.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ public class ItemsSourceView : IReadOnlyList<object?>,
2727
/// </summary>
2828
public static ItemsSourceView Empty { get; } = new ItemsSourceView(Array.Empty<object?>());
2929

30+
/// <summary>
31+
/// Gets an instance representing an uninitialized source.
32+
/// </summary>
33+
[SuppressMessage("Performance", "CA1825:Avoid zero-length array allocations", Justification = "This is a sentinel value and must be unique.")]
34+
[SuppressMessage("ReSharper", "UseCollectionExpression", Justification = "This is a sentinel value and must be unique.")]
35+
internal static object?[] UninitializedSource { get; } = new object?[0];
36+
3037
private IList _source;
3138
private NotifyCollectionChangedEventHandler? _collectionChanged;
3239
private NotifyCollectionChangedEventHandler? _preCollectionChanged;
@@ -49,6 +56,9 @@ public class ItemsSourceView : IReadOnlyList<object?>,
4956
/// </summary>
5057
public IList Source => _source;
5158

59+
internal IList? TryGetInitializedSource()
60+
=> _source == UninitializedSource ? null : _source;
61+
5262
/// <summary>
5363
/// Retrieves the item at the specified index.
5464
/// </summary>

‎src/Avalonia.Controls/Primitives/SelectingItemsControl.cs

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -592,10 +592,7 @@ protected override void OnInitialized()
592592
{
593593
base.OnInitialized();
594594

595-
if (_selection is object)
596-
{
597-
_selection.Source = ItemsView.Source;
598-
}
595+
TryInitializeSelectionSource(_selection, _updateState is null);
599596
}
600597

601598
/// <inheritdoc />
@@ -920,8 +917,8 @@ private ISelectionModel GetOrCreateSelectionModel()
920917

921918
private void OnItemsViewSourceChanged(object? sender, EventArgs e)
922919
{
923-
if (_selection is not null && _updateState is null)
924-
_selection.Source = ItemsView.Source;
920+
if (_updateState is null)
921+
TryInitializeSelectionSource(_selection, true);
925922
}
926923

927924
/// <summary>
@@ -1223,7 +1220,7 @@ private void InitializeSelectionModel(ISelectionModel model)
12231220
{
12241221
if (_updateState is null)
12251222
{
1226-
model.Source = ItemsView.Source;
1223+
TryInitializeSelectionSource(model, false);
12271224
}
12281225

12291226
model.PropertyChanged += OnSelectionModelPropertyChanged;
@@ -1258,6 +1255,32 @@ private void InitializeSelectionModel(ISelectionModel model)
12581255
}
12591256
}
12601257

1258+
private void TryInitializeSelectionSource(ISelectionModel? selection, bool shouldSelectItemFromSelectedValue)
1259+
{
1260+
if (selection is not null && ItemsView.TryGetInitializedSource() is { } source)
1261+
{
1262+
// InternalSelectionModel keeps the SelectedIndex and SelectedItem values before the ItemsSource is set.
1263+
// However, SelectedValue isn't part of that model, so we have to set the SelectedItem from
1264+
// SelectedValue manually now that we have a source.
1265+
//
1266+
// While this works, this is messy: we effectively have "lazy selection initialization" in 3 places:
1267+
// - UpdateState (all selection properties, for BeginInit/EndInit)
1268+
// - InternalSelectionModel (SelectedIndex/SelectedItem)
1269+
// - SelectedItemsControl (SelectedValue)
1270+
//
1271+
// There's the opportunity to have a single place responsible for this logic.
1272+
// TODO12 (or 13): refactor this.
1273+
if (shouldSelectItemFromSelectedValue && selection.SelectedIndex == -1 && selection.SelectedItem is null)
1274+
{
1275+
var item = FindItemWithValue(SelectedValue);
1276+
if (item != AvaloniaProperty.UnsetValue)
1277+
selection.SelectedItem = item;
1278+
}
1279+
1280+
selection.Source = source;
1281+
}
1282+
}
1283+
12611284
private void DeinitializeSelectionModel(ISelectionModel? model)
12621285
{
12631286
if (model is object)
@@ -1287,7 +1310,7 @@ private void EndUpdating()
12871310

12881311
if (_selection is InternalSelectionModel s)
12891312
{
1290-
s.Update(ItemsView.Source, state.SelectedItems);
1313+
s.Update(ItemsView.TryGetInitializedSource(), state.SelectedItems);
12911314
}
12921315
else
12931316
{
@@ -1296,7 +1319,7 @@ private void EndUpdating()
12961319
SelectedItems = state.SelectedItems.Value;
12971320
}
12981321

1299-
Selection.Source = ItemsView.Source;
1322+
TryInitializeSelectionSource(Selection, false);
13001323
}
13011324

13021325
if (state.SelectedValue.HasValue)

‎tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs

Lines changed: 86 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ public void Setting_SelectedIndex_Should_Set_SelectedItem()
441441
}
442442

443443
[Fact]
444-
public void Setting_SelectedIndex_Out_Of_Bounds_Should_Clear_Selection()
444+
public void Setting_SelectedIndex_Out_Of_Bounds_With_ItemsSource_Should_Clear_Selection()
445445
{
446446
var items = new[]
447447
{
@@ -462,11 +462,50 @@ public void Setting_SelectedIndex_Out_Of_Bounds_Should_Clear_Selection()
462462
}
463463

464464
[Fact]
465-
public void Setting_SelectedItem_To_Non_Existent_Item_Should_Clear_Selection()
465+
public void Setting_SelectedIndex_Out_Of_Bounds_Without_ItemsSource_Should_Keep_Selection_Until_ItemsSource_Is_Set()
466466
{
467467
var target = new SelectingItemsControl
468468
{
469-
Template = Template(),
469+
Template = Template()
470+
};
471+
472+
target.ApplyTemplate();
473+
target.SelectedIndex = 2;
474+
475+
Assert.Equal(2, target.SelectedIndex);
476+
477+
target.ItemsSource = Array.Empty<Item>();
478+
479+
Assert.Equal(-1, target.SelectedIndex);
480+
}
481+
482+
[Fact]
483+
public void Setting_SelectedIndex_Without_ItemsSource_Should_Keep_Selection_If_Index_Exists_When_ItemsSource_IsSet()
484+
{
485+
var target = new SelectingItemsControl
486+
{
487+
Template = Template()
488+
};
489+
490+
target.ApplyTemplate();
491+
target.SelectedIndex = 2;
492+
493+
Assert.Equal(2, target.SelectedIndex);
494+
495+
var items = new Item[] { new(), new(), new(), new() };
496+
target.ItemsSource = items;
497+
498+
Assert.Equal(2, target.SelectedIndex);
499+
Assert.Same(items[2], target.SelectedItem);
500+
}
501+
502+
[Fact]
503+
public void Setting_SelectedItem_To_Non_Existent_Item_With_ItemsSource_Should_Clear_Selection()
504+
{
505+
var target = new SelectingItemsControl
506+
{
507+
ItemsSource = Array.Empty<Item>(),
508+
Template = Template()
470509
};
471510

472511
target.ApplyTemplate();
@@ -476,6 +515,50 @@ public void Setting_SelectedItem_To_Non_Existent_Item_Should_Clear_Selection()
476515
Assert.Null(target.SelectedItem);
477516
}
478517

518+
[Fact]
519+
public void Setting_SelectedItem_To_Non_Existent_Item_Without_ItemsSource_Should_Keep_Selection_Until_ItemsSource_Is_Set()
520+
{
521+
var item = new Item();
522+
523+
var target = new SelectingItemsControl
524+
{
525+
Template = Template()
526+
};
527+
528+
target.ApplyTemplate();
529+
target.SelectedItem = item;
530+
531+
Assert.Equal(-1, target.SelectedIndex);
532+
Assert.Same(item, target.SelectedItem);
533+
534+
target.ItemsSource = Array.Empty<Item>();
535+
536+
Assert.Equal(-1, target.SelectedIndex);
537+
Assert.Null(target.SelectedItem);
538+
}
539+
540+
[Fact]
541+
public void Setting_SelectedItem_Without_ItemsSource_Should_Keep_Selection_If_Item_Exists_When_ItemsSource_IsSet()
542+
{
543+
var item = new Item();
544+
545+
var target = new SelectingItemsControl
546+
{
547+
Template = Template()
548+
};
549+
550+
target.ApplyTemplate();
551+
target.SelectedItem = item;
552+
553+
Assert.Equal(-1, target.SelectedIndex);
554+
Assert.Same(item, target.SelectedItem);
555+
556+
target.ItemsSource = new[] { new(), new(), item, new() };
557+
558+
Assert.Equal(2, target.SelectedIndex);
559+
Assert.Same(item, target.SelectedItem);
560+
}
561+
479562
[Fact]
480563
public void Adding_Selected_Item_Should_Update_Selection()
481564
{

‎tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_SelectedValue.cs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,53 @@ public void Setting_SelectedValue_Before_Initialize_Should_Retain_Selection()
170170
Assert.Equal(items[2].Name, sic.SelectedValue);
171171
}
172172

173+
[Fact]
174+
public void Setting_SelectedValue_To_Non_Existent_Item_Without_ItemsSource_Should_Keep_Selection_Until_ItemsSource_Is_Set()
175+
{
176+
var target = new SelectingItemsControl
177+
{
178+
Template = Template(),
179+
SelectedValueBinding = new Binding("Name")
180+
};
181+
182+
target.ApplyTemplate();
183+
target.SelectedValue = "Item2";
184+
185+
Assert.Equal(-1, target.SelectedIndex);
186+
Assert.Null(target.SelectedItem);
187+
Assert.Same("Item2", target.SelectedValue);
188+
189+
target.ItemsSource = Array.Empty<TestClass>();
190+
191+
Assert.Equal(-1, target.SelectedIndex);
192+
Assert.Null(target.SelectedItem);
193+
Assert.Null(target.SelectedValue);
194+
}
195+
196+
[Fact]
197+
public void Setting_SelectedValue_Without_ItemsSource_Should_Keep_Selection_If_Item_Exists_When_ItemsSource_IsSet()
198+
{
199+
var target = new SelectingItemsControl
200+
{
201+
Template = Template(),
202+
SelectedValueBinding = new Binding("Name")
203+
};
204+
205+
target.ApplyTemplate();
206+
target.SelectedValue = "Item2";
207+
208+
Assert.Equal(-1, target.SelectedIndex);
209+
Assert.Null(target.SelectedItem);
210+
Assert.Same("Item2", target.SelectedValue);
211+
212+
var items = TestClass.GetItems();
213+
target.ItemsSource = items;
214+
215+
Assert.Equal(2, target.SelectedIndex);
216+
Assert.Same(items[2], target.SelectedItem);
217+
Assert.Equal("Item2", target.SelectedValue);
218+
}
219+
173220
[Fact]
174221
public void Setting_SelectedValue_During_Initialize_Should_Take_Priority_Over_Previous_Value()
175222
{

0 commit comments

Comments
 (0)