From ceb6276d2ffd3e0e9b900225ea28bd12efdca146 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 9 May 2022 18:19:29 +0300 Subject: [PATCH 1/5] Add failing test case --- .../UserInterface/TestSceneRoundedButton.cs | 48 ++++++++++++++++--- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedButton.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedButton.cs index 9ccfba7c74..ef3c0c7fa4 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedButton.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedButton.cs @@ -2,11 +2,16 @@ // See the LICENCE file in the repository root for full licence text. using System; +using System.Collections.Generic; +using System.Linq; using NUnit.Framework; +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.Graphics.UserInterfaceV2; +using osu.Game.Overlays; +using osuTK; namespace osu.Game.Tests.Visual.UserInterface { @@ -22,15 +27,10 @@ namespace osu.Game.Tests.Visual.UserInterface RelativeSizeAxes = Axes.Both, Children = new Drawable[] { - new Box - { - RelativeSizeAxes = Axes.Both, - Colour = Colour4.DarkGray - }, button = new RoundedButton { Width = 400, - Text = "Test button", + Text = "Test Button", Anchor = Anchor.Centre, Origin = Anchor.Centre, Action = () => { } @@ -40,5 +40,39 @@ namespace osu.Game.Tests.Visual.UserInterface AddToggleStep("toggle disabled", disabled => button.Action = disabled ? (Action)null : () => { }); } + + [Test] + public void TestOverlay() + { + IEnumerable schemes = Enum.GetValues(typeof(OverlayColourScheme)).Cast(); + + AddStep("create buttons", () => + { + Child = new FillFlowContainer + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + AutoSizeAxes = Axes.Both, + Direction = FillDirection.Vertical, + Spacing = new Vector2(5f), + ChildrenEnumerable = schemes.Select(c => new DependencyProvidingContainer + { + AutoSizeAxes = Axes.Both, + CachedDependencies = new (Type, object)[] { (typeof(OverlayColourProvider), new OverlayColourProvider(c)) }, + Child = new RoundedButton + { + Width = 400, + Text = $"Test {c}", + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Action = () => { }, + } + }), + }; + }); + + AddAssert("first button has correct colour", () => this.ChildrenOfType().First().BackgroundColour == new OverlayColourProvider(schemes.First()).Highlight1); + AddToggleStep("toggle disabled", disabled => this.ChildrenOfType().ForEach(b => b.Action = disabled ? (Action)null : () => { })); + } } } From 172524ff8a3812ffce89361c5552a6426529df33 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 9 May 2022 18:21:00 +0300 Subject: [PATCH 2/5] Move default background colour specification to `OsuButton` --- osu.Game/Graphics/UserInterface/OsuButton.cs | 8 +++++--- osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs | 11 ----------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/OsuButton.cs b/osu.Game/Graphics/UserInterface/OsuButton.cs index 29a797bd78..6fa63e5874 100644 --- a/osu.Game/Graphics/UserInterface/OsuButton.cs +++ b/osu.Game/Graphics/UserInterface/OsuButton.cs @@ -2,6 +2,7 @@ // See the LICENCE file in the repository root for full licence text. using System.Diagnostics; +using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; @@ -12,6 +13,7 @@ using osu.Framework.Graphics.UserInterface; using osu.Framework.Input.Events; using osu.Framework.Localisation; using osu.Game.Graphics.Sprites; +using osu.Game.Overlays; using osuTK.Graphics; namespace osu.Game.Graphics.UserInterface @@ -86,11 +88,11 @@ namespace osu.Game.Graphics.UserInterface AddInternal(new HoverClickSounds(hoverSounds.Value)); } - [BackgroundDependencyLoader] - private void load(OsuColour colours) + [BackgroundDependencyLoader(permitNulls: true)] + private void load([CanBeNull] OverlayColourProvider overlayColourProvider, OsuColour colours) { if (backgroundColour == null) - BackgroundColour = colours.BlueDark; + BackgroundColour = overlayColourProvider?.Highlight1 ?? colours.Blue3; } protected override void LoadComplete() diff --git a/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs b/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs index f535a32b39..b1529774d9 100644 --- a/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs +++ b/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs @@ -2,13 +2,9 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; -using JetBrains.Annotations; -using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Graphics.UserInterface; -using osu.Game.Overlays; -using osuTK.Graphics; namespace osu.Game.Graphics.UserInterfaceV2 { @@ -26,13 +22,6 @@ namespace osu.Game.Graphics.UserInterfaceV2 } } - [BackgroundDependencyLoader(true)] - private void load([CanBeNull] OverlayColourProvider overlayColourProvider, OsuColour colours) - { - if (BackgroundColour == Color4.White) - BackgroundColour = overlayColourProvider?.Highlight1 ?? colours.Blue3; - } - protected override void LoadComplete() { base.LoadComplete(); From ace25af949ab39946e4a8b57103b40247c96dbfa Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 9 May 2022 20:55:26 +0300 Subject: [PATCH 3/5] Revert "Move default background colour specification to `OsuButton`" This reverts commit 172524ff8a3812ffce89361c5552a6426529df33. --- osu.Game/Graphics/UserInterface/OsuButton.cs | 8 +++----- osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs | 11 +++++++++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/OsuButton.cs b/osu.Game/Graphics/UserInterface/OsuButton.cs index 6fa63e5874..29a797bd78 100644 --- a/osu.Game/Graphics/UserInterface/OsuButton.cs +++ b/osu.Game/Graphics/UserInterface/OsuButton.cs @@ -2,7 +2,6 @@ // See the LICENCE file in the repository root for full licence text. using System.Diagnostics; -using JetBrains.Annotations; using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; @@ -13,7 +12,6 @@ using osu.Framework.Graphics.UserInterface; using osu.Framework.Input.Events; using osu.Framework.Localisation; using osu.Game.Graphics.Sprites; -using osu.Game.Overlays; using osuTK.Graphics; namespace osu.Game.Graphics.UserInterface @@ -88,11 +86,11 @@ namespace osu.Game.Graphics.UserInterface AddInternal(new HoverClickSounds(hoverSounds.Value)); } - [BackgroundDependencyLoader(permitNulls: true)] - private void load([CanBeNull] OverlayColourProvider overlayColourProvider, OsuColour colours) + [BackgroundDependencyLoader] + private void load(OsuColour colours) { if (backgroundColour == null) - BackgroundColour = overlayColourProvider?.Highlight1 ?? colours.Blue3; + BackgroundColour = colours.BlueDark; } protected override void LoadComplete() diff --git a/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs b/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs index b1529774d9..f535a32b39 100644 --- a/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs +++ b/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs @@ -2,9 +2,13 @@ // See the LICENCE file in the repository root for full licence text. using System.Collections.Generic; +using JetBrains.Annotations; +using osu.Framework.Allocation; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Graphics.UserInterface; +using osu.Game.Overlays; +using osuTK.Graphics; namespace osu.Game.Graphics.UserInterfaceV2 { @@ -22,6 +26,13 @@ namespace osu.Game.Graphics.UserInterfaceV2 } } + [BackgroundDependencyLoader(true)] + private void load([CanBeNull] OverlayColourProvider overlayColourProvider, OsuColour colours) + { + if (BackgroundColour == Color4.White) + BackgroundColour = overlayColourProvider?.Highlight1 ?? colours.Blue3; + } + protected override void LoadComplete() { base.LoadComplete(); From 1fcfeac05f8c499ae86c36d32bcfdaf0bfc736f4 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 9 May 2022 18:52:09 +0300 Subject: [PATCH 4/5] Fix `RoundedButton` not using its default background colour --- osu.Game/Graphics/UserInterface/OsuButton.cs | 31 ++++++++++++++----- .../Graphics/UserInterfaceV2/RoundedButton.cs | 4 +-- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/OsuButton.cs b/osu.Game/Graphics/UserInterface/OsuButton.cs index 29a797bd78..08514d94c3 100644 --- a/osu.Game/Graphics/UserInterface/OsuButton.cs +++ b/osu.Game/Graphics/UserInterface/OsuButton.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 System.Diagnostics; using osu.Framework.Allocation; using osu.Framework.Extensions.Color4Extensions; using osu.Framework.Graphics; @@ -33,9 +32,12 @@ namespace osu.Game.Graphics.UserInterface private Color4? backgroundColour; + /// + /// Sets a custom background colour to this button, replacing the provided default. + /// public Color4 BackgroundColour { - get => backgroundColour ?? Color4.White; + get => backgroundColour ?? defaultBackgroundColour; set { backgroundColour = value; @@ -43,6 +45,23 @@ namespace osu.Game.Graphics.UserInterface } } + private Color4 defaultBackgroundColour; + + /// + /// Sets a default background colour to this button. + /// + protected Color4 DefaultBackgroundColour + { + get => defaultBackgroundColour; + set + { + defaultBackgroundColour = value; + + if (backgroundColour == null) + Background.FadeColour(value); + } + } + protected override Container Content { get; } protected Box Hover; @@ -89,8 +108,7 @@ namespace osu.Game.Graphics.UserInterface [BackgroundDependencyLoader] private void load(OsuColour colours) { - if (backgroundColour == null) - BackgroundColour = colours.BlueDark; + DefaultBackgroundColour = colours.BlueDark; } protected override void LoadComplete() @@ -106,10 +124,7 @@ namespace osu.Game.Graphics.UserInterface protected override bool OnClick(ClickEvent e) { if (Enabled.Value) - { - Debug.Assert(backgroundColour != null); - Background.FlashColour(backgroundColour.Value.Lighten(0.4f), 200); - } + Background.FlashColour(BackgroundColour.Lighten(0.4f), 200); return base.OnClick(e); } diff --git a/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs b/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs index f535a32b39..ec56b6d784 100644 --- a/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs +++ b/osu.Game/Graphics/UserInterfaceV2/RoundedButton.cs @@ -8,7 +8,6 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Game.Graphics.UserInterface; using osu.Game.Overlays; -using osuTK.Graphics; namespace osu.Game.Graphics.UserInterfaceV2 { @@ -29,8 +28,7 @@ namespace osu.Game.Graphics.UserInterfaceV2 [BackgroundDependencyLoader(true)] private void load([CanBeNull] OverlayColourProvider overlayColourProvider, OsuColour colours) { - if (BackgroundColour == Color4.White) - BackgroundColour = overlayColourProvider?.Highlight1 ?? colours.Blue3; + DefaultBackgroundColour = overlayColourProvider?.Highlight1 ?? colours.Blue3; } protected override void LoadComplete() From 5726cf660f7ec219804fb0ebb6315ab5af5017ed Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 9 May 2022 21:19:34 +0300 Subject: [PATCH 5/5] Improve test coverage to use existing `ThemeComparisonTestScene` --- .../UserInterface/TestSceneRoundedButton.cs | 74 +++++-------------- 1 file changed, 18 insertions(+), 56 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedButton.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedButton.cs index ef3c0c7fa4..f45c55d912 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedButton.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedButton.cs @@ -1,78 +1,40 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System; -using System.Collections.Generic; using System.Linq; using NUnit.Framework; -using osu.Framework.Extensions.IEnumerableExtensions; +using osu.Framework.Bindables; using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; using osu.Framework.Testing; using osu.Game.Graphics.UserInterfaceV2; using osu.Game.Overlays; -using osuTK; namespace osu.Game.Tests.Visual.UserInterface { - public class TestSceneRoundedButton : OsuTestScene + public class TestSceneRoundedButton : ThemeComparisonTestScene { - [Test] - public void TestBasic() + private readonly BindableBool enabled = new BindableBool(true); + + protected override Drawable CreateContent() => new RoundedButton { - RoundedButton button = null; + Width = 400, + Text = "Test button", + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Enabled = { BindTarget = enabled }, + }; - AddStep("create button", () => Child = new Container - { - RelativeSizeAxes = Axes.Both, - Children = new Drawable[] - { - button = new RoundedButton - { - Width = 400, - Text = "Test Button", - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Action = () => { } - } - } - }); - - AddToggleStep("toggle disabled", disabled => button.Action = disabled ? (Action)null : () => { }); + [Test] + public void TestDisabled() + { + AddToggleStep("toggle disabled", disabled => enabled.Value = !disabled); } [Test] - public void TestOverlay() + public void TestBackgroundColour() { - IEnumerable schemes = Enum.GetValues(typeof(OverlayColourScheme)).Cast(); - - AddStep("create buttons", () => - { - Child = new FillFlowContainer - { - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - AutoSizeAxes = Axes.Both, - Direction = FillDirection.Vertical, - Spacing = new Vector2(5f), - ChildrenEnumerable = schemes.Select(c => new DependencyProvidingContainer - { - AutoSizeAxes = Axes.Both, - CachedDependencies = new (Type, object)[] { (typeof(OverlayColourProvider), new OverlayColourProvider(c)) }, - Child = new RoundedButton - { - Width = 400, - Text = $"Test {c}", - Anchor = Anchor.Centre, - Origin = Anchor.Centre, - Action = () => { }, - } - }), - }; - }); - - AddAssert("first button has correct colour", () => this.ChildrenOfType().First().BackgroundColour == new OverlayColourProvider(schemes.First()).Highlight1); - AddToggleStep("toggle disabled", disabled => this.ChildrenOfType().ForEach(b => b.Action = disabled ? (Action)null : () => { })); + AddStep("set red scheme", () => CreateThemedContent(OverlayColourScheme.Red)); + AddAssert("first button has correct colour", () => Cell(0, 1).ChildrenOfType().First().BackgroundColour == new OverlayColourProvider(OverlayColourScheme.Red).Highlight1); } } }