Skip to content

Commit

Permalink
Merge pull request ppy#6061 from frenzibyte/dropdown-handle-bindable-…
Browse files Browse the repository at this point in the history 
…list-properly

Add proper handling for `BindableList` collection changes in dropdown
  • Loading branch information
@bdach
bdach authored Dec 6, 2023
2 parents 5b64428 + 7feb8b1 commit 5a45622
Show file tree
Hide file tree
Showing 3 changed files with 198 additions and 55 deletions.
69 changes: 61 additions & 8 deletions osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,25 +259,78 @@ public void TestItemSource()

AddStep("setup dropdown", () => testDropdown = createDropdown());

AddStep("bind source", () =>
{
// todo: perhaps binding ItemSource should clear existing items.
testDropdown.ClearItems();
testDropdown.ItemSource = bindableList = new BindableList<TestModel?>();
});
AddStep("bind source", () => testDropdown.ItemSource = bindableList = new BindableList<TestModel?>());

toggleDropdownViaClick(() => testDropdown);

AddAssert("no elements in dropdown", () => !testDropdown.Items.Any());

AddStep("add items to bindable", () => bindableList.AddRange(new[] { "one", "two", "three" }.Select(s => new TestModel(s))));
AddStep("add items to bindable", () => bindableList.AddRange(new[] { "one", "2", "three" }.Select(s => new TestModel(s))));
AddStep("select 'three'", () => testDropdown.Current.Value = "three");

AddStep("replace '2' with 'two'", () => bindableList.ReplaceRange(1, 1, new TestModel[] { "two" }));
checkOrder(1, "two");

AddStep("remove 'one' from bindable", () => bindableList.RemoveAt(0));
AddAssert("two items in dropdown", () => testDropdown.Items.Count() == 2);
AddAssert("current value is still 'three'", () => testDropdown.Current.Value?.Identifier == "three");

AddStep("remove three", () => bindableList.Remove("three"));
AddStep("remove 'three'", () => bindableList.Remove("three"));
AddAssert("current value is 'two'", () => testDropdown.Current.Value?.Identifier == "two");

AddStep("add 'one' and 'three'", () =>
{
bindableList.Insert(0, "one");
bindableList.Add("three");
});

checkOrder(0, "one");
checkOrder(2, "three");

AddStep("add 'one-half'", () => bindableList.Add("one-half"));
AddStep("move 'one-half'", () => bindableList.Move(3, 1));
checkOrder(1, "one-half");
checkOrder(2, "two");

void checkOrder(int index, string item) => AddAssert($"item #{index + 1} is '{item}'",
() => testDropdown.ChildrenOfType<FillFlowContainer<Menu.DrawableMenuItem>>().Single().FlowingChildren.Cast<Menu.DrawableMenuItem>().ElementAt(index).Item.Text.Value.ToString(),
() => Is.EqualTo(item));
}

[Test]
public void TestItemReplacementDoesNotAffectScroll()
{
TestDropdown testDropdown = null!;
BindableList<TestModel?> bindableList = null!;

AddStep("setup dropdown", () => testDropdown = createDropdown());

AddStep("bind source", () => testDropdown.ItemSource = bindableList = new BindableList<TestModel?>());
AddStep("add many items", () => bindableList.AddRange(Enumerable.Range(0, 20).Select(i => (TestModel)$"test {i}")));
AddStep("set max height", () => testDropdown.Menu.MaxHeight = 100);

toggleDropdownViaClick(() => testDropdown);

AddStep("scroll to middle", () => testDropdown.ChildrenOfType<BasicScrollContainer>().Single().ScrollTo(200));
AddStep("replace item in middle", () => bindableList.ReplaceRange(10, 1, new TestModel[] { "test ten" }));
AddAssert("scroll is unchanged", () => testDropdown.ChildrenOfType<BasicScrollContainer>().Single().Target == 200);
}

[Test]
public void TestClearItemsInBindableWhileNotPresent()
{
TestDropdown testDropdown = null!;
BindableList<TestModel?> bindableList = null!;

AddStep("setup dropdown", () => testDropdown = createDropdown());

AddStep("bind source", () => testDropdown.ItemSource = bindableList = new BindableList<TestModel?>());
AddStep("add many items", () => bindableList.AddRange(Enumerable.Range(0, 20).Select(i => (TestModel)$"test {i}")));

AddStep("hide dropdown", () => testDropdown.Hide());
AddStep("clear items", () => bindableList.Clear());
AddStep("show dropdown", () => testDropdown.Show());
AddAssert("dropdown menu empty", () => !testDropdown.Menu.DrawableMenuItems.Any());
}

/// <summary>
Expand Down
142 changes: 101 additions & 41 deletions osu.Framework/Graphics/UserInterface/Dropdown.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@

using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Diagnostics;
using System.Linq;
using osu.Framework.Bindables;
using osu.Framework.Extensions;
using osu.Framework.Extensions.IEnumerableExtensions;
using osu.Framework.Extensions.ObjectExtensions;
using osu.Framework.Graphics.Containers;
using osu.Framework.Graphics.Sprites;
using osu.Framework.Input;
Expand Down Expand Up @@ -57,21 +59,6 @@ public IEnumerable<T> Items
}
}

private void setItems(IEnumerable<T> items)
{
clearItems();
if (items == null)
return;

foreach (var entry in items)
addDropdownItem(entry);

if (Current.Value == null || !itemMap.Keys.Contains(Current.Value, EqualityComparer<T>.Default))
Current.Value = itemMap.Keys.FirstOrDefault();
else
Current.TriggerChange();
}

private readonly IBindableList<T> itemSource = new BindableList<T>();
private IBindableList<T> boundItemSource;

Expand All @@ -89,9 +76,24 @@ public IBindableList<T> ItemSource

if (boundItemSource != null) itemSource.UnbindFrom(boundItemSource);
itemSource.BindTo(boundItemSource = value);

setItems(value);
}
}

private void setItems(IEnumerable<T> value)
{
clearItems();

if (value == null)
return;

foreach (var entry in value)
addDropdownItem(entry);

ensureItemSelectionIsValid();
}

/// <summary>
/// Add a menu item directly while automatically generating a label.
/// </summary>
Expand All @@ -104,7 +106,7 @@ public void AddDropdownItem(T value)
addDropdownItem(value);
}

private void addDropdownItem(T value)
private void addDropdownItem(T value, int? position = null)
{
if (itemMap.ContainsKey(value))
throw new ArgumentException($"The item {value} already exists in this {nameof(Dropdown<T>)}.");
Expand All @@ -121,7 +123,11 @@ private void addDropdownItem(T value)
if (LoadState >= LoadState.Ready)
item.Text.Value = GenerateItemText(value);

Menu.Add(item);
if (position != null)
Menu.Insert(position.Value, item);
else
Menu.Add(item);

itemMap[value] = item;
}

Expand All @@ -147,7 +153,6 @@ private bool removeDropdownItem(T value)

Menu.Remove(item);
itemMap.Remove(value);

return true;
}

Expand Down Expand Up @@ -221,15 +226,15 @@ protected Dropdown()
Header.Action = Menu.Toggle;
Header.ChangeSelection += selectionKeyPressed;
Menu.PreselectionConfirmed += preselectionConfirmed;
Current.ValueChanged += val => Scheduler.AddOnce(selectionChanged, val);
Current.ValueChanged += val => Scheduler.AddOnce(updateItemSelection, val.NewValue);
Current.DisabledChanged += disabled =>
{
Header.Enabled.Value = !disabled;
if (disabled && Menu.State == MenuState.Open)
Menu.State = MenuState.Closed;
};

ItemSource.CollectionChanged += (_, _) => setItems(itemSource);
ItemSource.CollectionChanged += collectionChanged;
}

private void preselectionConfirmed(int selectedIndex)
Expand Down Expand Up @@ -286,20 +291,76 @@ protected override void LoadComplete()
Header.Label = SelectedItem?.Text.Value ?? default;
}

private void selectionChanged(ValueChangedEvent<T> args)
private void collectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
// refresh if SelectedItem and SelectedValue mismatched
// null is not a valid value for Dictionary, so neither here
if (args.NewValue == null && SelectedItem != null)
switch (e.Action)
{
selectedItem = new DropdownMenuItem<T>(default(LocalisableString), default);
}
else if (SelectedItem == null || !EqualityComparer<T>.Default.Equals(SelectedItem.Value, args.NewValue))
{
if (args.NewValue == null || !itemMap.TryGetValue(args.NewValue, out selectedItem))
case NotifyCollectionChangedAction.Add:
{
selectedItem = new DropdownMenuItem<T>(GenerateItemText(args.NewValue), args.NewValue);
var newItems = e.NewItems.AsNonNull().Cast<T>().ToArray();

for (int i = 0; i < newItems.Length; i++)
addDropdownItem(newItems[i], e.NewStartingIndex + i);

break;
}

case NotifyCollectionChangedAction.Remove:
foreach (var item in e.OldItems.AsNonNull().Cast<T>())
removeDropdownItem(item);

break;

case NotifyCollectionChangedAction.Move:
{
var item = Items.ElementAt(e.OldStartingIndex);
removeDropdownItem(item);
addDropdownItem(item, e.NewStartingIndex);
break;
}

case NotifyCollectionChangedAction.Replace:
{
foreach (var item in e.OldItems.AsNonNull().Cast<T>())
removeDropdownItem(item);

var newItems = e.NewItems.AsNonNull().Cast<T>().ToArray();

for (int i = 0; i < newItems.Length; i++)
addDropdownItem(newItems[i], e.NewStartingIndex + i);

break;
}

case NotifyCollectionChangedAction.Reset:
clearItems();
break;
}

ensureItemSelectionIsValid();
}

private void ensureItemSelectionIsValid()
{
if (Current.Value == null || !itemMap.ContainsKey(Current.Value))
{
Current.Value = itemMap.Keys.FirstOrDefault();
return;
}

updateItemSelection(Current.Value);
}

private void updateItemSelection(T value)
{
if (value != null && itemMap.TryGetValue(value, out var existingItem))
selectedItem = existingItem;
else
{
if (value == null && selectedItem != null)
selectedItem = new DropdownMenuItem<T>(default(LocalisableString), default);
else
selectedItem = new DropdownMenuItem<T>(GenerateItemText(value), value);
}

Menu.SelectItem(selectedItem);
Expand Down Expand Up @@ -366,14 +427,6 @@ protected DropdownMenu()
StateChanged += clearPreselection;
}

public override void Add(MenuItem item)
{
base.Add(item);

var drawableDropdownMenuItem = (DrawableDropdownMenuItem)ItemsContainer.Single(drawableItem => drawableItem.Item == item);
drawableDropdownMenuItem.PreselectionRequested += PreselectItem;
}

private void clearPreselection(MenuState obj)
{
if (obj == MenuState.Closed)
Expand All @@ -396,8 +449,9 @@ public void SelectItem(DropdownMenuItem<T> item)
{
Children.OfType<DrawableDropdownMenuItem>().ForEach(c =>
{
bool wasSelected = c.IsSelected;
c.IsSelected = compareItemEquality(item, c.Item);
if (c.IsSelected)
if (c.IsSelected && !wasSelected)
ContentContainer.ScrollIntoView(c);
});
}
Expand Down Expand Up @@ -429,13 +483,19 @@ protected void PreselectItem(MenuItem item)
{
Children.OfType<DrawableDropdownMenuItem>().ForEach(c =>
{
bool wasPreSelected = c.IsPreSelected;
c.IsPreSelected = compareItemEquality(item, c.Item);
if (c.IsPreSelected)
if (c.IsPreSelected && !wasPreSelected)
ContentContainer.ScrollIntoView(c);
});
}

protected sealed override DrawableMenuItem CreateDrawableMenuItem(MenuItem item) => CreateDrawableDropdownMenuItem(item);
protected sealed override DrawableMenuItem CreateDrawableMenuItem(MenuItem item)
{
var drawableItem = CreateDrawableDropdownMenuItem(item);
drawableItem.PreselectionRequested += PreselectItem;
return drawableItem;
}

protected abstract DrawableDropdownMenuItem CreateDrawableDropdownMenuItem(MenuItem item);

Expand Down
Loading

0 comments on commit 5a45622

Please sign in to comment.