From 2e2b0c4e4109905f46e17f478ab1c0e936abfb23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 1 Aug 2023 00:22:08 +0200 Subject: [PATCH 1/5] Move `SliderWithTextBoxInput` to more general namespace --- .../UserInterfaceV2}/SliderWithTextBoxInput.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) rename osu.Game/{Screens/Edit/Timing => Graphics/UserInterfaceV2}/SliderWithTextBoxInput.cs (97%) diff --git a/osu.Game/Screens/Edit/Timing/SliderWithTextBoxInput.cs b/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs similarity index 97% rename from osu.Game/Screens/Edit/Timing/SliderWithTextBoxInput.cs rename to osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs index 1bf0e5299d..3f4e133d45 100644 --- a/osu.Game/Screens/Edit/Timing/SliderWithTextBoxInput.cs +++ b/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs @@ -8,12 +8,11 @@ using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.UserInterface; using osu.Framework.Localisation; -using osu.Game.Graphics.UserInterfaceV2; using osu.Game.Overlays.Settings; using osu.Game.Utils; using osuTK; -namespace osu.Game.Screens.Edit.Timing +namespace osu.Game.Graphics.UserInterfaceV2 { public partial class SliderWithTextBoxInput : CompositeDrawable, IHasCurrentValue where T : struct, IEquatable, IComparable, IConvertible From ca81f233563aca7d39822a6d551fc8824c1d3d48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 1 Aug 2023 00:44:05 +0200 Subject: [PATCH 2/5] Add test covering non-instantaneous behaviour --- .../TestSceneSliderWithTextBoxInput.cs | 63 +++++++++++++++++++ .../UserInterfaceV2/SliderWithTextBoxInput.cs | 2 - 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs new file mode 100644 index 0000000000..dc70e2b9cb --- /dev/null +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs @@ -0,0 +1,63 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System.Linq; +using NUnit.Framework; +using osu.Framework.Bindables; +using osu.Framework.Graphics; +using osu.Framework.Testing; +using osu.Game.Graphics.UserInterface; +using osu.Game.Graphics.UserInterfaceV2; +using osuTK.Input; + +namespace osu.Game.Tests.Visual.UserInterface +{ + public partial class TestSceneSliderWithTextBoxInput : OsuManualInputManagerTestScene + { + private SliderWithTextBoxInput sliderWithTextBoxInput = null!; + + private OsuSliderBar slider => sliderWithTextBoxInput.ChildrenOfType>().Single(); + private Nub nub => sliderWithTextBoxInput.ChildrenOfType().Single(); + private OsuTextBox textBox => sliderWithTextBoxInput.ChildrenOfType().Single(); + + [SetUpSteps] + public void SetUpSteps() + { + AddStep("create slider", () => Child = sliderWithTextBoxInput = new SliderWithTextBoxInput("Test Slider") + { + Anchor = Anchor.Centre, + Origin = Anchor.Centre, + Width = 0.5f, + Current = new BindableFloat + { + MinValue = -5, + MaxValue = 5, + Precision = 0.2f + } + }); + } + + [Test] + public void TestNonInstantaneousMode() + { + AddStep("focus textbox", () => InputManager.ChangeFocus(textBox)); + AddStep("change text", () => textBox.Text = "3"); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.Zero); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.Zero); + + AddStep("commit text", () => InputManager.Key(Key.Enter)); + AddAssert("slider moved", () => slider.Current.Value, () => Is.EqualTo(3)); + AddAssert("current changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(3)); + + AddStep("move mouse to nub", () => InputManager.MoveMouseTo(nub)); + AddStep("hold left mouse", () => InputManager.PressButton(MouseButton.Left)); + AddStep("move mouse to minimum", () => InputManager.MoveMouseTo(sliderWithTextBoxInput.ScreenSpaceDrawQuad.BottomLeft)); + AddAssert("textbox not changed", () => textBox.Current.Value, () => Is.EqualTo("3")); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(3)); + + AddStep("release left mouse", () => InputManager.ReleaseButton(MouseButton.Left)); + AddAssert("textbox changed", () => textBox.Current.Value, () => Is.EqualTo("-5")); + AddAssert("current changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + } + } +} diff --git a/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs b/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs index 3f4e133d45..38de0ea94a 100644 --- a/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs +++ b/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs @@ -51,8 +51,6 @@ namespace osu.Game.Graphics.UserInterfaceV2 textBox.OnCommit += (t, isNew) => { - if (!isNew) return; - try { switch (slider.Current) From 0b019f1d4526dbad742fb8d36d3d7923b575c091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 1 Aug 2023 00:45:57 +0200 Subject: [PATCH 3/5] Add test covering desired instantaneous behaviour --- .../TestSceneSliderWithTextBoxInput.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs index dc70e2b9cb..37f18cd3dd 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs @@ -59,5 +59,28 @@ namespace osu.Game.Tests.Visual.UserInterface AddAssert("textbox changed", () => textBox.Current.Value, () => Is.EqualTo("-5")); AddAssert("current changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); } + + [Test] + public void TestInstantaneousMode() + { + AddStep("focus textbox", () => InputManager.ChangeFocus(textBox)); + AddStep("change text", () => textBox.Text = "3"); + AddAssert("slider moved", () => slider.Current.Value, () => Is.EqualTo(3)); + AddAssert("current changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(3)); + + AddStep("commit text", () => InputManager.Key(Key.Enter)); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.EqualTo(3)); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(3)); + + AddStep("move mouse to nub", () => InputManager.MoveMouseTo(nub)); + AddStep("hold left mouse", () => InputManager.PressButton(MouseButton.Left)); + AddStep("move mouse to minimum", () => InputManager.MoveMouseTo(sliderWithTextBoxInput.ScreenSpaceDrawQuad.BottomLeft)); + AddAssert("textbox changed", () => textBox.Current.Value, () => Is.EqualTo("-5")); + AddAssert("current changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + + AddStep("release left mouse", () => InputManager.ReleaseButton(MouseButton.Left)); + AddAssert("textbox not changed", () => textBox.Current.Value, () => Is.EqualTo("-5")); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + } } } From 4fb0ff8800281dc7c4fd97c99359791e1edd6390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 1 Aug 2023 00:51:29 +0200 Subject: [PATCH 4/5] Implement instantaneous `SliderWithTextBoxInput` mode --- .../TestSceneSliderWithTextBoxInput.cs | 4 + .../UserInterfaceV2/SliderWithTextBoxInput.cs | 128 ++++++++++++------ 2 files changed, 88 insertions(+), 44 deletions(-) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs index 37f18cd3dd..797afccc77 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs @@ -40,6 +40,8 @@ namespace osu.Game.Tests.Visual.UserInterface [Test] public void TestNonInstantaneousMode() { + AddStep("set instantaneous to false", () => sliderWithTextBoxInput.Instantaneous = false); + AddStep("focus textbox", () => InputManager.ChangeFocus(textBox)); AddStep("change text", () => textBox.Text = "3"); AddAssert("slider not moved", () => slider.Current.Value, () => Is.Zero); @@ -63,6 +65,8 @@ namespace osu.Game.Tests.Visual.UserInterface [Test] public void TestInstantaneousMode() { + AddStep("set instantaneous to true", () => sliderWithTextBoxInput.Instantaneous = true); + AddStep("focus textbox", () => InputManager.ChangeFocus(textBox)); AddStep("change text", () => textBox.Text = "3"); AddAssert("slider moved", () => slider.Current.Value, () => Is.EqualTo(3)); diff --git a/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs b/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs index 38de0ea94a..fc0e4d2083 100644 --- a/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs +++ b/osu.Game/Graphics/UserInterfaceV2/SliderWithTextBoxInput.cs @@ -17,12 +17,42 @@ namespace osu.Game.Graphics.UserInterfaceV2 public partial class SliderWithTextBoxInput : CompositeDrawable, IHasCurrentValue where T : struct, IEquatable, IComparable, IConvertible { + /// + /// A custom step value for each key press which actuates a change on this control. + /// + public float KeyboardStep + { + get => slider.KeyboardStep; + set => slider.KeyboardStep = value; + } + + public Bindable Current + { + get => slider.Current; + set => slider.Current = value; + } + + private bool instantaneous; + + /// + /// Whether changes to the slider should instantaneously transfer to the text box (and vice versa). + /// If , the transfer will happen on text box commit (explicit, or implicit via focus loss), or on slider drag end. + /// + public bool Instantaneous + { + get => instantaneous; + set + { + instantaneous = value; + slider.TransferValueOnCommit = !instantaneous; + } + } + private readonly SettingsSlider slider; + private readonly LabelledTextBox textBox; public SliderWithTextBoxInput(LocalisableString labelText) { - LabelledTextBox textBox; - RelativeSizeAxes = Axes.X; AutoSizeAxes = Axes.Y; @@ -49,55 +79,65 @@ namespace osu.Game.Graphics.UserInterfaceV2 }, }; - textBox.OnCommit += (t, isNew) => - { - try - { - switch (slider.Current) - { - case Bindable bindableInt: - bindableInt.Value = int.Parse(t.Text); - break; + textBox.OnCommit += textCommitted; + textBox.Current.BindValueChanged(textChanged); - case Bindable bindableDouble: - bindableDouble.Value = double.Parse(t.Text); - break; - - default: - slider.Current.Parse(t.Text); - break; - } - } - catch - { - // TriggerChange below will restore the previous text value on failure. - } - - // This is run regardless of parsing success as the parsed number may not actually trigger a change - // due to bindable clamping. Even in such a case we want to update the textbox to a sane visual state. - Current.TriggerChange(); - }; - - Current.BindValueChanged(_ => - { - decimal decimalValue = slider.Current.Value.ToDecimal(NumberFormatInfo.InvariantInfo); - textBox.Text = decimalValue.ToString($@"N{FormatUtils.FindPrecision(decimalValue)}"); - }, true); + Current.BindValueChanged(updateTextBoxFromSlider, true); } - /// - /// A custom step value for each key press which actuates a change on this control. - /// - public float KeyboardStep + private bool updatingFromTextBox; + + private void textChanged(ValueChangedEvent change) { - get => slider.KeyboardStep; - set => slider.KeyboardStep = value; + if (!instantaneous) return; + + tryUpdateSliderFromTextBox(); } - public Bindable Current + private void textCommitted(TextBox t, bool isNew) { - get => slider.Current; - set => slider.Current = value; + tryUpdateSliderFromTextBox(); + + // If the attempted update above failed, restore text box to match the slider. + Current.TriggerChange(); + } + + private void tryUpdateSliderFromTextBox() + { + updatingFromTextBox = true; + + try + { + switch (slider.Current) + { + case Bindable bindableInt: + bindableInt.Value = int.Parse(textBox.Current.Value); + break; + + case Bindable bindableDouble: + bindableDouble.Value = double.Parse(textBox.Current.Value); + break; + + default: + slider.Current.Parse(textBox.Current.Value); + break; + } + } + catch + { + // ignore parsing failures. + // sane state will eventually be restored by a commit (either explicit, or implicit via focus loss). + } + + updatingFromTextBox = false; + } + + private void updateTextBoxFromSlider(ValueChangedEvent _) + { + if (updatingFromTextBox) return; + + decimal decimalValue = slider.Current.Value.ToDecimal(NumberFormatInfo.InvariantInfo); + textBox.Text = decimalValue.ToString($@"N{FormatUtils.FindPrecision(decimalValue)}"); } } } From 049215f2dc1cbdd2ae17c10d28a0430b0dd28ee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 1 Aug 2023 00:54:40 +0200 Subject: [PATCH 5/5] Add test coverage for invalid input scenarios --- .../TestSceneSliderWithTextBoxInput.cs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs b/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs index 797afccc77..d23fcebae3 100644 --- a/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs +++ b/osu.Game.Tests/Visual/UserInterface/TestSceneSliderWithTextBoxInput.cs @@ -60,6 +60,26 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("release left mouse", () => InputManager.ReleaseButton(MouseButton.Left)); AddAssert("textbox changed", () => textBox.Current.Value, () => Is.EqualTo("-5")); AddAssert("current changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + + AddStep("focus textbox", () => InputManager.ChangeFocus(textBox)); + AddStep("set text to invalid", () => textBox.Text = "garbage"); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.EqualTo(-5)); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + + AddStep("commit text", () => InputManager.Key(Key.Enter)); + AddAssert("text restored", () => textBox.Text, () => Is.EqualTo("-5")); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.EqualTo(-5)); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + + AddStep("focus textbox", () => InputManager.ChangeFocus(textBox)); + AddStep("set text to invalid", () => textBox.Text = "garbage"); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.EqualTo(-5)); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + + AddStep("lose focus", () => InputManager.ChangeFocus(null)); + AddAssert("text restored", () => textBox.Text, () => Is.EqualTo("-5")); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.EqualTo(-5)); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); } [Test] @@ -85,6 +105,26 @@ namespace osu.Game.Tests.Visual.UserInterface AddStep("release left mouse", () => InputManager.ReleaseButton(MouseButton.Left)); AddAssert("textbox not changed", () => textBox.Current.Value, () => Is.EqualTo("-5")); AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + + AddStep("focus textbox", () => InputManager.ChangeFocus(textBox)); + AddStep("set text to invalid", () => textBox.Text = "garbage"); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.EqualTo(-5)); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + + AddStep("commit text", () => InputManager.Key(Key.Enter)); + AddAssert("text restored", () => textBox.Text, () => Is.EqualTo("-5")); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.EqualTo(-5)); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + + AddStep("focus textbox", () => InputManager.ChangeFocus(textBox)); + AddStep("set text to invalid", () => textBox.Text = "garbage"); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.EqualTo(-5)); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); + + AddStep("lose focus", () => InputManager.ChangeFocus(null)); + AddAssert("text restored", () => textBox.Text, () => Is.EqualTo("-5")); + AddAssert("slider not moved", () => slider.Current.Value, () => Is.EqualTo(-5)); + AddAssert("current not changed", () => sliderWithTextBoxInput.Current.Value, () => Is.EqualTo(-5)); } } }