From 4c34e0caeb17a9f98e25519a2b3f2197bbc90f8c Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 25 Nov 2023 01:04:38 +0300 Subject: [PATCH 1/9] Refactor current / selected item logic to make more sense --- .../Graphics/UserInterface/Dropdown.cs | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/osu.Framework/Graphics/UserInterface/Dropdown.cs b/osu.Framework/Graphics/UserInterface/Dropdown.cs index da63f6494d..54a72474b6 100644 --- a/osu.Framework/Graphics/UserInterface/Dropdown.cs +++ b/osu.Framework/Graphics/UserInterface/Dropdown.cs @@ -66,10 +66,7 @@ private void setItems(IEnumerable items) foreach (var entry in items) addDropdownItem(entry); - if (Current.Value == null || !itemMap.Keys.Contains(Current.Value, EqualityComparer.Default)) - Current.Value = itemMap.Keys.FirstOrDefault(); - else - Current.TriggerChange(); + ensureItemSelectionIsValid(); } private readonly IBindableList itemSource = new BindableList(); @@ -221,7 +218,7 @@ 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; @@ -286,20 +283,28 @@ protected override void LoadComplete() Header.Label = SelectedItem?.Text.Value ?? default; } - private void selectionChanged(ValueChangedEvent args) + private void ensureItemSelectionIsValid() { - // refresh if SelectedItem and SelectedValue mismatched - // null is not a valid value for Dictionary, so neither here - if (args.NewValue == null && SelectedItem != null) + if (Current.Value == null || !itemMap.ContainsKey(Current.Value)) { - selectedItem = new DropdownMenuItem(default(LocalisableString), default); + Current.Value = itemMap.Keys.FirstOrDefault(); + return; } - else if (SelectedItem == null || !EqualityComparer.Default.Equals(SelectedItem.Value, args.NewValue)) + + if (SelectedItem == null || !EqualityComparer.Default.Equals(Current.Value, selectedItem.Value)) + updateItemSelection(Current.Value); + } + + private void updateItemSelection(T value) + { + if (value != null && itemMap.TryGetValue(value, out var existingItem)) + selectedItem = existingItem; + else { - if (args.NewValue == null || !itemMap.TryGetValue(args.NewValue, out selectedItem)) - { - selectedItem = new DropdownMenuItem(GenerateItemText(args.NewValue), args.NewValue); - } + if (value == null && selectedItem != null) + selectedItem = new DropdownMenuItem(default(LocalisableString), default); + else + selectedItem = new DropdownMenuItem(GenerateItemText(value), value); } Menu.SelectItem(selectedItem); @@ -321,6 +326,7 @@ private void clearItems() { itemMap.Clear(); Menu.Clear(); + selectedItem = null; } /// From ab0c8993f00a7365e2ad9213403d9c6a4787e459 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 25 Nov 2023 01:28:41 +0300 Subject: [PATCH 2/9] Add support for reordering menu items --- osu.Framework/Graphics/UserInterface/Menu.cs | 38 +++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/osu.Framework/Graphics/UserInterface/Menu.cs b/osu.Framework/Graphics/UserInterface/Menu.cs index cc7823d39e..0b5f6e273d 100644 --- a/osu.Framework/Graphics/UserInterface/Menu.cs +++ b/osu.Framework/Graphics/UserInterface/Menu.cs @@ -277,7 +277,14 @@ private void resetState() /// Adds a to this . /// /// The to add. - public virtual void Add(MenuItem item) + public virtual void Add(MenuItem item) => Insert(itemsFlow.Count, item); + + /// + /// Inserts a at a specified position inside this . + /// + /// The position to insert this item at. + /// The to insert. + public void Insert(int position, MenuItem item) { var drawableItem = CreateDrawableMenuItem(item); drawableItem.Clicked = menuItemClicked; @@ -286,7 +293,12 @@ public virtual void Add(MenuItem item) drawableItem.SetFlowDirection(Direction); - ItemsContainer.Add(drawableItem); + var items = ItemsContainer.FlowingChildren.Cast().ToList(); + + for (int i = position; i < items.Count; i++) + ItemsContainer.SetLayoutPosition(items[i], i + 1); + + ItemsContainer.Insert(position, drawableItem); itemsFlow.SizeCache.Invalidate(); } @@ -306,10 +318,26 @@ private void itemStateChanged(DrawableMenuItem item, MenuItemState state) /// Whether was successfully removed. public bool Remove(MenuItem item) { - bool result = ItemsContainer.RemoveAll(d => d.Item == item, true) > 0; - itemsFlow.SizeCache.Invalidate(); + var items = ItemsContainer.FlowingChildren.Cast().ToList(); + bool removed = false; + + for (int i = 0; i < items.Count; i++) + { + var d = items[i]; + + if (d.Item == item) + { + for (int j = i + 1; j < items.Count; j++) + ItemsContainer.SetLayoutPosition(items[j], j - 1); - return result; + ItemsContainer.Remove(d, true); + items.RemoveAt(i--); + removed = true; + } + } + + itemsFlow.SizeCache.Invalidate(); + return removed; } /// From 571b75f14d6a7c1f0095acf006b272352ddf9da6 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 25 Nov 2023 01:30:56 +0300 Subject: [PATCH 3/9] Add proper handling for `BindableList` collection changes in dropdowns --- .../Visual/UserInterface/TestSceneDropdown.cs | 7 +- .../Graphics/UserInterface/Dropdown.cs | 103 +++++++++++++----- osu.Framework/Graphics/UserInterface/Menu.cs | 2 +- 3 files changed, 80 insertions(+), 32 deletions(-) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs index a22119190d..d388ced822 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs @@ -259,12 +259,7 @@ public void TestItemSource() AddStep("setup dropdown", () => testDropdown = setupDropdowns(1)[0]); - AddStep("bind source", () => - { - // todo: perhaps binding ItemSource should clear existing items. - testDropdown.ClearItems(); - testDropdown.ItemSource = bindableList = new BindableList(); - }); + AddStep("bind source", () => testDropdown.ItemSource = bindableList = new BindableList()); toggleDropdownViaClick(() => testDropdown); diff --git a/osu.Framework/Graphics/UserInterface/Dropdown.cs b/osu.Framework/Graphics/UserInterface/Dropdown.cs index 54a72474b6..6b7313fbc3 100644 --- a/osu.Framework/Graphics/UserInterface/Dropdown.cs +++ b/osu.Framework/Graphics/UserInterface/Dropdown.cs @@ -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; @@ -57,18 +59,6 @@ public IEnumerable Items } } - private void setItems(IEnumerable items) - { - clearItems(); - if (items == null) - return; - - foreach (var entry in items) - addDropdownItem(entry); - - ensureItemSelectionIsValid(); - } - private readonly IBindableList itemSource = new BindableList(); private IBindableList boundItemSource; @@ -86,9 +76,23 @@ public IBindableList ItemSource if (boundItemSource != null) itemSource.UnbindFrom(boundItemSource); itemSource.BindTo(boundItemSource = value); + + setItems(value); } } + private void setItems(IEnumerable value) + { + clearItems(); + if (value == null) + return; + + foreach (var entry in value) + addDropdownItem(entry); + + ensureItemSelectionIsValid(); + } + /// /// Add a menu item directly while automatically generating a label. /// @@ -101,7 +105,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)}."); @@ -118,7 +122,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; } @@ -144,7 +152,6 @@ private bool removeDropdownItem(T value) Menu.Remove(item); itemMap.Remove(value); - return true; } @@ -226,7 +233,7 @@ protected Dropdown() Menu.State = MenuState.Closed; }; - ItemSource.CollectionChanged += (_, _) => setItems(itemSource); + ItemSource.CollectionChanged += collectionChanged; } private void preselectionConfirmed(int selectedIndex) @@ -283,6 +290,55 @@ protected override void LoadComplete() Header.Label = SelectedItem?.Text.Value ?? default; } + private void collectionChanged(object sender, NotifyCollectionChangedEventArgs e) + { + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + { + var newItems = e.NewItems.AsNonNull().Cast().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()) + 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()) + removeDropdownItem(item); + + var newItems = e.NewItems.AsNonNull().Cast().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)) @@ -372,14 +428,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) @@ -441,7 +489,12 @@ protected void PreselectItem(MenuItem item) }); } - 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); diff --git a/osu.Framework/Graphics/UserInterface/Menu.cs b/osu.Framework/Graphics/UserInterface/Menu.cs index 0b5f6e273d..4c83ab9b86 100644 --- a/osu.Framework/Graphics/UserInterface/Menu.cs +++ b/osu.Framework/Graphics/UserInterface/Menu.cs @@ -277,7 +277,7 @@ private void resetState() /// Adds a to this . /// /// The to add. - public virtual void Add(MenuItem item) => Insert(itemsFlow.Count, item); + public void Add(MenuItem item) => Insert(itemsFlow.Count, item); /// /// Inserts a at a specified position inside this . From 7a6d7e2f459c251e27b16c2aacf392f3bcb620e1 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 25 Nov 2023 00:55:20 +0300 Subject: [PATCH 4/9] Add test coverage --- .../Visual/UserInterface/TestSceneDropdown.cs | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs index d388ced822..5af215d4f5 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs @@ -265,14 +265,56 @@ public void TestItemSource() 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>().Single().FlowingChildren.Cast().ElementAt(index).Item.Text.Value == item); + } + } + + [Test] + public void TestItemReplacementDoesNotAffectScroll() + { + TestDropdown testDropdown = null!; + BindableList bindableList = null!; + + AddStep("setup dropdown", () => testDropdown = setupDropdowns(1)[0]); + + AddStep("bind source", () => testDropdown.ItemSource = bindableList = new BindableList()); + 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().Single().ScrollTo(200)); + AddStep("replace item in middle", () => bindableList.ReplaceRange(10, 1, new TestModel[] { "test ten" })); + AddAssert("scroll is unchanged", () => testDropdown.ChildrenOfType().Single().Target == 200); } /// From f4f4e31748e0ce37c782057a96e0303ffcff0883 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 25 Nov 2023 02:30:09 +0300 Subject: [PATCH 5/9] Add failing test case --- .../Visual/UserInterface/TestSceneDropdown.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs index 5af215d4f5..8e4baf128d 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs @@ -317,6 +317,23 @@ public void TestItemReplacementDoesNotAffectScroll() AddAssert("scroll is unchanged", () => testDropdown.ChildrenOfType().Single().Target == 200); } + [Test] + public void TestClearItemsInBindableWhileNotPresent() + { + TestDropdown testDropdown = null!; + BindableList bindableList = null!; + + AddStep("setup dropdown", () => testDropdown = setupDropdowns(1)[0]); + + AddStep("bind source", () => testDropdown.ItemSource = bindableList = new BindableList()); + 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()); + } + /// /// Adds an item before a dropdown is loaded, and ensures item labels are assigned correctly. /// From 6e483d72800a091313cce323b51b86c61dbf7dbc Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Sat, 25 Nov 2023 02:25:03 +0300 Subject: [PATCH 6/9] Handle non-alive menu items during addition/removal --- osu.Framework/Graphics/UserInterface/Menu.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Framework/Graphics/UserInterface/Menu.cs b/osu.Framework/Graphics/UserInterface/Menu.cs index 4c83ab9b86..bf93c76c1d 100644 --- a/osu.Framework/Graphics/UserInterface/Menu.cs +++ b/osu.Framework/Graphics/UserInterface/Menu.cs @@ -293,7 +293,7 @@ public void Insert(int position, MenuItem item) drawableItem.SetFlowDirection(Direction); - var items = ItemsContainer.FlowingChildren.Cast().ToList(); + var items = Children.OrderBy(ItemsContainer.GetLayoutPosition).ToList(); for (int i = position; i < items.Count; i++) ItemsContainer.SetLayoutPosition(items[i], i + 1); @@ -318,7 +318,7 @@ private void itemStateChanged(DrawableMenuItem item, MenuItemState state) /// Whether was successfully removed. public bool Remove(MenuItem item) { - var items = ItemsContainer.FlowingChildren.Cast().ToList(); + var items = Children.OrderBy(ItemsContainer.GetLayoutPosition).ToList(); bool removed = false; for (int i = 0; i < items.Count; i++) From 413dc6ff2f016a964b86f56d98fb3168aff33103 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 28 Nov 2023 06:07:10 +0300 Subject: [PATCH 7/9] Change type exposure of `Menu.ItemsContainer` to avoid potential manipulation in layout position --- osu.Framework/Graphics/UserInterface/Menu.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/osu.Framework/Graphics/UserInterface/Menu.cs b/osu.Framework/Graphics/UserInterface/Menu.cs index bf93c76c1d..e35f98cf64 100644 --- a/osu.Framework/Graphics/UserInterface/Menu.cs +++ b/osu.Framework/Graphics/UserInterface/Menu.cs @@ -47,7 +47,9 @@ public abstract partial class Menu : CompositeDrawable, IStateful /// /// The that contains the items of this . /// - protected FillFlowContainer ItemsContainer => itemsFlow; + // this is intentionally not a FillFlowContainer, as to not allow the consumers to mutate the layout position of menu items, + // since we manage it ourselves to define a specific order for menu items and allow inserting ones between others. + protected Container ItemsContainer => itemsFlow; /// /// The container that provides the masking effects for this . @@ -293,12 +295,12 @@ public void Insert(int position, MenuItem item) drawableItem.SetFlowDirection(Direction); - var items = Children.OrderBy(ItemsContainer.GetLayoutPosition).ToList(); + var items = Children.OrderBy(itemsFlow.GetLayoutPosition).ToList(); for (int i = position; i < items.Count; i++) - ItemsContainer.SetLayoutPosition(items[i], i + 1); + itemsFlow.SetLayoutPosition(items[i], i + 1); - ItemsContainer.Insert(position, drawableItem); + itemsFlow.Insert(position, drawableItem); itemsFlow.SizeCache.Invalidate(); } @@ -318,7 +320,7 @@ private void itemStateChanged(DrawableMenuItem item, MenuItemState state) /// Whether was successfully removed. public bool Remove(MenuItem item) { - var items = Children.OrderBy(ItemsContainer.GetLayoutPosition).ToList(); + var items = Children.OrderBy(itemsFlow.GetLayoutPosition).ToList(); bool removed = false; for (int i = 0; i < items.Count; i++) @@ -328,9 +330,9 @@ public bool Remove(MenuItem item) if (d.Item == item) { for (int j = i + 1; j < items.Count; j++) - ItemsContainer.SetLayoutPosition(items[j], j - 1); + itemsFlow.SetLayoutPosition(items[j], j - 1); - ItemsContainer.Remove(d, true); + itemsFlow.Remove(d, true); items.RemoveAt(i--); removed = true; } From 9d3275e50948eb4d32f62daaca392ddb76f0e87a Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Wed, 29 Nov 2023 15:08:59 +0900 Subject: [PATCH 8/9] Fix test scene code quality issues --- .../Visual/UserInterface/TestSceneDropdown.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs b/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs index 6d207b2b33..d36281e1cd 100644 --- a/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs +++ b/osu.Framework.Tests/Visual/UserInterface/TestSceneDropdown.cs @@ -292,10 +292,9 @@ public void TestItemSource() checkOrder(1, "one-half"); checkOrder(2, "two"); - void checkOrder(int index, string item) - { - AddAssert($"item #{index + 1} is '{item}'", () => testDropdown.ChildrenOfType>().Single().FlowingChildren.Cast().ElementAt(index).Item.Text.Value == item); - } + void checkOrder(int index, string item) => AddAssert($"item #{index + 1} is '{item}'", + () => testDropdown.ChildrenOfType>().Single().FlowingChildren.Cast().ElementAt(index).Item.Text.Value.ToString(), + () => Is.EqualTo(item)); } [Test] @@ -304,7 +303,7 @@ public void TestItemReplacementDoesNotAffectScroll() TestDropdown testDropdown = null!; BindableList bindableList = null!; - AddStep("setup dropdown", () => testDropdown = setupDropdowns(1)[0]); + AddStep("setup dropdown", () => testDropdown = createDropdown()); AddStep("bind source", () => testDropdown.ItemSource = bindableList = new BindableList()); AddStep("add many items", () => bindableList.AddRange(Enumerable.Range(0, 20).Select(i => (TestModel)$"test {i}"))); @@ -323,7 +322,7 @@ public void TestClearItemsInBindableWhileNotPresent() TestDropdown testDropdown = null!; BindableList bindableList = null!; - AddStep("setup dropdown", () => testDropdown = setupDropdowns(1)[0]); + AddStep("setup dropdown", () => testDropdown = createDropdown()); AddStep("bind source", () => testDropdown.ItemSource = bindableList = new BindableList()); AddStep("add many items", () => bindableList.AddRange(Enumerable.Range(0, 20).Select(i => (TestModel)$"test {i}"))); From 4d359a9dc1f1d0d2d7af44788be7992a05dc0847 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 5 Dec 2023 22:19:04 +0300 Subject: [PATCH 9/9] Remove necessity of clearing item selection on `Clear` to maintain behaviour --- osu.Framework/Graphics/UserInterface/Dropdown.cs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/osu.Framework/Graphics/UserInterface/Dropdown.cs b/osu.Framework/Graphics/UserInterface/Dropdown.cs index 6b7313fbc3..515e3fbb46 100644 --- a/osu.Framework/Graphics/UserInterface/Dropdown.cs +++ b/osu.Framework/Graphics/UserInterface/Dropdown.cs @@ -84,6 +84,7 @@ public IBindableList ItemSource private void setItems(IEnumerable value) { clearItems(); + if (value == null) return; @@ -347,8 +348,7 @@ private void ensureItemSelectionIsValid() return; } - if (SelectedItem == null || !EqualityComparer.Default.Equals(Current.Value, selectedItem.Value)) - updateItemSelection(Current.Value); + updateItemSelection(Current.Value); } private void updateItemSelection(T value) @@ -382,7 +382,6 @@ private void clearItems() { itemMap.Clear(); Menu.Clear(); - selectedItem = null; } /// @@ -450,8 +449,9 @@ public void SelectItem(DropdownMenuItem item) { Children.OfType().ForEach(c => { + bool wasSelected = c.IsSelected; c.IsSelected = compareItemEquality(item, c.Item); - if (c.IsSelected) + if (c.IsSelected && !wasSelected) ContentContainer.ScrollIntoView(c); }); } @@ -483,8 +483,9 @@ protected void PreselectItem(MenuItem item) { Children.OfType().ForEach(c => { + bool wasPreSelected = c.IsPreSelected; c.IsPreSelected = compareItemEquality(item, c.Item); - if (c.IsPreSelected) + if (c.IsPreSelected && !wasPreSelected) ContentContainer.ScrollIntoView(c); }); }