Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let DecoratorPane be used with FXML #30

Open
aalmiray opened this issue Mar 20, 2015 · 7 comments
Open

Let DecoratorPane be used with FXML #30

aalmiray opened this issue Mar 20, 2015 · 7 comments

Comments

@aalmiray
Copy link
Contributor

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.

@jidesoft
Copy link
Owner

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.

@aalmiray
Copy link
Contributor Author

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.

@jidesoft
Copy link
Owner

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

@aalmiray
Copy link
Contributor Author

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.

@jidesoft
Copy link
Owner

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

@aalmiray
Copy link
Contributor Author

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 😄

@jidesoft
Copy link
Owner

Cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants