Make TextFields.setupClearButtonField public

#693 Open

Bitbucket cannot automatically merge this request.

The commits that make up this pull request have been removed.

Bitbucket cannot automatically merge this request due to conflicts.

Review the conflicts on the Overview tab. You can then either decline the request or merge it manually on your local system using the following commands:

hg update default
hg pull -r tobiasdiez/make-textfieldssetupclearbuttonfield-pub-1518086685624
hg merge tobiasdiez/make-textfieldssetupclearbuttonfield-pub-1518086685624
hg commit -m 'Merged in tobiasdiez/controlsfx-3/tobiasdiez/make-textfieldssetupclearbuttonfield-pub-1518086685624 (pull request #693)'
  1. tobiasdiez

Make TextFields.setupClearButtonField public as requested in

Comments (7)

    1. Abhinay Agarwal

      Hi @tobiasdiez ,

      Thank you for taking time for working on this issue. I am not very well versed with SceneBuilder, so I cannot comment on how to improve its support.

      As, from the API point of view, can you share you views (if possible, with use-case ) on how this change is helpful?

      Also, the second parameter in the API is redundant 😉

      1. tobiasdiez author

        Sure! A public setupClearButtonFieldhas mainly two possible applications:

        1. You have a CustomTextField control defined in the FXML file and would like to add a clear button to it.
        2. You want to add a clear button to a control that derives from CustomTextField.

        Right now, both things are not easily possible since the only public API creates a text field instead of allowing to modify an existing one.

        With second parameter you mean the rightProperty? This one is used in the method (line 105)

        1. Abhinay Agarwal

          OK, makes sense.

          I am not sure if we can create a clearableTextField by passing an instance of TextField and an ObjectProperty<Node> to setupClearButtonField(). The method needs an instance of CustomTextField. What are your thoughts about it?

          1. tobiasdiez author

            I only changed the visibility of the method and didn't change its semantics/function. The other methods in the class show how you could setup a clearable text field. For example,

                     CustomPasswordField inputField = new CustomPasswordField();
                     setupClearButtonField(inputField, inputField.rightProperty());
            1. Abhinay Agarwal

              Whenever a method is made public or a new public method is added, it is not easy to change it later. Therefore, we should put some thought while making such changes.

              When I look at public static void setupClearButtonField(TextField inputField, ObjectProperty<Node> rightProperty), it doesn't feel right. From the definition, the immediate thought that comes to me is: If I pass a TextField and an ObjectProperty<Node>, the node will automatically appear inside the TextField and clear the TextField, when pressed. I am sceptical if this is what happens.

              A better definition for the method in such situations would be:

              public static void setupClearButtonField(CustomTextField inputField)

              Even better, leave this method private to avoid confusion and create a new createClearableTextField which accepts CustomTextField:

              public static TextField createClearableTextField(CustomTextField customTextField) {

              Again, this is just my opinion. It doesn't mean, this is the right path 🙂