diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs index 00a0d4a849..94c6130f15 100644 --- a/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs +++ b/osu.Game.Tests/Visual/SongSelect/TestSceneFilterControl.cs @@ -192,7 +192,7 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("select collection", () => { - InputManager.MoveMouseTo(getCollectionDropdownItems().ElementAt(1)); + InputManager.MoveMouseTo(getCollectionDropdownItemAt(1)); InputManager.Click(MouseButton.Left); }); @@ -206,7 +206,8 @@ namespace osu.Game.Tests.Visual.SongSelect AddStep("click manage collections filter", () => { - InputManager.MoveMouseTo(getCollectionDropdownItems().Last()); + int lastItemIndex = control.ChildrenOfType().Single().Items.Count() - 1; + InputManager.MoveMouseTo(getCollectionDropdownItemAt(lastItemIndex)); InputManager.Click(MouseButton.Left); }); @@ -232,10 +233,10 @@ namespace osu.Game.Tests.Visual.SongSelect private void assertCollectionDropdownContains(string collectionName, bool shouldContain = true) => AddUntilStep($"collection dropdown {(shouldContain ? "contains" : "does not contain")} '{collectionName}'", // A bit of a roundabout way of going about this, see: https://github.com/ppy/osu-framework/issues/3871 + https://github.com/ppy/osu-framework/issues/3872 - () => shouldContain == (getCollectionDropdownItems().Any(i => i.ChildrenOfType().OfType().First().Text == collectionName))); + () => shouldContain == control.ChildrenOfType().Any(i => i.ChildrenOfType().OfType().First().Text == collectionName)); private IconButton getAddOrRemoveButton(int index) - => getCollectionDropdownItems().ElementAt(index).ChildrenOfType().Single(); + => getCollectionDropdownItemAt(index).ChildrenOfType().Single(); private void addExpandHeaderStep() => AddStep("expand header", () => { @@ -249,7 +250,11 @@ namespace osu.Game.Tests.Visual.SongSelect InputManager.Click(MouseButton.Left); }); - private IEnumerable.DropdownMenu.DrawableDropdownMenuItem> getCollectionDropdownItems() - => control.ChildrenOfType().Single().ChildrenOfType.DropdownMenu.DrawableDropdownMenuItem>(); + private Menu.DrawableMenuItem getCollectionDropdownItemAt(int index) + { + // todo: we should be able to use Items, but apparently that's not guaranteed to be ordered... see: https://github.com/ppy/osu-framework/pull/6079 + CollectionFilterMenuItem item = control.ChildrenOfType().Single().ItemSource.ElementAt(index); + return control.ChildrenOfType().Single(i => i.Item.Text.Value == item.CollectionName); + } } } diff --git a/osu.Game/Collections/CollectionDropdown.cs b/osu.Game/Collections/CollectionDropdown.cs index e435992381..299594b0a0 100644 --- a/osu.Game/Collections/CollectionDropdown.cs +++ b/osu.Game/Collections/CollectionDropdown.cs @@ -43,11 +43,13 @@ namespace osu.Game.Collections private IDisposable? realmSubscription; + private readonly CollectionFilterMenuItem allBeatmapsItem = new AllBeatmapsCollectionFilterMenuItem(); + public CollectionDropdown() { ItemSource = filters; - Current.Value = new AllBeatmapsCollectionFilterMenuItem(); + Current.Value = allBeatmapsItem; } protected override void LoadComplete() @@ -61,37 +63,51 @@ namespace osu.Game.Collections private void collectionsChanged(IRealmCollection collections, ChangeSet? changes) { - var selectedItem = SelectedItem?.Value?.Collection; - - var allBeatmaps = new AllBeatmapsCollectionFilterMenuItem(); - - filters.Clear(); - filters.Add(allBeatmaps); - filters.AddRange(collections.Select(c => new CollectionFilterMenuItem(c.ToLive(realm)))); - - if (ShowManageCollectionsItem) - filters.Add(new ManageCollectionsFilterMenuItem()); - - // This current update and schedule is required to work around dropdown headers not updating text even when the selected item - // changes. It's not great but honestly the whole dropdown menu structure isn't great. This needs to be fixed, but I'll issue - // a warning that it's going to be a frustrating journey. - Current.Value = allBeatmaps; - Schedule(() => + if (changes == null) { - // current may have changed before the scheduled call is run. - if (Current.Value != allBeatmaps) - return; - - Current.Value = filters.SingleOrDefault(f => f.Collection != null && f.Collection.ID == selectedItem?.ID) ?? filters[0]; - }); - - // Trigger a re-filter if the current item was in the change set. - if (selectedItem != null && changes != null) + filters.Add(allBeatmapsItem); + filters.AddRange(collections.Select(c => new CollectionFilterMenuItem(c.ToLive(realm)))); + if (ShowManageCollectionsItem) + filters.Add(new ManageCollectionsFilterMenuItem()); + } + else { - foreach (int index in changes.ModifiedIndices) + foreach (int i in changes.DeletedIndices) + filters.RemoveAt(i + 1); + + foreach (int i in changes.InsertedIndices) + filters.Insert(i + 1, new CollectionFilterMenuItem(collections[i].ToLive(realm))); + + var selectedItem = SelectedItem?.Value; + + foreach (int i in changes.NewModifiedIndices) { - if (collections[index].ID == selectedItem.ID) + var updatedItem = collections[i]; + + // This is responsible for updating the state of the +/- button and the collection's name. + // TODO: we can probably make the menu items update with changes to avoid this. + filters.RemoveAt(i + 1); + filters.Insert(i + 1, new CollectionFilterMenuItem(updatedItem.ToLive(realm))); + + if (updatedItem.ID == selectedItem?.Collection?.ID) + { + // This current update and schedule is required to work around dropdown headers not updating text even when the selected item + // changes. It's not great but honestly the whole dropdown menu structure isn't great. This needs to be fixed, but I'll issue + // a warning that it's going to be a frustrating journey. + Current.Value = allBeatmapsItem; + Schedule(() => + { + // current may have changed before the scheduled call is run. + if (Current.Value != allBeatmapsItem) + return; + + Current.Value = filters.SingleOrDefault(f => f.Collection?.ID == selectedItem.Collection?.ID) ?? filters[0]; + }); + + // Trigger an external re-filter if the current item was in the change set. RequestFilter?.Invoke(); + break; + } } } }