Merge pull request #33301 from peppy/remember-group-expansion

SongSelectV2: Fix expanded group not being persisted over filter operations

osu.Game.Tests/Visual/SongSelectV2/BeatmapCarouselTestScene.cs

@@ -367,6 +367,9 @@ namespace osu.Game.Tests.Visual.SongSelectV2
         {
             public IEnumerable<BeatmapInfo> PostFilterBeatmaps = null!;
 
+            public new BeatmapSetInfo? ExpandedBeatmapSet => base.ExpandedBeatmapSet;
+            public new GroupDefinition? ExpandedGroup => base.ExpandedGroup;
+
             protected override Task<IEnumerable<CarouselItem>> FilterAsync(bool clearExistingPanels = false)
             {
                 var filterAsync = base.FilterAsync(clearExistingPanels);

osu.Game.Tests/Visual/SongSelectV2/TestSceneBeatmapCarouselDifficultyGrouping.cs

@@ -224,5 +224,25 @@ namespace osu.Game.Tests.Visual.SongSelectV2
             CheckDisplayedGroupsCount(3);
             CheckDisplayedBeatmapsCount(30);
         }
+
+        [Test]
+        public void TestExpandedGroupStillExpandedAfterFilter()
+        {
+            SelectPrevGroup();
+
+            WaitForGroupSelection(2, 9);
+            AddAssert("expanded group is last", () => (Carousel.ExpandedGroup as StarDifficultyGroupDefinition)?.Difficulty.Stars, () => Is.EqualTo(6));
+
+            SelectNextPanel();
+            Select();
+
+            WaitForGroupSelection(2, 9);
+            AddAssert("expanded group is first", () => (Carousel.ExpandedGroup as StarDifficultyGroupDefinition)?.Difficulty.Stars, () => Is.EqualTo(0));
+
+            // doesn't actually filter anything away, but triggers a filter.
+            ApplyToFilter("filter", c => c.SearchText = "Some");
+
+            AddAssert("expanded group is still first", () => (Carousel.ExpandedGroup as StarDifficultyGroupDefinition)?.Difficulty.Stars, () => Is.EqualTo(0));
+        }
     }
 }

osu.Game/Graphics/Carousel/Carousel.cs

@@ -195,6 +195,13 @@ namespace osu.Game.Graphics.Carousel
             return filterTask;
         }
 
+        /// <summary>
+        /// Fired after a filter operation completed.
+        /// </summary>
+        protected virtual void HandleFilterCompleted()
+        {
+        }
+
         /// <summary>
         /// Check whether two models are the same for display purposes.
         /// </summary>
@@ -343,8 +350,7 @@ namespace osu.Game.Graphics.Carousel
                     filterReusesPanels.Validate();
                 }
 
-                // Need to call this to ensure correct post-selection logic is handled on the new items list.
-                HandleItemSelected(currentSelection.Model);
+                HandleFilterCompleted();
 
                 refreshAfterSelection();
                 if (!Scroll.UserScrolling)

osu.Game/Screens/SelectV2/BeatmapCarousel.cs

@@ -174,8 +174,9 @@ namespace osu.Game.Screens.SelectV2
 
         #region Selection handling
 
-        private GroupDefinition? lastSelectedGroup;
-        private BeatmapInfo? lastSelectedBeatmap;
+        protected GroupDefinition? ExpandedGroup { get; private set; }
+
+        protected BeatmapSetInfo? ExpandedBeatmapSet { get; private set; }
 
         protected override void HandleItemActivated(CarouselItem item)
         {
@@ -185,10 +186,10 @@ namespace osu.Game.Screens.SelectV2
                 {
                     case GroupDefinition group:
                         // Special case – collapsing an open group.
-                        if (lastSelectedGroup == group)
+                        if (ExpandedGroup == group)
                         {
-                            setExpansionStateOfGroup(lastSelectedGroup, false);
-                            lastSelectedGroup = null;
+                            setExpansionStateOfGroup(ExpandedGroup, false);
+                            ExpandedGroup = null;
                             return;
                         }
 
@@ -243,6 +244,22 @@ namespace osu.Game.Screens.SelectV2
             }
         }
 
+        protected override void HandleFilterCompleted()
+        {
+            base.HandleFilterCompleted();
+
+            // Store selected group before handling selection (it may implicitly change the expanded group).
+            var groupForReselection = ExpandedGroup;
+
+            // Ensure correct post-selection logic is handled on the new items list.
+            // This will update the visual state of the selected item.
+            HandleItemSelected(CurrentSelection);
+
+            // If a group was selected that is not the one containing the selection, reselect it.
+            if (groupForReselection != null)
+                setExpandedGroup(groupForReselection);
+        }
+
         protected override bool CheckValidForGroupSelection(CarouselItem item)
         {
             switch (item.Model)
@@ -263,9 +280,9 @@ namespace osu.Game.Screens.SelectV2
 
         private void setExpandedGroup(GroupDefinition group)
         {
-            if (lastSelectedGroup != null)
-                setExpansionStateOfGroup(lastSelectedGroup, false);
-            lastSelectedGroup = group;
+            if (ExpandedGroup != null)
+                setExpansionStateOfGroup(ExpandedGroup, false);
+            ExpandedGroup = group;
             setExpansionStateOfGroup(group, true);
         }
 
@@ -319,10 +336,10 @@ namespace osu.Game.Screens.SelectV2
 
         private void setExpandedSet(BeatmapInfo beatmapInfo)
         {
-            if (lastSelectedBeatmap != null)
-                setExpansionStateOfSetItems(lastSelectedBeatmap.BeatmapSet!, false);
-            lastSelectedBeatmap = beatmapInfo;
-            setExpansionStateOfSetItems(beatmapInfo.BeatmapSet!, true);
+            if (ExpandedBeatmapSet != null)
+                setExpansionStateOfSetItems(ExpandedBeatmapSet, false);
+            ExpandedBeatmapSet = beatmapInfo.BeatmapSet!;
+            setExpansionStateOfSetItems(ExpandedBeatmapSet, true);
         }
 
         private void setExpansionStateOfSetItems(BeatmapSetInfo set, bool expanded)