From 910d74fdda85433e2cf6b1f62daa3edc4a75f816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 11:59:38 +0200 Subject: [PATCH 1/4] Add failing test coverage for double-click on disabled sliders --- .../UserInterface/TestSceneRoundedSliderBar.cs | 17 +++++++++++++++++ .../UserInterface/TestSceneShearedSliderBar.cs | 17 +++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs index 419e88137c..ae52c26fd2 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs @@ -55,5 +55,22 @@ namespace osu.Game.Tests.Visual.UserInterface AddAssert("slider is default", () => slider.Current.IsDefault); } + + [Test] + public void TestNubDoubleClickOnDisabledSliderDoesNothing() + { + AddStep("set slider to 1", () => slider.Current.Value = 1); + AddStep("disable slider", () => slider.Current.Disabled = true); + + AddStep("move mouse to nub", () => InputManager.MoveMouseTo(slider.ChildrenOfType().Single())); + + AddStep("double click nub", () => + { + InputManager.Click(MouseButton.Left); + InputManager.Click(MouseButton.Left); + }); + + AddAssert("slider is still at 1", () => slider.Current.Value, () => Is.EqualTo(1)); + } } } diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs index d459a2c701..334fc4563a 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs @@ -55,5 +55,22 @@ namespace osu.Game.Tests.Visual.UserInterface AddAssert("slider is default", () => slider.Current.IsDefault); } + + [Test] + public void TestNubDoubleClickOnDisabledSliderDoesNothing() + { + AddStep("set slider to 1", () => slider.Current.Value = 1); + AddStep("disable slider", () => slider.Current.Disabled = true); + + AddStep("move mouse to nub", () => InputManager.MoveMouseTo(slider.ChildrenOfType().Single())); + + AddStep("double click nub", () => + { + InputManager.Click(MouseButton.Left); + InputManager.Click(MouseButton.Left); + }); + + AddAssert("slider is still at 1", () => slider.Current.Value, () => Is.EqualTo(1)); + } } } From 96437c4518a394c2354853b95c5f07853c6340d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 12:03:35 +0200 Subject: [PATCH 2/4] Fix tests specifying float precision for double bindable Would cause assignments to `.Value` to become imprecise even if the underlying bindable could represent the value 100% accurately. --- .../Visual/UserInterface/TestSceneRoundedSliderBar.cs | 2 +- .../Visual/UserInterface/TestSceneShearedSliderBar.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs index ae52c26fd2..311034d595 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs @@ -20,7 +20,7 @@ namespace osu.Game.Tests.Visual.UserInterface private readonly BindableDouble current = new BindableDouble(5) { - Precision = 0.1f, + Precision = 0.1, MinValue = 0, MaxValue = 15 }; diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs index 334fc4563a..a5072b4e60 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs @@ -20,7 +20,7 @@ namespace osu.Game.Tests.Visual.UserInterface private readonly BindableDouble current = new BindableDouble(5) { - Precision = 0.1f, + Precision = 0.1, MinValue = 0, MaxValue = 15 }; From 89fec95b016364d2a97a30a1d6cbbbf6e46e19a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 12:11:25 +0200 Subject: [PATCH 3/4] Fix cross-test data dependency by recreating slider every time --- .../TestSceneRoundedSliderBar.cs | 22 +++++++++---------- .../TestSceneShearedSliderBar.cs | 22 +++++++++---------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs index 311034d595..66d54c8562 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneRoundedSliderBar.cs @@ -18,26 +18,24 @@ namespace osu.Game.Tests.Visual.UserInterface [Cached] private OverlayColourProvider colourProvider { get; set; } = new OverlayColourProvider(OverlayColourScheme.Purple); - private readonly BindableDouble current = new BindableDouble(5) - { - Precision = 0.1, - MinValue = 0, - MaxValue = 15 - }; - private RoundedSliderBar slider = null!; - [BackgroundDependencyLoader] - private void load() + [SetUpSteps] + public void SetUpSteps() { - Child = slider = new RoundedSliderBar + AddStep("create slider", () => Child = slider = new RoundedSliderBar { Anchor = Anchor.Centre, Origin = Anchor.Centre, - Current = current, + Current = new BindableDouble(5) + { + Precision = 0.1, + MinValue = 0, + MaxValue = 15 + }, RelativeSizeAxes = Axes.X, Width = 0.4f - }; + }); } [Test] diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs index a5072b4e60..c3038ddb3d 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneShearedSliderBar.cs @@ -18,26 +18,24 @@ namespace osu.Game.Tests.Visual.UserInterface [Cached] private OverlayColourProvider colourProvider { get; set; } = new OverlayColourProvider(OverlayColourScheme.Purple); - private readonly BindableDouble current = new BindableDouble(5) - { - Precision = 0.1, - MinValue = 0, - MaxValue = 15 - }; - private ShearedSliderBar slider = null!; - [BackgroundDependencyLoader] - private void load() + [SetUpSteps] + public void SetUpSteps() { - Child = slider = new ShearedSliderBar + AddStep("create slider", () => Child = slider = new ShearedSliderBar { Anchor = Anchor.Centre, Origin = Anchor.Centre, - Current = current, + Current = new BindableDouble(5) + { + Precision = 0.1, + MinValue = 0, + MaxValue = 15 + }, RelativeSizeAxes = Axes.X, Width = 0.4f - }; + }); } [Test] From 3b9c4c9d530eeee26f29a920299b0b363cd7348c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 26 Oct 2023 12:04:27 +0200 Subject: [PATCH 4/4] Do not revert to default value when double-clicking disabled slider Closes https://github.com/ppy/osu/issues/25228. --- osu.Game/Graphics/UserInterface/RoundedSliderBar.cs | 6 +++++- osu.Game/Graphics/UserInterface/ShearedSliderBar.cs | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/osu.Game/Graphics/UserInterface/RoundedSliderBar.cs b/osu.Game/Graphics/UserInterface/RoundedSliderBar.cs index e5976fe893..0981881ead 100644 --- a/osu.Game/Graphics/UserInterface/RoundedSliderBar.cs +++ b/osu.Game/Graphics/UserInterface/RoundedSliderBar.cs @@ -98,7 +98,11 @@ namespace osu.Game.Graphics.UserInterface Origin = Anchor.TopCentre, RelativePositionAxes = Axes.X, Current = { Value = true }, - OnDoubleClicked = () => Current.SetDefault(), + OnDoubleClicked = () => + { + if (!Current.Disabled) + Current.SetDefault(); + }, }, }, hoverClickSounds = new HoverClickSounds() diff --git a/osu.Game/Graphics/UserInterface/ShearedSliderBar.cs b/osu.Game/Graphics/UserInterface/ShearedSliderBar.cs index 9ef5f3073a..60a6670492 100644 --- a/osu.Game/Graphics/UserInterface/ShearedSliderBar.cs +++ b/osu.Game/Graphics/UserInterface/ShearedSliderBar.cs @@ -101,7 +101,11 @@ namespace osu.Game.Graphics.UserInterface Origin = Anchor.TopCentre, RelativePositionAxes = Axes.X, Current = { Value = true }, - OnDoubleClicked = () => Current.SetDefault(), + OnDoubleClicked = () => + { + if (!Current.Disabled) + Current.SetDefault(); + }, }, }, hoverClickSounds = new HoverClickSounds()