Merge pull request #18120 from peppy/fix-toolbox-expansion

Fix user-contracted toolbox groups in editor incorrectly expanding on hover

osu.Game.Tests/Visual/UserInterface/TestSceneExpandingContainer.cs

@@ -99,15 +99,15 @@ namespace osu.Game.Tests.Visual.UserInterface
         }
 
         /// <summary>
-        /// Tests expanding a container will expand underlying groups if contracted.
+        /// Tests expanding a container will not expand underlying groups if they were manually contracted by the user.
         /// </summary>
         [Test]
-        public void TestExpandingContainerExpandsContractedGroup()
+        public void TestExpandingContainerDoesNotExpandContractedGroup()
         {
             AddStep("contract group", () => toolboxGroup.Expanded.Value = false);
 
             AddStep("expand container", () => container.Expanded.Value = true);
-            AddAssert("group expanded", () => toolboxGroup.Expanded.Value);
+            AddAssert("group not expanded", () => !toolboxGroup.Expanded.Value);
             AddAssert("controls expanded", () => slider1.Expanded.Value && slider2.Expanded.Value);
 
             AddStep("contract container", () => container.Expanded.Value = false);

osu.Game/Overlays/SettingsToolboxGroup.cs

@@ -1,7 +1,6 @@
 // Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
 // See the LICENCE file in the repository root for full licence text.
 
-using osu.Framework.Allocation;
 using osu.Framework.Bindables;
 using osu.Framework.Caching;
 using osu.Framework.Extensions.EnumExtensions;
@@ -35,14 +34,13 @@ namespace osu.Game.Overlays
         private readonly Cached headerTextVisibilityCache = new Cached();
 
         private readonly FillFlowContainer content;
-        private readonly IconButton button;
 
         public BindableBool Expanded { get; } = new BindableBool(true);
 
-        private Color4 expandedColour;
-
         private readonly OsuSpriteText headerText;
 
+        private readonly Container headerContent;
+
         /// <summary>
         /// Create a new instance.
         /// </summary>
@@ -71,7 +69,7 @@ namespace osu.Game.Overlays
                     AutoSizeAxes = Axes.Y,
                     Children = new Drawable[]
                     {
-                        new Container
+                        headerContent = new Container
                         {
                             Name = @"Header",
                             Origin = Anchor.TopCentre,
@@ -88,7 +86,7 @@ namespace osu.Game.Overlays
                                     Font = OsuFont.GetFont(weight: FontWeight.Bold, size: 17),
                                     Padding = new MarginPadding { Left = 10, Right = 30 },
                                 },
-                                button = new IconButton
+                                new IconButton
                                 {
                                     Origin = Anchor.Centre,
                                     Anchor = Anchor.CentreRight,
@@ -117,12 +115,25 @@ namespace osu.Game.Overlays
             };
         }
 
-        protected override bool OnInvalidate(Invalidation invalidation, InvalidationSource source)
+        protected override void LoadComplete()
         {
-            if (invalidation.HasFlagFast(Invalidation.DrawSize))
+            base.LoadComplete();
-                headerTextVisibilityCache.Invalidate();
 
-            return base.OnInvalidate(invalidation, source);
+            Expanded.BindValueChanged(updateExpandedState, true);
+
+            this.Delay(600).Schedule(updateFadeState);
+        }
+
+        protected override bool OnHover(HoverEvent e)
+        {
+            updateFadeState();
+            return false;
+        }
+
+        protected override void OnHoverLost(HoverLostEvent e)
+        {
+            updateFadeState();
+            base.OnHoverLost(e);
         }
 
         protected override void Update()
@@ -135,72 +146,30 @@ namespace osu.Game.Overlays
                 headerText.FadeTo(headerText.DrawWidth < DrawWidth ? 1 : 0, 150, Easing.OutQuint);
         }
 
-        [Resolved(canBeNull: true)]
+        protected override bool OnInvalidate(Invalidation invalidation, InvalidationSource source)
-        private IExpandingContainer expandingContainer { get; set; }
-
-        private bool expandedByContainer;
-
-        protected override void LoadComplete()
         {
-            base.LoadComplete();
+            if (invalidation.HasFlagFast(Invalidation.DrawSize))
+                headerTextVisibilityCache.Invalidate();
 
-            expandingContainer?.Expanded.BindValueChanged(containerExpanded =>
+            return base.OnInvalidate(invalidation, source);
+        }
+
+        private void updateExpandedState(ValueChangedEvent<bool> expanded)
+        {
+            if (expanded.NewValue)
+                content.AutoSizeAxes = Axes.Y;
+            else
             {
-                if (containerExpanded.NewValue && !Expanded.Value)
+                content.AutoSizeAxes = Axes.None;
-                {
+                content.ResizeHeightTo(0, transition_duration, Easing.OutQuint);
-                    Expanded.Value = true;
+            }
-                    expandedByContainer = true;
-                }
-                else if (!containerExpanded.NewValue && expandedByContainer)
-                {
-                    Expanded.Value = false;
-                    expandedByContainer = false;
-                }
 
-                updateActiveState();
+            headerContent.FadeColour(expanded.NewValue ? Color4.White : OsuColour.Gray(0.5f), 200, Easing.OutQuint);
-            }, true);
-
-            Expanded.BindValueChanged(v =>
-            {
-                // clearing transforms can break autosizing, see: https://github.com/ppy/osu-framework/issues/5064
-                if (v.NewValue != v.OldValue)
-                    content.ClearTransforms();
-
-                if (v.NewValue)
-                    content.AutoSizeAxes = Axes.Y;
-                else
-                {
-                    content.AutoSizeAxes = Axes.None;
-                    content.ResizeHeightTo(0, transition_duration, Easing.OutQuint);
-                }
-
-                button.FadeColour(Expanded.Value ? expandedColour : Color4.White, 200, Easing.InOutQuint);
-            }, true);
-
-            this.Delay(600).Schedule(updateActiveState);
         }
 
-        protected override bool OnHover(HoverEvent e)
+        private void updateFadeState()
         {
-            updateActiveState();
+            this.FadeTo(IsHovered ? 1 : inactive_alpha, fade_duration, Easing.OutQuint);
-            return false;
-        }
-
-        protected override void OnHoverLost(HoverLostEvent e)
-        {
-            updateActiveState();
-            base.OnHoverLost(e);
-        }
-
-        [BackgroundDependencyLoader]
-        private void load(OsuColour colours)
-        {
-            expandedColour = colours.Yellow;
-        }
-
-        private void updateActiveState()
-        {
-            this.FadeTo(IsHovered || expandingContainer?.Expanded.Value == true ? 1 : inactive_alpha, fade_duration, Easing.OutQuint);
         }
 
         protected override Container<Drawable> Content => content;