JFXtras / jfxtras

A supporting library for JavaFX, containing helper classes, extended layouts, controls and other interesting widgets.
http://jfxtras.org
Other
599 stars 123 forks source link

TextProperty of CalendarTextField is not updated #77

Closed Maxoudela closed 7 years ago

Maxoudela commented 7 years ago

The documentation on the textProperty is a bit light so we don't really know what's intended.

From what I understood, it should reflect the textProperty of the TextField underneath. Right now, this text is only updated when we parsed the TextField. So if you write a simple test like that :


@Test
    public void textFieldGetText() {
        Assert.assertTrue(find(".text-field").isFocused());

        // Type 0
        type(KeyCode.NUMPAD2);

        //We should have the same value everywhere.
        Assert.assertEquals(((TextField) find(".text-field")).getText(), calendarTextField.getText());
        Assert.assertEquals("2", calendarTextField.getText());
    }

It will currently fail. My suggestion is to simply bind these two property like that : textField.textProperty().bindBidirectional(getSkinnable().textProperty());

The only thing that bothers me is that there were a listener before of this TextProperty on Calendar side. When the user would modify this property, a call to "parse" would be done in the skin in order to verify the input. If we go towards this bidirectional binding, this cannot stay. Because as soon as the user will input something, the parse method will be triggered, the value will not be accepted, and the TextField will be erased (current behavior).

I've modified the code by removing that listener and applying the bidirectional binding and no test is failing. Let me know what you think of all that.

tbee commented 7 years ago

The text property should be identical. The bidirectional binding seems like a good solution. Please put it in.

tbee commented 7 years ago

Fixed in https://github.com/JFXtras/jfxtras/pull/78