From 949e30c4b4d85ed4b50e04494fe20fd3f6143ab7 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 19:36:41 +0900 Subject: [PATCH 1/7] Remove auto-expansion of individual toolbox groups when parent expanding container expands --- osu.Game/Overlays/SettingsToolboxGroup.cs | 54 ++++++++--------------- 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/osu.Game/Overlays/SettingsToolboxGroup.cs b/osu.Game/Overlays/SettingsToolboxGroup.cs index b9e5283a44..ca4864293c 100644 --- a/osu.Game/Overlays/SettingsToolboxGroup.cs +++ b/osu.Game/Overlays/SettingsToolboxGroup.cs @@ -135,60 +135,31 @@ namespace osu.Game.Overlays headerText.FadeTo(headerText.DrawWidth < DrawWidth ? 1 : 0, 150, Easing.OutQuint); } - [Resolved(canBeNull: true)] - private IExpandingContainer expandingContainer { get; set; } - - private bool expandedByContainer; - protected override void LoadComplete() { base.LoadComplete(); - expandingContainer?.Expanded.BindValueChanged(containerExpanded => - { - if (containerExpanded.NewValue && !Expanded.Value) - { - Expanded.Value = true; - expandedByContainer = true; - } - else if (!containerExpanded.NewValue && expandedByContainer) - { - Expanded.Value = false; - expandedByContainer = false; - } - - updateActiveState(); - }, 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); + Scheduler.AddOnce(updateExpandedState); }, true); - this.Delay(600).Schedule(updateActiveState); + this.Delay(600).Schedule(updateFadeState); } protected override bool OnHover(HoverEvent e) { - updateActiveState(); + updateFadeState(); return false; } protected override void OnHoverLost(HoverLostEvent e) { - updateActiveState(); + updateFadeState(); base.OnHoverLost(e); } @@ -198,9 +169,22 @@ namespace osu.Game.Overlays expandedColour = colours.Yellow; } - private void updateActiveState() + private void updateExpandedState() { - this.FadeTo(IsHovered || expandingContainer?.Expanded.Value == true ? 1 : inactive_alpha, fade_duration, Easing.OutQuint); + if (Expanded.Value) + 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); + } + + private void updateFadeState() + { + this.FadeTo(IsHovered ? 1 : inactive_alpha, fade_duration, Easing.OutQuint); } protected override Container Content => content; From 59add66632fcd15cad5b81f0baf99653bdabb483 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 19:39:00 +0900 Subject: [PATCH 2/7] Remove unnecessary autosize workaround (was fixed long ago) --- osu.Game/Overlays/SettingsToolboxGroup.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/osu.Game/Overlays/SettingsToolboxGroup.cs b/osu.Game/Overlays/SettingsToolboxGroup.cs index ca4864293c..10a09f3965 100644 --- a/osu.Game/Overlays/SettingsToolboxGroup.cs +++ b/osu.Game/Overlays/SettingsToolboxGroup.cs @@ -141,10 +141,6 @@ namespace osu.Game.Overlays Expanded.BindValueChanged(v => { - // clearing transforms can break autosizing, see: https://github.com/ppy/osu-framework/issues/5064 - if (v.NewValue != v.OldValue) - content.ClearTransforms(); - Scheduler.AddOnce(updateExpandedState); }, true); From b432885e5f4b20cee67ab44b3321df1ea954ecca Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 19:39:09 +0900 Subject: [PATCH 3/7] Tidy up ordering of `SettingsToolboxGroup` --- osu.Game/Overlays/SettingsToolboxGroup.cs | 47 +++++++++++------------ 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/osu.Game/Overlays/SettingsToolboxGroup.cs b/osu.Game/Overlays/SettingsToolboxGroup.cs index 10a09f3965..eb8c0248e7 100644 --- a/osu.Game/Overlays/SettingsToolboxGroup.cs +++ b/osu.Game/Overlays/SettingsToolboxGroup.cs @@ -117,32 +117,17 @@ namespace osu.Game.Overlays }; } - protected override bool OnInvalidate(Invalidation invalidation, InvalidationSource source) + [BackgroundDependencyLoader] + private void load(OsuColour colours) { - if (invalidation.HasFlagFast(Invalidation.DrawSize)) - headerTextVisibilityCache.Invalidate(); - - return base.OnInvalidate(invalidation, source); - } - - protected override void Update() - { - base.Update(); - - if (!headerTextVisibilityCache.IsValid) - // These toolbox grouped may be contracted to only show icons. - // For now, let's hide the header to avoid text truncation weirdness in such cases. - headerText.FadeTo(headerText.DrawWidth < DrawWidth ? 1 : 0, 150, Easing.OutQuint); + expandedColour = colours.Yellow; } protected override void LoadComplete() { base.LoadComplete(); - Expanded.BindValueChanged(v => - { - Scheduler.AddOnce(updateExpandedState); - }, true); + Expanded.BindValueChanged(updateExpandedState, true); this.Delay(600).Schedule(updateFadeState); } @@ -159,15 +144,27 @@ namespace osu.Game.Overlays base.OnHoverLost(e); } - [BackgroundDependencyLoader] - private void load(OsuColour colours) + protected override void Update() { - expandedColour = colours.Yellow; + base.Update(); + + if (!headerTextVisibilityCache.IsValid) + // These toolbox grouped may be contracted to only show icons. + // For now, let's hide the header to avoid text truncation weirdness in such cases. + headerText.FadeTo(headerText.DrawWidth < DrawWidth ? 1 : 0, 150, Easing.OutQuint); } - private void updateExpandedState() + protected override bool OnInvalidate(Invalidation invalidation, InvalidationSource source) { - if (Expanded.Value) + if (invalidation.HasFlagFast(Invalidation.DrawSize)) + headerTextVisibilityCache.Invalidate(); + + return base.OnInvalidate(invalidation, source); + } + + private void updateExpandedState(ValueChangedEvent expanded) + { + if (expanded.NewValue) content.AutoSizeAxes = Axes.Y; else { @@ -175,7 +172,7 @@ namespace osu.Game.Overlays content.ResizeHeightTo(0, transition_duration, Easing.OutQuint); } - button.FadeColour(Expanded.Value ? expandedColour : Color4.White, 200, Easing.InOutQuint); + button.FadeColour(expanded.NewValue ? expandedColour : Color4.White, 200, Easing.InOutQuint); } private void updateFadeState() From 88c190f3e359f9cb3bf9449dba97d19af0de7329 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 6 May 2022 19:45:14 +0900 Subject: [PATCH 4/7] Change colour denoting expanded state to be gray rather than yellow I always found the yellow colour very non-descript in this case. Gray seems to work better? --- osu.Game/Overlays/SettingsToolboxGroup.cs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/osu.Game/Overlays/SettingsToolboxGroup.cs b/osu.Game/Overlays/SettingsToolboxGroup.cs index eb8c0248e7..fd81d092c9 100644 --- a/osu.Game/Overlays/SettingsToolboxGroup.cs +++ b/osu.Game/Overlays/SettingsToolboxGroup.cs @@ -39,10 +39,10 @@ namespace osu.Game.Overlays public BindableBool Expanded { get; } = new BindableBool(true); - private Color4 expandedColour; - private readonly OsuSpriteText headerText; + private readonly Container headerContent; + /// /// Create a new instance. /// @@ -71,7 +71,7 @@ namespace osu.Game.Overlays AutoSizeAxes = Axes.Y, Children = new Drawable[] { - new Container + headerContent = new Container { Name = @"Header", Origin = Anchor.TopCentre, @@ -117,12 +117,6 @@ namespace osu.Game.Overlays }; } - [BackgroundDependencyLoader] - private void load(OsuColour colours) - { - expandedColour = colours.Yellow; - } - protected override void LoadComplete() { base.LoadComplete(); @@ -172,7 +166,7 @@ namespace osu.Game.Overlays content.ResizeHeightTo(0, transition_duration, Easing.OutQuint); } - button.FadeColour(expanded.NewValue ? expandedColour : Color4.White, 200, Easing.InOutQuint); + headerContent.FadeColour(expanded.NewValue ? Color4.White : OsuColour.Gray(0.5f), 200, Easing.OutQuint); } private void updateFadeState() From a915b7333c8935ce0c643431824806059d494cd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 6 May 2022 13:08:54 +0200 Subject: [PATCH 5/7] Remove unused using directive --- osu.Game/Overlays/SettingsToolboxGroup.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game/Overlays/SettingsToolboxGroup.cs b/osu.Game/Overlays/SettingsToolboxGroup.cs index fd81d092c9..f9cfd8ff2a 100644 --- a/osu.Game/Overlays/SettingsToolboxGroup.cs +++ b/osu.Game/Overlays/SettingsToolboxGroup.cs @@ -1,7 +1,6 @@ // Copyright (c) ppy Pty Ltd . 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; From e9d52aa9547f0df158310fac9bf7a632232d34fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 6 May 2022 13:09:37 +0200 Subject: [PATCH 6/7] Remove not-accessed field --- osu.Game/Overlays/SettingsToolboxGroup.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/osu.Game/Overlays/SettingsToolboxGroup.cs b/osu.Game/Overlays/SettingsToolboxGroup.cs index f9cfd8ff2a..808d4fc422 100644 --- a/osu.Game/Overlays/SettingsToolboxGroup.cs +++ b/osu.Game/Overlays/SettingsToolboxGroup.cs @@ -34,7 +34,6 @@ 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); @@ -87,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, From 19297375e270839c443bfd3165b79c641f692de9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Fri, 6 May 2022 13:44:04 +0200 Subject: [PATCH 7/7] Update tests to reflect new expected behaviour --- .../Visual/UserInterface/TestSceneExpandingContainer.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneExpandingContainer.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneExpandingContainer.cs index 288c0cb140..2bb6e58448 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneExpandingContainer.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneExpandingContainer.cs @@ -99,15 +99,15 @@ namespace osu.Game.Tests.Visual.UserInterface } /// - /// 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. /// [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);