Skip to content
This repository was archived by the owner on Apr 16, 2019. It is now read-only.

AFv3.0 - Event "panelunload" triggered twice during panel unloading. Is this expected? #903

Closed
bugre opened this issue Nov 18, 2015 · 6 comments

Comments

@bugre
Copy link

bugre commented Nov 18, 2015

Hi,
i was surprised (with a bad programming decision on my side) when i was "confronted" with the "panelunload" event being triggered twice when unloading a panel. I was sure that i was doing something wrong (code was a mess :(( ) -- and it still can be something wrongly understood on my code also on the testcode attached bellow.

So doing a very simple code to test it.

  • "panelload" is called once
  • while "panelunload" is called twice in this sample.

Attached the sample code that reproduces it. I did look at the appframework.ui.js source code, but i don't understand all the nuances to suggest another approach. So for my case, i just decided to check if i already did the unload stuff or not (this is anyway good practice i think).
event_loadpanel-test.html.txt

Also attached a image of the stack trace showing where the double call is generated.
appfwv3-panelunload-event-twice01

But, decided to rise this issue so eventually some other person also is having this behavior and/or it's really something that needs to be changed.

Thanks
-wm

@imaffett
Copy link
Contributor

Thanks - a few people did an overhaul of how events were dispatched and it could have popped up in there. I will try to look into it when I have time, but it won't be until December.

@bugre
Copy link
Author

bugre commented Nov 19, 2015

Thanks @imaffett. No hurry. I solved my problem taking care that my code was correctly checking it's own state and nor relying on unknown external state/assumptions.

Yesterday night i did look at the code again, but there isn't much comment to help me understand how that function "runTransition" should flow (both unload triggers are generated inside this function). Did a few debugging sessions, so i write down my observations to eventually being of some help when you get a look at the code, but realized that i'm not able to understand it :(, sorry.

** If i totally misunderstood the code, please forgive-me and ignore thinking :)

  • there is a attribute "doingTransition", that i understood should act like state flag, but it never gets set to true (at least i could not find it). In func runTransition(), in the first block "from.end(..).run(..)" it's set false at the end of the block, makes sense, but should it not also be set true at the start. On the second block "to.end(..).run(..)" it is set to false at the start, should it not being set as true here and at the end of the block as false again?

Thanks
-wm

@holmberd
Copy link
Contributor

The problem seems to be from fixes #850, #860, #873 see here

Note: This only happens when transitioning panels using $.afui.loadContent().
$.afui.goBack() transition only trigger 'panelunload' once.

             if(!back){
                    this.classList.add("active");
                    $(this).trigger("panelload", [back]);
                    $(noTrans).trigger("panelload", [back]);

                    //Previous panel needs to be hidden after animation
                    //Fixes #850, #860, #873
                    var tmpActive = $(hide).find(".active").get(0);
                    if (undefined !== tmpActive) {
                        tmpActive.classList.remove("active");

                    }
                    $(hide).trigger("panelunload", [back]); <----- FIRED TWICE HERE
                }

*Merged it to previous pull request.

@imaffett
Copy link
Contributor

@holmberd thanks for the fix!

@bugre
Copy link
Author

bugre commented Mar 28, 2016

Hi @imaffett and @holmberd

Thanks for working on this. I know that the case is closed, but i'll comment here and you can decide if it should be reopened or treated as a different case.

I did update my local repo (git pull) and run some tests with a slightly modified version of my test html (attached) as to log load and unload for panel1 (P1) and panel2 (P2)), and the result's where not exactly those i expected.

I attach an Image with the chrome console output for my tests with some my comments.

*_Note: *_On tests 3 and 4, i did re-enable the line 1320 of appframework.ui.js (removing the "comment" sign)

Thanks,
test-event_loadpanel - tests 20160328-b after af3 upgrade
test-event_loadpanel.html.txt

@holmberd
Copy link
Contributor

@bugre I see what you mean and seems my scope of testing was too narrow. I'll take a look when I get home.

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

No branches or pull requests

3 participants