search

fury-gl / fury

FURY - Free Unified Rendering in pYthon.
https://fury.gl
Other
227 stars 165 forks source link

Overload set_visibility for Panel2D and Combobox2D #768

Closed dwijrajhari closed 1 year ago

dwijrajhari commented 1 year ago

Related issue: #731 (Clicking the tab of a ComboBox2D opens dropdown without changing icon)

Currently, the way set_visibility is implemented, it collects a list of all the actors down the tree and sets visibility of all of them. Thus the child elements of, say a Panel2D, don't get to choose which actors of themselves they want to make visible. Thus when a TabPanel2D containing a comboBox2D is selected, it sets the visibility of the comboBox's drop_down_menu to true, even though its _menu_visibility is set to false.

Possible fix: We overload the set_visibility method of Panel2D to call set_visibility on all of its elements. We then overload the set_visibility of comboBox2D to only make the dropdown visible if _menu_visibility is true

This change fixes #731 and also passes existing tests in fury\ui\tests

However, since I am not fully aware of the codebase, I don't know if there was a particular reason, set_visiblility was implemented that way. @skoudoro Could you give me some hints?

dwijrajhari commented 1 year ago

@skoudoro Added test. Kindly review

ganimtron-10 commented 1 year ago

@dwijrajhari can you rebase this PR so that we can review it?

@skoudoro This looks a good solution for the issue #562 #731

ganimtron-10 commented 1 year ago

@skoudoro Please have a look at this PR!

skoudoro commented 1 year ago

Hi @ganimtron-10 and @dwijrajhari,

I think the fix can be even simpler than that. Actually, I do not succeed to reproduce the issue on the main branch.

I agree that the arrow do not start on the right position but after 2 clicks, all is good. it look like an initialization issue and not set_visibility issue. I am not 100% sure.

So I recommend to test https://github.com/fury-gl/fury/issues/731 using master branch.

skoudoro commented 1 year ago

Ok, I looked deeper, I see what you are doing.