Update position of decoration when target changes size

Issue #791 new
tobiasdiez
created an issue

Consider a GraphicDecoration that is placed at the bottom of a text field. If the target node is increased in size, then the decoration still is placed at its original offsett relative to the text field (i.e. somewhere which is now in the middle) instead of at the bottom.

Before resizing: Untitled.png

After resizing: Untitled2.png

The problem is that size listener is only registered if the initial size was 0, see https://bitbucket.org/controlsfx/controlsfx/src/b85047499308377c28303e65834701abc8247269/controlsfx/src/main/java/org/controlsfx/control/decoration/GraphicDecoration.java?at=default&fileviewer=file-view-default#GraphicDecoration.java-150.

Comments (23)

  1. Rich Meyer

    Also, GraphicDecoration fails to size the decorationNode, which is its responsibility because it sets it to unmanaged. Many nodes will remain at size 0x0 if not explicitly resized. Specifically, GraphicDecoration should call decorationNode.autosize(), which will set the correct size based on min/pref/max width/heigth and content bias.

    Also, method updateGraphicPosition(Node) assumes that decorationNode's computed prefWidth and prefHeight are its actual width and height, which is incorrect. After sizing decorationNode, then the width and height of decorationNode.getLayoutBounds() should be used to compute the desired position.

    Also, it will be insufficient to simply listen to target.layoutBoundsProperty(), because other state changes that do not affect the target (Parent) layout bounds can and often will affect decorationNode's desired size. For example, the CSS cascade determines a new font size for the decorationNode that doesn't change the layout bounds of its Parent. I found that listening to the Parent's needsLayoutProperty() seemed effective to catch all cases in which GraphicDecoration needed to lay out decorationNode.

  2. Eugene Ryzhikov

    There is no special way to do it. On the pull request page you can see the source of changes provided by Tobias - it is a link to his fork with the branch which has those changes. Clone his fork and use his changes in your own fork.

  3. Rich Meyer

    Thanks Tobias - I guess I am confused about the Bitbucket.org UI. According to their tutorial, when I create a pull request, there should be an entry for the ControlsFX public repository in the destination pull-down as shown here: pull-request-7.png

    But my destination pull-down only has the entry for my forked repository: Capture.PNG I cannot select the other repository.

  4. tobiasdiez reporter

    @Rich Meyer Sadly your fix doesn't resolve this issue for me (I tried it out with the most recent SNAPSHOT build). The decoration node's position is still not updated when the text field is resized (by changing the size of a parent grid). However, as soon as I type something in the text field the node jumps to the correct position. So I imagine that the needsLayoutProperty changes to late. Maybe we need to add also a listener on the size property?

  5. Rich Meyer

    We haven't yet duplicated your remaining problem with TextField. By "grid" do you mean a GridPane or a ControlsFX Grid?

    However, we have now noticed a more serious problem using GraphicDecorator with Controls that use Skins extending LabeledSkinBase, such as Label, Button, etc.

    Every change that causes a call to LabeledSkinBase.updateChildren() after the decoration is applied causes the decoration to be removed from the Skin's children, and thus no longer displayed. For example, setting the text, setting the graphic, or setting the content display mode.

    Any Skin has the right to add and remove children at any time, thus calling into question the whole approach used by this decoration mechanism. That is, tampering with the Skin's children list and hoping the change will stick.

    I suppose we could raise another issue for that and implement another fix that listens for the decorationNode being removed from its Parent and re-adds it. But that type of hack, however, risks getting in an endless loop with a Skin that might have a listener that does the opposite.

  6. tobiasdiez reporter

    Yes, in our setting the text field is in a GridPane. But I suspect the problem occurs whenever a resize of the parent control yields a resize of the TextField. Sorry for not providing a minimal example, but I have a lot of work on my plate at the moment.

  7. tobiasdiez reporter

    @Rich Meyer I must have tried it out with a snapshot that not yet included your fix. I tried it again with the most recent version and now can confirm that it works perfectly. Sorry for the false report!

  8. Log in to comment