Resize `Panel2D` with `WindowResizeEvent`

[antrikshmisri]

[codecov[bot]]

Codecov Report

Merging #446 (82d1131) into master (0f97a6d) will decrease coverage by 0.14%. The diff coverage is 80.70%.

@@            Coverage Diff             @@
##           master     #446      +/-   ##
==========================================
- Coverage   88.53%   88.38%   -0.15%     
==========================================
  Files          31       31              
  Lines        6575     6632      +57     
  Branches      787      793       +6     
==========================================
+ Hits         5821     5862      +41     
- Misses        535      547      +12     
- Partials      219      223       +4     

Impacted Files	Coverage Δ
fury/ui/core.py	92.39% <50.00%> (-1.00%)	:arrow_down:
fury/ui/containers.py	83.94% <85.36%> (+0.12%)	:arrow_up:
fury/ui/tests/test_core.py	96.61% <100.00%> (+0.10%)	:arrow_up:
fury/interactor.py	95.23% <0.00%> (-1.74%)	:arrow_down:

[pep8speaks]

Hello @antrikshmisri! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2021-08-03 14:04:06 UTC

[antrikshmisri]

Hey @skoudoro, while rebasing this PR w.r.t the UI restructuring I noticed that you cannot import any class from fury/ui/elements.py in fury/ui/containers.py because Panel2D from containers is being imported into elements which makes them circularly dependent. This is the error:-

  File "D:\pythonProjects\fury-event\fury\fury\ui\containers.py", line 12, in <module>
    from fury.ui.elements import Button2D
  File "D:\pythonProjects\fury-event\fury\fury\ui\elements.py", line 19, in <module>  
    from fury.ui.containers import Panel2D
ImportError: cannot import name 'Panel2D'

[antrikshmisri]

Hey @skoudoro, thanks for the review. I have addressed all the comments. The tests will be added soon. I need to figure out how to record window invoked events first before I add tests.

[skoudoro]

ok, thank you for letting us know. it is strange that all CI's are failing, we need to figure out why there is this issue after your change

[antrikshmisri]

ok, thank you for letting us know. it is strange that all CI's are failing, we need to figure out why there is this issue after your change

Seems like the tests for Tab UI are failing. I think i faced the same problem earlier seemed to me that something was wrong with the recordings.

[antrikshmisri]

Also, could you add some tests?

As almost all the changes are based on events, I think I need to re-record the events for the existing tests for Panel2D, which can be seen here. But if I do that it will overwrite the existing recording, is there a way to extend the recording instead of overwriting the whole recording?

[skoudoro]

But if I do that it will overwrite the existing recording, is there a way to extend the recording instead of overwriting the whole recording?

No problem if you overwrite it. Make sure to watch first the recording. Then, you can replicate it approximately

[antrikshmisri]

Hey @skoudoro @Nibba2018, I am not sure how to test callbacks like these or these. Also, the events invoked by window are not being recorded even though the same event is being propagated to the respective actor.

[antrikshmisri]

Hey @skoudoro @Nibba2018, the tests for tab_ui are passing now Not sure what was happening earlier. Also, the WindowResizeEvent is still not being counted even though it has been added to the EventCounter. Also, the invokation/calling of the WindowResizeEvent is really slow, for 1-2 Panel2D its fine but from 4-5+ Panel2D it becomes painfully slow. I think this is because the event is being propagated to many actors at once instead of aborting at panel's actor. What do you guys think?

[skoudoro]

ok, Thank you for the feedback. Please, keep an example on a side to show how slow it is.

For now, we won't use WindowResizeEvent which means this PR is almost ready to be merged

[antrikshmisri]

Hey @skoudoro @Nibba2018, all the proposed changes have been added. PTAL.

[antrikshmisri]

Hey @skoudoro @Nibba2018, just a quick note the icon is fetched from a URL, if there is no internet connection this will error out. Possible fixes are to add the resize icon to the FURY icon dataset or add a broken link icon and if there is no internet connection the icon will be changed to that.

[ganimtron-10]

Hey @antrikshmisri , Can you please rebase this PR?

[ganimtron-10]

As we can see below when we resize the panel horizontally it works fine, but when we resize it vertically it seem to reposition itself as shown below.

https://github.com/fury-gl/fury/assets/64432063/6a8d3580-4e28-47b7-b0a3-9d048634bff0

[skoudoro]

No, it is not expected @ganimtron-10? You might need to look what is going on