intel/appframework

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

Closed this issue · 6 comments

bugre commented

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

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 commented

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

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.

@holmberd thanks for the fix!

bugre commented

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

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