dooApp/FXForm2

Skinning: binding via ID stops working when using a ScrollPane

Closed this issue · 15 comments

oova commented

Hi,

I took the sample code for skinning a form using an FXML file from
https://github.com/dooApp/FXForm2/blob/master/samples/src/main/java/com/dooapp/fxform/samples/FXMLSkin.java

Everything worked as expected, the bean properties were shown in the form.
Then I wrapped the AnchorPane's children in a ScrollPane like this:

<AnchorPane id="AnchorPane" maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="400.0" prefWidth="600.0" xmlns:fx="http://javafx.com/fxml/1" xmlns="http://javafx.com/javafx/2.2">
  <children>
    <ScrollPane>
      <content>
        <Label id="firstName-form-label" layoutX="272.0" layoutY="14.0" text="Label" />
        <TextField id="firstName-form-editor" layoutX="199.0" layoutY="36.0" prefWidth="200.0" />
        <Label id="lastName-form-label" layoutX="14.0" layoutY="184.0" text="Label" />
        <TextArea id="lastName-form-editor" layoutX="79.0" layoutY="115.0" prefWidth="200.0" wrapText="true" />
        <Label id="age-form-label" layoutX="334.0" layoutY="326.0" text="Label" />
        <TextField id="age-form-editor" layoutX="362.0" layoutY="342.0" prefWidth="200.0" />
      </content>
    </ScrollPane>
  </children>
</AnchorPane>

This resulted in an empty form being shown. There seemed to be no bindings between bean properties and GUI components.

Did I make a mistake or is ScrollPane not supported at the moment?

Keep up the great work!
oova

This is an issue with your FXML. The content of your ScrollPane is a list of nodes where it should contains a single content node. You might want to use a container there, which will then contain your form elements like in the example below.

Note : I recommend you to use the SceneBuilder to check or build your FXML interface !

<AnchorPane maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="400.0" prefWidth="600.0" xmlns="http://javafx.com/javafx/8.0.111" xmlns:fx="http://javafx.com/fxml/1">
   <children>
      <ScrollPane layoutX="188.0" layoutY="94.0" prefHeight="200.0" prefWidth="200.0" AnchorPane.bottomAnchor="0.0" AnchorPane.leftAnchor="0.0" AnchorPane.rightAnchor="0.0" AnchorPane.topAnchor="0.0">
         <content>
            <VBox prefHeight="200.0" prefWidth="100.0">
               <children>
                  <Label text="Label" />
                  <Label text="Label" />
               </children>
            </VBox>
         </content>
      </ScrollPane>
   </children>
</AnchorPane>
oova commented

Sorry, I pasted the wrong FXML code. Here's the correct version:

<AnchorPane id="AnchorPane" maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity"
            prefHeight="400.0" prefWidth="600.0" xmlns:fx="http://javafx.com/fxml/1"
            xmlns="http://javafx.com/javafx/2.2">
    <children>
        <ScrollPane>
            <content>
                <VBox>
                    <children>
                        <Label id="firstName-form-label" layoutX="272.0" layoutY="14.0" text="Label"/>
                        <TextField id="firstName-form-editor" layoutX="199.0" layoutY="36.0" prefWidth="200.0"/>
                        <Label id="lastName-form-label" layoutX="14.0" layoutY="184.0" text="Label"/>
                        <TextArea id="lastName-form-editor" layoutX="79.0" layoutY="115.0" prefWidth="200.0"
                                  wrapText="true"/>
                        <Label id="age-form-label" layoutX="334.0" layoutY="326.0" text="Label"/>
                        <TextField id="age-form-editor" layoutX="362.0" layoutY="342.0" prefWidth="200.0"/>
                    </children>
                </VBox>
            </content>
        </ScrollPane>
    </children>
</AnchorPane>

It works if you comment out the ScrollPane node, but it stops working if the ScrollPane is included.

The issue is still related to your FXML which is invalid. Please check with the Scene Builder as suggested, you will see that it's not able to load it.

Once our FXML is valid, the form bindings should work correctly, there is nothing specific to do to handle the ScrollPane.

oova commented

Sorry again for posting an invalid FXML file. The issue does not seem to be related to my lack of copy and paste talents, though.

Here is my full (and valid) ScrollPaneForm.fxml file:

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.scene.layout.AnchorPane?>
<?import javafx.scene.control.ScrollPane?>
<?import javafx.scene.layout.VBox?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.TextField?>
<?import javafx.scene.control.TextArea?>
<AnchorPane id="AnchorPane" maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity"
            prefHeight="400.0" prefWidth="600.0" xmlns="http://javafx.com/javafx/8.0.172">
    <children>
        <ScrollPane>
            <content>
                <VBox>
                    <children>
                        <Label id="firstName-form-label" layoutX="272.0" layoutY="14.0" text="Label"/>
                        <TextField id="firstName-form-editor" layoutX="199.0" layoutY="36.0" prefWidth="200.0"/>
                        <Label id="lastName-form-label" layoutX="14.0" layoutY="184.0" text="Label"/>
                        <TextArea id="lastName-form-editor" layoutX="79.0" layoutY="115.0" prefWidth="200.0"
                                  wrapText="true"/>
                        <Label id="age-form-label" layoutX="334.0" layoutY="326.0" text="Label"/>
                        <TextField id="age-form-editor" layoutX="362.0" layoutY="342.0" prefWidth="200.0"/>
                    </children>
                </VBox>
            </content>
        </ScrollPane>
    </children>
</AnchorPane>

And here is the Java code for showing the form:

import java.net.URL;

import com.dooapp.fxform.FXForm;
import com.dooapp.fxform.annotation.Accessor;
import com.dooapp.fxform.annotation.FormFactory;
import com.dooapp.fxform.view.factory.impl.TextAreaFactory;
import com.dooapp.fxform.view.skin.FXMLSkin;

import javafx.application.Application;
import javafx.beans.property.IntegerProperty;
import javafx.beans.property.SimpleIntegerProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.scene.Scene;
import javafx.scene.layout.Pane;
import javafx.stage.Stage;

public class ScrollPaneForm extends Application {

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) {
        Pane root = new Pane();
        FXForm<User> form = new FXForm<>();
        User user = new User();

        URL fxmlUrl = this.getClass().getResource("ScrollPaneForm.fxml");
        form.setSkin(new FXMLSkin(form, fxmlUrl));

        form.setSource(user);
        root.getChildren().add(form);

        Scene scene = new Scene(root);
        primaryStage.setScene(scene);
        primaryStage.show();
    }

    @Accessor(value = Accessor.AccessType.FIELD)
    public class User {

        public StringProperty firstName = new SimpleStringProperty("Jon");

        @FormFactory(TextAreaFactory.class) //The field will use a TextArea instead of the default TextField
        public StringProperty lastName = new SimpleStringProperty("Smith");

        public IntegerProperty age = new SimpleIntegerProperty(10);

    }
}

When I launch this application, the bindings do not work, i.e., I do not see any values in the text fields or the text area.

When I use the following FXML which does not wrap the VBox in a ScrollPane, everything works as expected:

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.scene.control.Label?>
<?import javafx.scene.control.TextArea?>
<?import javafx.scene.control.TextField?>
<?import javafx.scene.layout.AnchorPane?>
<?import javafx.scene.layout.VBox?>
<AnchorPane id="AnchorPane" maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity"
            prefHeight="400.0" prefWidth="600.0" xmlns="http://javafx.com/javafx/8.0.172">
    <children>
        <!--<ScrollPane>-->
        <!--<content>-->
        <VBox>
            <children>
                <Label id="firstName-form-label" layoutX="272.0" layoutY="14.0" text="Label"/>
                <TextField id="firstName-form-editor" layoutX="199.0" layoutY="36.0" prefWidth="200.0"/>
                <Label id="lastName-form-label" layoutX="14.0" layoutY="184.0" text="Label"/>
                <TextArea id="lastName-form-editor" layoutX="79.0" layoutY="115.0" prefWidth="200.0"
                          wrapText="true"/>
                <Label id="age-form-label" layoutX="334.0" layoutY="326.0" text="Label"/>
                <TextField id="age-form-editor" layoutX="362.0" layoutY="342.0" prefWidth="200.0"/>
            </children>
        </VBox>
        <!--</content>-->
        <!--</ScrollPane>-->
    </children>
</AnchorPane>

I hope you can reproduce my problem now.

Thank you for your support.

oova commented

The issue I'm seeing seems to be caused by JavaFX's behaviour when performing a lookup on a scene graph that contains a ScrollPane or TitledPane. This is explained in more detail in this SO question.

In my case, the lookup is performed in the private FXFormNode lookupNode(Element element, String suffix) method in FXForm's NodeSkin class. This lookup returns null if my FXML includes the ScrollPane.

The solution recommended on SO is to wrap the code performing the lookup in a Platform.runLater call. I might give this a try.

@amischler Do you see any other potential solutions which might work with the existing FXForm code?

oova commented

A workaround for the sample application above is to set the source on the form after calling primaryStage.show(). At this point, the CSS has been applied such that the lookup finds the scroll pane content nodes .

Interesting, thank you for investigating on this.

According to this other conversation on SO, it looks like looking up nodes throught their ids is rather not recommended, since we can not predict when an id will be attributed to the node.

As a fix, I pushed a commit that allow to lookup nodes in the FXMLSkin through the FXMLLoader namespace (as suggested in the conversation) if the id lookup fails. This way we can retrieve the nodes throught their fx:id. This seems a bit more robust than the id lookup since we are not dependent of the rendering/css. I will update the documentation according to this.

Deployed in 8.3.0-SNAPSHOT

oova commented

Thanks a lot! I tested your change with my application successfully. The bindings work if I follow the naming convention for the fx:id properties you used in your example.

Do we still need the id properties when we are using FXML? In your issue170.fxml, it looks like one has to follow two naming conventions: one for the id properties and another one for the fx:id properties. This might be a source of confusion.

oova commented

One more thing: when I use an FXMLSkin and provide my custom controller class, I would like to add nodes to the FXML scene graph dynamically in the controller. These nodes need to be bound to model properties as well, but since they are not known to the FXML namespace, the lookup will fail.

I could use a NodeSkin instead of an FXMLSkin but the NodeSkin class uses the old lookup method which fails to recognise children of ScrollPane if CSS has not been applied.

What to do in this case?

With the new implementation you can use both naming conventions. The FXMLSkin will first try to lookup nodes using their id and if it fails it will fallback to the fx:id. But you are right, this might be a source of confusion. I have removed the id from issue170.fxml.

I'm not sure that I totally understand your question regarding the dynamic addition of nodes to the FXML scene graph. Could you please provide a small example ?

oova commented

Here's an example of a use case where the binding does not work as expected.

The FXML looks like this:

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.scene.control.Label?>
<?import javafx.scene.control.ScrollPane?>
<?import javafx.scene.control.TextField?>
<?import javafx.scene.layout.StackPane?>
<?import javafx.scene.layout.VBox?>
<StackPane xmlns:fx="http://javafx.com/fxml/1" xmlns="http://javafx.com/javafx/8.0.172">
    <children>
        <ScrollPane>
            <content>
                <VBox fx:id="container">
                    <children>
                        <Label fx:id="firstNameFormLabel"/>
                        <TextField fx:id="firstNameFormEditor" prefWidth="200.0"/>
                    </children>
                </VBox>
            </content>
        </ScrollPane>
    </children>
</StackPane>

The controller class looks like this:

import java.net.URL;
import java.util.ResourceBundle;

import javafx.fxml.FXML;
import javafx.fxml.Initializable;
import javafx.scene.Node;
import javafx.scene.control.Label;
import javafx.scene.control.TextField;
import javafx.scene.layout.VBox;

public class ExtensibleScrollPaneController implements Initializable {

    @FXML
    VBox container;
    @FXML
    Label firstNameFormLabel;
    @FXML
    TextField firstNameFormEditor;

    @Override
    public void initialize(URL location, ResourceBundle resources) {
    }

    public void add(Node node) {
        container.getChildren().add(node);
    }
}

And the JavaFX application showing the form (using an FXMLSkin) looks like this:

import java.net.URL;

import com.dooapp.fxform.FXForm;
import com.dooapp.fxform.annotation.Accessor;
import com.dooapp.fxform.reflection.MultipleBeanSource;
import com.dooapp.fxform.view.skin.FXMLSkin;

import javafx.application.Application;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.scene.Scene;
import javafx.scene.control.TextField;
import javafx.scene.layout.Pane;
import javafx.stage.Stage;

public class ExtensibleScrollPaneWithFxmlSkinForm extends Application {

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) {
        Pane root = new Pane();
        FXForm<MultipleBeanSource> form = new FXForm<>();

        URL fxmlUrl = this.getClass().getResource("ExtensibleScrollPaneForm.fxml");
        ExtensibleScrollPaneController controller = new ExtensibleScrollPaneController();
        form.setSkin(new FXMLSkin(form, fxmlUrl, controller));

        controller.add(createDescriptionTextField());

        User user = new User();
        Job job = new Job();
        form.setSource(new MultipleBeanSource(user, job));

        root.getChildren().add(form);
        Scene scene = new Scene(root);
        primaryStage.setScene(scene);
        primaryStage.show();
    }

    TextField createDescriptionTextField() {
        TextField descriptionTextField = new TextField();
        descriptionTextField.setId("descriptionFormEditor");
        return descriptionTextField;
    }

    @Accessor(value = Accessor.AccessType.FIELD)
    public class User {
        public StringProperty firstName = new SimpleStringProperty(this, "firstName", "Jon");
    }

    @Accessor(value = Accessor.AccessType.FIELD)
    public class Job {
        public StringProperty description = new SimpleStringProperty(this, "description", "Developer");
    }
}

When I run this application, the text field for the description property is empty. The binding to the bean property does not seem to work.

As another approach, I tried to use a NodeSkin with nodes loaded manually from FXML. In this case, the FXML looks like this:

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.scene.control.Label?>
<?import javafx.scene.control.ScrollPane?>
<?import javafx.scene.control.TextField?>
<?import javafx.scene.layout.StackPane?>
<?import javafx.scene.layout.VBox?>
<StackPane xmlns:fx="http://javafx.com/fxml/1" xmlns="http://javafx.com/javafx/8.0.172"
           fx:controller="com.zeiss.garching.spikes.client.inputvalidation.samples.ExtensibleScrollPaneController">
    <children>
        <ScrollPane>
            <content>
                <VBox fx:id="container">
                    <children>
                        <Label fx:id="firstNameFormLabel" id="firstName-form-label"/>
                        <TextField fx:id="firstNameFormEditor" id="firstName-form-editor" prefWidth="200.0"/>
                    </children>
                </VBox>
            </content>
        </ScrollPane>
    </children>
</StackPane>

Unfortunately, I think it is necessary to specify both the id and the fx:id properties here.

The controller class is the same as above.

The application looks like this:

import javafx.application.Application;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.fxml.FXMLLoader;
import javafx.scene.Scene;
import javafx.scene.control.TextField;
import javafx.scene.layout.Pane;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;

public class ExtensibleScrollPaneWithNodeSkinForm extends Application {

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage primaryStage) throws IOException {
        Pane root = new Pane();
        FXForm<MultipleBeanSource> form = new FXForm<>();

        URL fxmlUrl = this.getClass().getResource("ExtensibleScrollPaneFormWithController.fxml");
        FXMLLoader fxmlLoader = new FXMLLoader(fxmlUrl);
        StackPane pane = fxmlLoader.load();
        form.setSkin(new NodeSkin(form, () -> pane));

        ExtensibleScrollPaneController controller = fxmlLoader.getController();
        controller.add(createDescriptionTextField());

        User user = new User();
        Job job = new Job();
        form.setSource(new MultipleBeanSource(user, job));

        root.getChildren().add(form);
        Scene scene = new Scene(root);
        primaryStage.setScene(scene);
        primaryStage.show();

    }

    TextField createDescriptionTextField() {
        TextField descriptionTextField = new TextField();
        descriptionTextField.setId("description-form-editor");
        return descriptionTextField;
    }

    @Accessor(value = Accessor.AccessType.FIELD)
    public class User {

        public StringProperty firstName = new SimpleStringProperty(this, "firstName", "Jon");
    }

    @Accessor(value = Accessor.AccessType.FIELD)
    public class Job {
        public StringProperty description = new SimpleStringProperty(this, "description", "Developer");
    }
}

When I run this application, no bindings work since the NodeSkin class uses the "old" lookupNode() method that has an issue with scroll panes. It works fine if I use this FXML without the scroll pane:

<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.scene.control.Label?>
<?import javafx.scene.control.ScrollPane?>
<?import javafx.scene.control.TextField?>
<?import javafx.scene.layout.StackPane?>
<?import javafx.scene.layout.VBox?>
<StackPane xmlns:fx="http://javafx.com/fxml/1" xmlns="http://javafx.com/javafx/8.0.172"
           fx:controller="com.zeiss.garching.spikes.client.inputvalidation.samples.ExtensibleScrollPaneController">
    <children>
        <!--<ScrollPane>-->
            <!--<content>-->
                <VBox fx:id="container">
                    <children>
                        <Label fx:id="firstNameFormLabel" id="firstName-form-label"/>
                        <TextField fx:id="firstNameFormEditor" id="firstName-form-editor" prefWidth="200.0"/>
                    </children>
                </VBox>
            <!--</content>-->
        <!--</ScrollPane>-->
    </children>
</StackPane>

So neither approach is really satisfying. Or maybe I'm doing something wrong here. My use case may be a bit unusual, but it would be great if FXForm could support it without the need to use the workaround mentioned in #170 (comment).

Again, thank you for your support. I really appreciate it.

Ok I understand better. This case is not supported yet. It might be possible to update the FXMLSkin or NodeSkin to listen to their node graph and bind additionnal nodes dynamically, but it requires some more work.
Also, why don't you put the text field node in the original FXML and hide it until you need it ? You could show it dynamically later instead of adding it to get the same effect. Would it fit your use case ?

oova commented

Your suggestion would not be feasible for my use case, unfortunately, as the type of control to be added is not known in advance, but rather determined at runtime (using the ServiceLoader mechanism).

It is a small annoyance for me but my workaround is simple and seems to do the job pretty well. I can fully understand that this is an uncommon use case and supporting it might require an unjustifiable amount of work at this point.

I appreciate your time and efforts for looking into this. Thanks!