Merge pull request #328 from ThomasJClark/spinner-bug

Fix the buggy spinner validation

Author: ThomasJClark

ui/src/main/java/edu/wpi/grip/ui/deployment/FRCDeploymentOptionsController.java

@@ -14,6 +14,7 @@
 
 import java.io.OutputStream;
 import java.net.InetAddress;
+import java.text.NumberFormat;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
@@ -31,7 +32,7 @@ FRCDeploymentOptionsController create(
                 Consumer<DeployedInstanceManager> onDeployCallback,
                 @Assisted("stdOut") Supplier<OutputStream> stdOut,
                 @Assisted("stdErr") Supplier<OutputStream> stdErr
-            );
+        );
     }
 
     @Inject
@@ -53,14 +54,13 @@ protected void postInit() {
         final SpinnerValueFactory.IntegerSpinnerValueFactory spinnerValueFactory =
                 new SpinnerValueFactory.IntegerSpinnerValueFactory(0, Integer.MAX_VALUE, teamNumber);
         this.teamNumberSpinner = new Spinner(spinnerValueFactory);
-        Spinners.makeEditableSafely(teamNumberSpinner, Integer::valueOf);
+        Spinners.makeEditableSafely(teamNumberSpinner, NumberFormat.getIntegerInstance(), 0);
         label.setLabelFor(teamNumberSpinner);
 
         getOptionsGrid().addRow(0, label, this.teamNumberSpinner);
     }
 
 
-
     @Override
     protected Promise<DeployedInstanceManager, String, String> onDeploy() {
         final Deferred<DeployedInstanceManager, String, String> deferredDeploy = new DeferredObject<>();

ui/src/main/java/edu/wpi/grip/ui/pipeline/input/NumberSpinnerInputSocketController.java

@@ -13,6 +13,8 @@
 import javafx.scene.control.Spinner;
 import javafx.scene.control.SpinnerValueFactory;
 
+import java.text.NumberFormat;
+
 import static com.google.common.base.Preconditions.checkArgument;
 
 /**
@@ -55,7 +57,8 @@ public interface Factory {
     public void initialize() {
         super.initialize();
         final Spinner<Double> spinner = new Spinner<>(this.valueFactory);
-        Spinners.makeEditableSafely(spinner, Double::valueOf);
+        Spinners.makeEditableSafely(spinner, NumberFormat.getNumberInstance(),
+                getSocket().getSocketHint().createInitialValue().get().doubleValue());
         spinner.disableProperty().bind(this.getHandle().connectedProperty());
 
         this.setContent(spinner);

ui/src/main/java/edu/wpi/grip/ui/util/Spinners.java

@@ -3,9 +3,13 @@
 
 import javafx.scene.control.Spinner;
 import javafx.scene.control.SpinnerValueFactory;
+import javafx.scene.control.TextField;
+import javafx.scene.control.TextFormatter;
 import javafx.util.StringConverter;
 
-import java.util.function.Function;
+import java.text.NumberFormat;
+import java.text.ParsePosition;
+import java.util.Optional;
 
 /**
  * Utility methods to set up spinners in a safe way.
@@ -19,43 +23,52 @@ private Spinners() {
     /**
      * Makes the spinner editable.
      * Ensures that the values will be committed when focus is lost.
-     * Ensures that the only values that the spinner can accept are parable into whatever number type they represent.
-     * @param <T> The number type of the spinner
+     *
+     * @param defaultValue The value to use if nothing is entered
+     * @param <T>          The number type of the spinner
      */
     @SuppressWarnings("PMD.IfElseStmtsMustUseBraces")
-    public static <T extends Number> void makeEditableSafely(Spinner<T> spinner, Function<String, T> parseToNumber) {
+    public static <T extends Number> void makeEditableSafely(Spinner<T> spinner, NumberFormat format, T defaultValue) {
         spinner.setEditable(true);
-        spinner.focusedProperty().addListener((s, ov, nv) -> {
-            if (nv) return;
-            commitEditorText(spinner);
+
+        TextField editor = spinner.getEditor();
+
+        // Override the converter used to interpret editor values.  If there's an error, we want to set the default
+        // value instead of throwing an exception.
+        StringConverter<T> oldConverter = spinner.getValueFactory().getConverter();
+        spinner.getValueFactory().setConverter(new StringConverter<T>() {
+            @Override
+            public String toString(T t) {
+                return oldConverter.toString(t);
+            }
+
+            @Override
+            public T fromString(String s) {
+                return Optional.ofNullable(oldConverter.fromString(s)).orElse(defaultValue);
+            }
         });
-        // Ensure the value entered is only a number
-        spinner.getEditor().textProperty().addListener((observable, oldValue, newValue) -> {
-            if ("".equals(newValue)) {
-                spinner.getEditor().setText("0");
-            } else try {
-                T value = parseToNumber.apply(newValue);
-                spinner.getEditor().setText(value.toString());
-            } catch (NumberFormatException e) {
-                spinner.getEditor().setText(oldValue);
+
+        // Commit the spinner value when the editor loses focus
+        editor.focusedProperty().addListener((s, ov, focused) -> {
+            if (!focused) {
+                SpinnerValueFactory<T> valueFactory = spinner.getValueFactory();
+                StringConverter<T> converter = valueFactory.getConverter();
+                valueFactory.setValue(converter.fromString(editor.getText()));
             }
         });
-    }
 
-    /**
-     *
-     * @see <a href="http://stackoverflow.com/questions/32340476/manually-typing-in-text-in-javafx-spinner-is-not-updating-the-value-unless-user">Stack Overflow Post</a>
-     */
-    private static <T> void commitEditorText(Spinner<T> spinner) {
-        if (!spinner.isEditable()) return;
-        String text = spinner.getEditor().getText();
-        SpinnerValueFactory<T> valueFactory = spinner.getValueFactory();
-        if (valueFactory != null) {
-            StringConverter<T> converter = valueFactory.getConverter();
-            if (converter != null) {
-                T value = converter.fromString(text);
-                valueFactory.setValue(value);
+        // Filter out invalid editor changes
+        editor.setTextFormatter(new TextFormatter<T>(change -> {
+            if (change.isContentChange() && !change.getControlNewText().isEmpty()) {
+                ParsePosition position = new ParsePosition(0);
+                format.parse(change.getControlNewText(), position);
+
+                if (position.getIndex() == 0 || position.getIndex() < change.getControlNewText().length()) {
+                    return null;
+                }
             }
-        }
+
+            return change;
+        }));
     }
 }