diff --git a/src/Libraries/CoreNodeModels/Input/IntegerSlider.cs b/src/Libraries/CoreNodeModels/Input/IntegerSlider.cs index 7a6b1a94e30..9bc70ea67ac 100644 --- a/src/Libraries/CoreNodeModels/Input/IntegerSlider.cs +++ b/src/Libraries/CoreNodeModels/Input/IntegerSlider.cs @@ -291,7 +291,7 @@ public IntegerSlider64Bit() } // If the value field in the slider has a number greater than - // long.Maxvalue (or MinValue), the value will be changed to long.MaxValue (or MinValue) + // long.MaxValue (or MinValue), the value will be changed to long.MaxValue (or MinValue) // The property setter is overridden here to update the UI, in case the value is changed. public override long Value { @@ -315,35 +315,112 @@ protected override bool UpdateValueCore(UpdateValueParams updateValueParams) { case nameof(Min): case "MinText": - Min = ConvertStringToInt64(value); - return true; // UpdateValueCore handled. case nameof(Max): case "MaxText": - Max = ConvertStringToInt64(value); - return true; // UpdateValueCore handled. - case nameof(Value): - case "ValueText": - UpdateNodeInfo(value); - Value = ConvertStringToInt64(value); - return true; // UpdateValueCore handled. case nameof(Step): case "StepText": - Step = ConvertStringToInt64(value); - return true; + case nameof(Value): + case "ValueText": + if (string.IsNullOrEmpty(value)) + return false; + + // Reject anything that isn't a strict Int64 literal (no decimals/thousands + // separators). Distinguish out-of-range from non-numeric so the user gets + // an accurate error message; both cases keep the last valid value. + if (!TryParseInt64(value, out long parsed, out bool isOutOfRange)) + { + Info(Resources.IntegerSliderNonIntegerInputMessage, true); + + // The textbox is OneWay-bound and already displays the rejected text. + // Nothing reverts it automatically since Min/Max/Step/Value never changed. + // Re-raise the change notification for the canonical property so + // SliderViewModel re-reads the last valid value and the stale, invalid + // text the user typed is visibly replaced. + RaisePropertyChanged(ToCanonicalPropertyName(name)); + return false; + } + + if (isOutOfRange) + { + Info(Resources.IntegerSliderInfoMessage, true); + } + else + { + ClearInfoMessages(); + } + + switch (name) + { + case nameof(Min): + case "MinText": + Min = parsed; + return true; + case nameof(Max): + case "MaxText": + Max = parsed; + return true; + case nameof(Step): + case "StepText": + Step = parsed; + return true; + case nameof(Value): + case "ValueText": + Value = parsed; + return true; + } + break; } return base.UpdateValueCore(updateValueParams); } - private void UpdateNodeInfo(string value) + /// + /// Attempts to parse a strict 64-bit integer literal. Returns false only when + /// is not an integer literal at all (contains a decimal point, + /// letters, etc.). If is a well-formed integer (optional sign + /// followed only by digits) that exceeds the Int64 range, + /// is set to true and is clamped to Int64.MaxValue/MinValue, + /// matching how the slider already clamps values dragged or set past Min/Max. + /// + private static bool TryParseInt64(string value, out long result, out bool isOutOfRange) { - if (IsValueInt64(value)) + isOutOfRange = false; + + if (long.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out result)) + return true; + + var start = value.Length > 0 && (value[0] == '-' || value[0] == '+') ? 1 : 0; + if (start < value.Length && value.Skip(start).All(char.IsDigit)) { - ClearInfoMessages(); + isOutOfRange = true; + result = value[0] == '-' ? long.MinValue : long.MaxValue; + return true; } - else + + result = 0; + return false; + } + + /// + /// Maps a text-box property name (e.g. "MinText") to the canonical model property + /// name (e.g. "Min") that listens for when deciding + /// which bound text (MinText/MaxText/StepText/ValueText) to refresh. + /// + private static string ToCanonicalPropertyName(string name) + { + switch (name) { - Info(Resources.IntegerSliderInfoMessage, true); + case nameof(Min): + case "MinText": + return "Min"; + case nameof(Max): + case "MaxText": + return "Max"; + case nameof(Step): + case "StepText": + return "Step"; + default: + return "Value"; } } diff --git a/src/Libraries/CoreNodeModels/Input/SliderBase.cs b/src/Libraries/CoreNodeModels/Input/SliderBase.cs index 9ffac94039c..f9c0feae0d8 100644 --- a/src/Libraries/CoreNodeModels/Input/SliderBase.cs +++ b/src/Libraries/CoreNodeModels/Input/SliderBase.cs @@ -160,23 +160,5 @@ protected static long ConvertStringToInt64(string value) } return result; } - - /// - /// check if the value is within int64 range - /// - /// - /// - protected static bool IsValueInt64(string value) - { - try - { - var result = Convert.ToInt64(value); - return true; - } - catch (OverflowException) - { - return false; - } - } } } \ No newline at end of file diff --git a/src/Libraries/CoreNodeModels/Properties/Resources.Designer.cs b/src/Libraries/CoreNodeModels/Properties/Resources.Designer.cs index 33037e201de..eb6326e0e01 100644 --- a/src/Libraries/CoreNodeModels/Properties/Resources.Designer.cs +++ b/src/Libraries/CoreNodeModels/Properties/Resources.Designer.cs @@ -987,6 +987,15 @@ public static string IntegerSliderNodeDescription { } } + /// + /// Looks up a localized string similar to The input must be an integer.. + /// + public static string IntegerSliderNonIntegerInputMessage { + get { + return ResourceManager.GetString("IntegerSliderNonIntegerInputMessage", resourceCulture); + } + } + /// /// Looks up a localized string similar to integerslider;. /// diff --git a/src/Libraries/CoreNodeModels/Properties/Resources.en-US.resx b/src/Libraries/CoreNodeModels/Properties/Resources.en-US.resx index c219535bbc0..40cec87517b 100644 --- a/src/Libraries/CoreNodeModels/Properties/Resources.en-US.resx +++ b/src/Libraries/CoreNodeModels/Properties/Resources.en-US.resx @@ -754,4 +754,7 @@ double[] When mapping numbers along the curve, some Y values fall outside the specified Y-value domain range. + + The input must be an integer. + \ No newline at end of file diff --git a/src/Libraries/CoreNodeModels/Properties/Resources.resx b/src/Libraries/CoreNodeModels/Properties/Resources.resx index 2dd8afe3279..82ce7b572e0 100644 --- a/src/Libraries/CoreNodeModels/Properties/Resources.resx +++ b/src/Libraries/CoreNodeModels/Properties/Resources.resx @@ -754,4 +754,7 @@ double[] When mapping numbers along the curve, some Y values fall outside the specified Y-value domain range. + + The input must be an integer. + \ No newline at end of file diff --git a/src/Libraries/CoreNodeModelsWpf/Controls/DynamoSlider.xaml.cs b/src/Libraries/CoreNodeModelsWpf/Controls/DynamoSlider.xaml.cs index b363143f9dc..2f5e9657dd7 100644 --- a/src/Libraries/CoreNodeModelsWpf/Controls/DynamoSlider.xaml.cs +++ b/src/Libraries/CoreNodeModelsWpf/Controls/DynamoSlider.xaml.cs @@ -6,6 +6,10 @@ using Dynamo.Graph.Workspaces; using Dynamo.UI; using Dynamo.ViewModels; +using System; +using System.Text.RegularExpressions; +using System.Windows; +using CoreNodeModels.Input; namespace CoreNodeModelsWpf.Controls { @@ -29,6 +33,58 @@ public DynamoSlider(NodeModel model, IViewModelView nodeUI) nodeUI.ViewModel.DynamoViewModel.OnRequestReturnFocusToView(); }; + // DynamoSlider is shared with DoubleSlider, which still needs the decimal point, + // so the integer-only keystroke/paste filter is only applied for integer sliders. + if (nodeModel is IntegerSlider64Bit) + { + RestrictToIntegerInput(MinTb); + RestrictToIntegerInput(MaxTb); + RestrictToIntegerInput(StepTb); + RestrictToIntegerInput(ValTb); + } + } + + private static readonly Regex IntegerInputPattern = new Regex(@"^-?\d*$", RegexOptions.Compiled, TimeSpan.FromMilliseconds(100)); + + private static void RestrictToIntegerInput(TextBox textBox) + { + textBox.PreviewTextInput += IntegerTextBox_PreviewTextInput; + DataObject.AddPastingHandler(textBox, IntegerTextBox_Pasting); + } + + private static void IntegerTextBox_PreviewTextInput(object sender, TextCompositionEventArgs e) + { + var textBox = sender as TextBox; + if (textBox == null) return; + + e.Handled = !IsValidIntegerText(GetProposedText(textBox, e.Text)); + } + + private static void IntegerTextBox_Pasting(object sender, DataObjectPastingEventArgs e) + { + var textBox = sender as TextBox; + if (textBox == null || !e.DataObject.GetDataPresent(typeof(string))) + { + e.CancelCommand(); + return; + } + + var pastedText = (string)e.DataObject.GetData(typeof(string)); + if (!IsValidIntegerText(GetProposedText(textBox, pastedText))) + { + e.CancelCommand(); + } + } + + private static string GetProposedText(TextBox textBox, string newText) + { + var text = textBox.Text.Remove(textBox.SelectionStart, textBox.SelectionLength); + return text.Insert(textBox.SelectionStart, newText); + } + + private static bool IsValidIntegerText(string text) + { + return IntegerInputPattern.IsMatch(text); } #region Event Handlers diff --git a/test/DynamoCoreWpf3Tests/SliderTests.cs b/test/DynamoCoreWpf3Tests/SliderTests.cs index 4f17becffae..14d62caeac9 100644 --- a/test/DynamoCoreWpf3Tests/SliderTests.cs +++ b/test/DynamoCoreWpf3Tests/SliderTests.cs @@ -3,6 +3,7 @@ using System.Reflection; using System.Xml; using CoreNodeModels.Input; +using CoreNodeModels.Properties; using Dynamo.Graph; using Dynamo.Models; using NUnit.Framework; @@ -149,5 +150,79 @@ public void DeserializeCoreTest() slider.Deserialize(xmlElement, SaveContext.None); Assert.AreEqual(10, slider.Min); } + + [Test] + public void WhenValueTextIsDecimalThenInputIsRejectedAndValueUnchanged() + { + var slider = new IntegerSlider64Bit(); + Assert.NotNull(slider); + + var handled = slider.UpdateValue(new UpdateValueParams("ValueText", "3.5")); + + Assert.IsFalse(handled); + Assert.AreEqual(1, slider.Value); + Assert.AreEqual(1, slider.Infos.Count); + Assert.IsTrue(slider.Infos.Any(i => i.Message.Equals(Resources.IntegerSliderNonIntegerInputMessage))); + } + + [Test] + public void WhenValueTextIsNonNumericThenInputIsRejectedAndValueUnchanged() + { + var slider = new IntegerSlider64Bit(); + + var handled = slider.UpdateValue(new UpdateValueParams("ValueText", "abc")); + + Assert.IsFalse(handled); + Assert.AreEqual(1, slider.Value); + Assert.IsTrue(slider.Infos.Any(i => i.Message.Equals(Resources.IntegerSliderNonIntegerInputMessage))); + } + + [Test] + public void WhenValidIntegerFollowsRejectedInputThenInfoIsCleared() + { + var slider = new IntegerSlider64Bit(); + slider.UpdateValue(new UpdateValueParams("ValueText", "3.5")); + Assert.AreEqual(1, slider.Infos.Count); + + var handled = slider.UpdateValue(new UpdateValueParams("ValueText", "42")); + + Assert.IsTrue(handled); + Assert.AreEqual(42, slider.Value); + Assert.AreEqual(0, slider.Infos.Count); + } + + [Test] + public void WhenMinTextIsDecimalThenMinIsUnchanged() + { + var slider = new IntegerSlider64Bit(); + + var handled = slider.UpdateValue(new UpdateValueParams("MinText", "0.5")); + + Assert.IsFalse(handled); + Assert.AreEqual(0, slider.Min); + } + + [Test] + public void WhenMaxIsDecimalThenMaxIsUnchanged() + { + // "Max" (no "Text" suffix) is the property name sent by IntegerSliderSettingsControl. + var slider = new IntegerSlider64Bit(); + + var handled = slider.UpdateValue(new UpdateValueParams("Max", "100.5")); + + Assert.IsFalse(handled); + Assert.AreEqual(100, slider.Max); + } + + [Test] + public void WhenStepTextIsNonNumericThenStepIsUnchanged() + { + var slider = new IntegerSlider64Bit(); + + var handled = slider.UpdateValue(new UpdateValueParams("StepText", "one")); + + Assert.IsFalse(handled); + Assert.AreEqual(1, slider.Step); + } } }