Menu ▾ ▴

#27 remove recursive call in Element.setVisible() method

nifty-1.0
closed
nobody
None
5
2012-09-02
2011-04-11
mulova
No

https://sourceforge.net/projects/nifty-gui/forums/forum/807892/topic/4478259

Recursive call in setVisible()/setFocusable() may incur unexpected results to element hierarchy.
Let's say, element A has B1, B2, B3, B4, C as children.
user want to show B1, B2, B3, B4 but not C.
C is shown only for a specific situation.
In such situation, user have to call setVisible(true) for each B*.
It's not only inconvenient but also weird.

I think setting some value to parent element should not change child element properties.

I changed the setVisible()/isVisble() code and confirms it works well on AllExamplesMain.
But setFocusable() doesn't work. I don't know why.
So I attach the setVisible() fix only.

Discussion

  • mulova

    mulova - 2011-04-11

    Actually I think setFocusable() has no relationship with parent,
    So I just removed setFocusable() recursive call. no change to isFocusable()

     

  • mulova

    mulova - 2011-04-11

    remove recursive call in Element.setVisible()/setFocusable()

     
    Element.java.patch

  • void256

    void256 - 2011-04-11

    why do you think that this is a bug?

    if you hide element x then all child elements of element x (as well as all child elements of all child elements and so on) will be hidden too. this is intended behaviour.

    and this is not a trivial issue to solve for all cases. your patch breaks some unit tests too.

    it "might" be possible to change this but this will be more involving. first of all nifty does currently not check the visibility related to the parent elements (your patch does this but not for all checks - just check how many code lines access the plain visible flag at the element level without checking the parents visibility state).

    being very close to 1.3 and being not convinced that this is a bug I don't think I'd like to change this - al least not now.

     

  • mulova

    mulova - 2011-04-12

    I agree it is not appropriate to fix this because it is about to release 1.3.
    It is not important when this patch is applied. But I think it should be applied even later.
    This feature will make it very difficult to handle nifty ui.
    When someone has a complex UI hierarchy and the visibility is changed often,
    it makes the nifty ui management catastropic.
    I experienced this situation from my current project so I'm convinced. :(

     

  • void256

    void256 - 2012-09-02

    Closed here and moved to github issue tracker to be considered for Nifty 1.4

    see -> https://github.com/void256/nifty-gui/issues/43

    • status: open --> closed
    • milestone: --> nifty-1.0