Make TextFields.setupClearButtonField public

#693 Declined
Repository
ControlsFX
Branch
default
Author
  1. tobiasdiez
Reviewers
Description

Make TextFields.setupClearButtonField public as requested in https://bitbucket.org/controlsfx/controlsfx/issues/330.

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 🙂