Merge pull request #16094 from peppy/beatmap-card-expanded-state-fix

Fix beatmap card potentially collapsing when it shouldn't

osu.Game.Tests/Visual/Beatmaps/TestSceneBeatmapCard.cs

@@ -6,12 +6,12 @@ using System.Collections.Generic;
 using System.Linq;
 using NUnit.Framework;
 using osu.Framework.Allocation;
-using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
+using osu.Game.Beatmaps.Drawables;
 using osu.Game.Beatmaps.Drawables.Cards;
 using osu.Game.Graphics.Containers;
 using osu.Game.Online.API;
@@ -19,11 +19,10 @@ using osu.Game.Online.API.Requests;
 using osu.Game.Online.API.Requests.Responses;
 using osu.Game.Overlays;
 using osuTK;
-using APIUser = osu.Game.Online.API.Requests.Responses.APIUser;
 
 namespace osu.Game.Tests.Visual.Beatmaps
 {
-    public class TestSceneBeatmapCard : OsuTestScene
+    public class TestSceneBeatmapCard : OsuManualInputManagerTestScene
     {
         /// <summary>
         /// All cards on this scene use a common online ID to ensure that map download, preview tracks, etc. can be tested manually with online sources.
@@ -253,14 +252,32 @@ namespace osu.Game.Tests.Visual.Beatmaps
         public void TestNormal()
         {
             createTestCase(beatmapSetInfo => new BeatmapCard(beatmapSetInfo));
+        }
 
-            AddToggleStep("toggle expanded state", expanded =>
+        [Test]
-            {
+        public void TestHoverState()
-                var card = this.ChildrenOfType<BeatmapCard>().Last();
+        {
-                if (!card.Expanded.Disabled)
+            AddStep("create cards", () => Child = createContent(OverlayColourScheme.Blue, s => new BeatmapCard(s)));
-                    card.Expanded.Value = expanded;
+
-            });
+            AddStep("Hover card", () => InputManager.MoveMouseTo(firstCard()));
-            AddToggleStep("disable/enable expansion", disabled => this.ChildrenOfType<BeatmapCard>().ForEach(card => card.Expanded.Disabled = disabled));
+            AddWaitStep("wait for potential state change", 5);
+            AddAssert("card is not expanded", () => !firstCard().Expanded.Value);
+
+            AddStep("Hover spectrum display", () => InputManager.MoveMouseTo(firstCard().ChildrenOfType<DifficultySpectrumDisplay>().Single()));
+            AddUntilStep("card is expanded", () => firstCard().Expanded.Value);
+
+            AddStep("Hover difficulty content", () => InputManager.MoveMouseTo(firstCard().ChildrenOfType<BeatmapCardDifficultyList>().Single()));
+            AddWaitStep("wait for potential state change", 5);
+            AddAssert("card is still expanded", () => firstCard().Expanded.Value);
+
+            AddStep("Hover main content again", () => InputManager.MoveMouseTo(firstCard()));
+            AddWaitStep("wait for potential state change", 5);
+            AddAssert("card is still expanded", () => firstCard().Expanded.Value);
+
+            AddStep("Hover away", () => InputManager.MoveMouseTo(this.ChildrenOfType<BeatmapCard>().Last()));
+            AddUntilStep("card is not expanded", () => !firstCard().Expanded.Value);
+
+            BeatmapCard firstCard() => this.ChildrenOfType<BeatmapCard>().First();
         }
     }
 }

osu.Game/Beatmaps/Drawables/Cards/BeatmapCard.cs

@@ -33,7 +33,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards
         public const float TRANSITION_DURATION = 400;
         public const float CORNER_RADIUS = 10;
 
-        public Bindable<bool> Expanded { get; } = new BindableBool();
+        public IBindable<bool> Expanded { get; }
 
         private const float width = 408;
         private const float height = 100;
@@ -64,9 +64,11 @@ namespace osu.Game.Beatmaps.Drawables.Cards
         [Resolved]
         private OverlayColourProvider colourProvider { get; set; } = null!;
 
-        public BeatmapCard(APIBeatmapSet beatmapSet)
+        public BeatmapCard(APIBeatmapSet beatmapSet, bool allowExpansion = true)
             : base(HoverSampleSet.Submit)
         {
+            Expanded = new BindableBool { Disabled = !allowExpansion };
+
             this.beatmapSet = beatmapSet;
             favouriteState = new Bindable<BeatmapSetFavouriteState>(new BeatmapSetFavouriteState(beatmapSet.HasFavourited, beatmapSet.FavouriteCount));
             downloadTracker = new BeatmapDownloadTracker(beatmapSet);
@@ -282,15 +284,15 @@ namespace osu.Game.Beatmaps.Drawables.Cards
                                                 {
                                                     Hovered = _ =>
                                                     {
-                                                        content.ScheduleShow();
+                                                        content.ExpandAfterDelay();
                                                         return false;
                                                     },
                                                     Unhovered = _ =>
                                                     {
-                                                        // This hide should only trigger if the expanded content has not shown yet.
+                                                        // Handles the case where a user has not shown explicit intent to view expanded info.
-                                                        // ie. if the user has not shown intent to want to see it (quickly moved over the info row area).
+                                                        // ie. quickly moved over the info row area but didn't remain within it.
                                                         if (!Expanded.Value)
-                                                            content.ScheduleHide();
+                                                            content.CancelExpand();
                                                     }
                                                 }
                                             }
@@ -366,8 +368,6 @@ namespace osu.Game.Beatmaps.Drawables.Cards
 
         protected override void OnHoverLost(HoverLostEvent e)
         {
-            content.ScheduleHide();
-
             updateState();
             base.OnHoverLost(e);
         }

osu.Game/Beatmaps/Drawables/Cards/BeatmapCardContent.cs

@@ -31,7 +31,9 @@ namespace osu.Game.Beatmaps.Drawables.Cards
             set => dropdownScroll.Child = value;
         }
 
-        public Bindable<bool> Expanded { get; } = new BindableBool();
+        public IBindable<bool> Expanded => expanded;
+
+        private readonly BindableBool expanded = new BindableBool();
 
         private readonly Box background;
         private readonly Container content;
@@ -54,7 +56,7 @@ namespace osu.Game.Beatmaps.Drawables.Cards
                 AutoSizeAxes = Axes.Y,
                 CornerRadius = BeatmapCard.CORNER_RADIUS,
                 Masking = true,
-                Unhovered = _ => checkForHide(),
+                Unhovered = _ => updateFromHoverChange(),
                 Children = new Drawable[]
                 {
                     background = new Box
@@ -76,10 +78,10 @@ namespace osu.Game.Beatmaps.Drawables.Cards
                         Alpha = 0,
                         Hovered = _ =>
                         {
-                            keep();
+                            updateFromHoverChange();
                             return true;
                         },
-                        Unhovered = _ => checkForHide(),
+                        Unhovered = _ => updateFromHoverChange(),
                         Child = dropdownScroll = new ExpandedContentScrollContainer
                         {
                             RelativeSizeAxes = Axes.X,
@@ -119,51 +121,20 @@ namespace osu.Game.Beatmaps.Drawables.Cards
 
         private ScheduledDelegate? scheduledExpandedChange;
 
-        public void ScheduleShow()
+        public void ExpandAfterDelay() => queueExpandedStateChange(true, 100);
-        {
-            scheduledExpandedChange?.Cancel();
-            if (Expanded.Disabled || Expanded.Value)
-                return;
 
-            scheduledExpandedChange = Scheduler.AddDelayed(() =>
+        public void CancelExpand() => scheduledExpandedChange?.Cancel();
-            {
-                if (!Expanded.Disabled)
-                    Expanded.Value = true;
-            }, 100);
-        }
 
-        public void ScheduleHide()
+        private void updateFromHoverChange() =>
-        {
+            queueExpandedStateChange(content.IsHovered || dropdownContent.IsHovered, 100);
-            scheduledExpandedChange?.Cancel();
-            if (Expanded.Disabled || !Expanded.Value)
-                return;
 
-            scheduledExpandedChange = Scheduler.AddDelayed(() =>
+        private void queueExpandedStateChange(bool newState, int delay = 0)
-            {
-                if (!Expanded.Disabled)
-                    Expanded.Value = false;
-            }, 500);
-        }
-
-        private void checkForHide()
-        {
-            if (Expanded.Disabled)
-                return;
-
-            if (content.IsHovered || dropdownContent.IsHovered)
-                return;
-
-            scheduledExpandedChange?.Cancel();
-            Expanded.Value = false;
-        }
-
-        private void keep()
         {
             if (Expanded.Disabled)
                 return;
 
             scheduledExpandedChange?.Cancel();
-            Expanded.Value = true;
+            scheduledExpandedChange = Scheduler.AddDelayed(() => expanded.Value = newState, delay);
         }
 
         private void updateState()

osu.Game/Screens/Play/SoloSpectator.cs

@@ -228,10 +228,7 @@ namespace osu.Game.Screens.Play
             onlineBeatmapRequest.Success += beatmapSet => Schedule(() =>
             {
                 this.beatmapSet = beatmapSet;
-                beatmapPanelContainer.Child = new BeatmapCard(this.beatmapSet)
+                beatmapPanelContainer.Child = new BeatmapCard(this.beatmapSet, allowExpansion: false);
-                {
-                    Expanded = { Disabled = true }
-                };
                 checkForAutomaticDownload();
             });