jidefx-oss icon indicating copy to clipboard operation
jidefx-oss copied to clipboard

Let DecoratorPane be used with FXML

Open aalmiray opened this issue 10 years ago • 7 comments

The current design of DecoratorPane assumes that setting the content is an immutable operation, this it forces the content to be set on the constructor. However DecoratorPane may have its children rearranged at any time, thus the immutable condition is rendered moot. Providing an explicit no-args constructor hinders the usage of DecoratorPane with FXML.

Given that immutability of content cannot be enforced I say it's best to remove the explicit constructor and handle registration/deregistration of listeners whenever child content occurs. It could be the case that DecoratorPane enforces a single child in in it lists of children.

aalmiray avatar Mar 20 '15 11:03 aalmiray

You may take a look and see if it is possible to make it FXML friendly. I thought about it too but I felt it is not that critical. I can still design the panel using FXML. When I add the FXML designed panel to its parent using code, I can then wrap it in a DecorationPane. I know it is not perfect but probably enough to cover most cases.

jidesoft avatar Mar 20 '15 15:03 jidesoft

Indeed. The design of DecorationPanel feels that it was made with programmatic usage in mind. And though I like direct API usages I've come to appreciate the easiness of FXML to quickly setup an UI. Take the following FXML for example

<?xml version="1.0" encoding="UTF-8"?>
<?import griffon.javafx.scene.layout.MigLayoutPane?>
<?import griffon.javafx.support.fontawesome.FontAwesomeIcon?>
<?import javafx.scene.control.ListView?>
<?import javafx.scene.control.TitledPane?>
<?import org.example.DecorationPane?>
<MigLayoutPane maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity"
               xmlns="http://javafx.com/javafx/8" xmlns:fx="http://javafx.com/fxml/1"
               layout="fill" cols="[][grow 2][]"
               fx:controller="org.example.SampleController">
    <TitledPane collapsible="false" text="People">
        <graphic>
            <FontAwesomeIcon fontAwesome="fa_users"/>
        </graphic>
        <ListView fx:id="peopleView" prefWidth="200.0" prefHeight="190.0"/>
    </TitledPane>
    <TitledPane collapsible="false" text="Details">
        <graphic>
            <FontAwesomeIcon fontAwesome="fa_user"/>
        </graphic>
        <DecorationPane>
            <MigLayoutPane fx:id="formPanel" layout="fill" cols="[::80][]" prefHeight="190.0"/>
        </DecorationPane>
    </TitledPane>
    <TitledPane collapsible="false" text="Actions">
        <graphic>
            <FontAwesomeIcon fontAwesome="fa_star"/>
        </graphic>
        <MigLayoutPane fx:id="actionsPanel" layout="fill" prefHeight="190.0"/>
    </TitledPane>
</MigLayoutPane>

The two innermost MigLayoutPane instances will have their children set using the Java API as their content is dynamic. The DecorationPane used here simply removes the default no-arg constructor to make a quick test. If the constructor is left in place then the class doing the FXML loading has to have references to the TitledPane instances, create the MigLayoutPanes and wrap them with DecoratorPane. Not much of a change given that the loading class performs direct Java API usage anyway. But what if the contents of the MigLayoutPanes were static? Then everything could be defined in the FXML.

aalmiray avatar Mar 20 '15 15:03 aalmiray

I see. What exactly need to be done? A default constructor and being able to set the content on fly?

jidesoft avatar Mar 20 '15 15:03 jidesoft

Yes. The current constructor defines lifecycle methods that must be called when content is added. I believe the following measures are needed to make DecorationPane be FXML friendly:

  • remove explicit constructor
  • add setContent() method that applies content Node and runs the lifecycle. This method must make sure previous content is disposed gracefully -> additional lifecycle methods are needed to deregister listeners on old content.
  • override getChildren() to force a single element on the list. This is to prevent someone from calling decorationPane.getChildren.add(someRandomControl). If this is too troublesome then an entry on the javadoc should be added specifying this pane assumes a single child element and makes no guarantees if additional children are added.

aalmiray avatar Mar 20 '15 15:03 aalmiray

Thanks for the explanation. It'd be possible although needs some work. Let me see if I can find time for it next week.

jidesoft avatar Mar 20 '15 20:03 jidesoft

Actually the current constructor must stay in place, otherwise it breaks compatibility. The no-args constructor must be added; then rhe content wiring/unwiring. Next week there is a 2 day @hackergarten at Javaland. I hope to get some people interested in jidefx and improvs :smile:

aalmiray avatar Mar 20 '15 20:03 aalmiray

Cool!

jidesoft avatar Mar 21 '15 02:03 jidesoft