Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Registered event handlers are not removed when closing a panel #53

Closed
akstek opened this issue Aug 30, 2018 · 10 comments
Closed

Registered event handlers are not removed when closing a panel #53

akstek opened this issue Aug 30, 2018 · 10 comments

Comments

@akstek
Copy link

akstek commented Aug 30, 2018

Registered event handlers are not removed when closing a panel. This causes errors when creating a new window with a (custom) id that has already been used inside a container (not the body!)

@akstek
Copy link
Author

akstek commented Aug 30, 2018

@Flyer53
Demonstatrion https://jsfiddle.net/yjcwgdpr/37
Steps to reproduce:

  1. Click on Button "Create"
  2. Click on Button "Remove" or simply close the window
  3. Click on Button "Create" again
  4. Move the window around by dragging it by the title bar
  5. Look inside the console of the dev tools to see the exception
    VM1698 jspanel.min.js:1 Uncaught TypeError: Failed to execute 'getComputedStyle' on 'Window': parameter 1 is not of type 'Element'. at HTMLDivElement.s.calcSizeFactors (VM1698 jspanel.min.js:1) at HTMLDocument.<anonymous> (VM1698 jspanel.min.js:1) at HTMLDocument.<anonymous> (VM1698 jspanel.min.js:1)

@Flyer53
Copy link
Owner

Flyer53 commented Aug 30, 2018

@akstek
Thanks for the info. I'm aware of this error msg but have to admit that I didn't look into it yet. In part because it never showed any negative side effect (... and you're the first one reporting it). Is there something in your app that really doesn't work due to this error?

However, there is no excuse and I have to take care of that. Just give me some time, I'm on vacation currently 😄. I'm back in about 10 days and then I'll start looking into this issue.

Regards,
Stefan

@akstek
Copy link
Author

akstek commented Aug 31, 2018

In our case we use all the time the same pre generated IDs for the panels that run in a scaled container (not the body) an so it affects us. But it is not realy urgent as we wrote a plugin which hides the panels on close and does not remove them from the dom. But we have still some panels left which are removed from the dom and as we use a single page app that nearley never gets reloaded the list of this unremoved handlers is getting bigger. But as alyways thank you for your fast response. Maybe you can also take a look at our patched version of jspanel which solves the scaling problem.
#46 (comment)

@harsh00008
Copy link

I'm seeing the same issue for a parent container (not 'body').

@akstek
Copy link
Author

akstek commented Sep 3, 2018

@harsh00008
as I mentioned in my previous post this only happens if the container IS NOT the body.

@Flyer53
Copy link
Owner

Flyer53 commented Sep 5, 2018

@akstek @harsh00008
I'll be back home next week and than I'll start looking into this issue ...
@akstek
... and then I'll also look at your patched pen. Thanks for your work 👍

@grapenut
Copy link

@Flyer53 I also noticed this issue, under the same circumstances of having a non-body container with jspanels sharing the same id being destroyed and recreated. I submitted PR #55 that fixes the issue by moving those 3 event handlers into variables on self, then calling document.removeEventListener after cleanup in self.close(). Enjoy the rest of your vacation!

@Flyer53
Copy link
Owner

Flyer53 commented Sep 11, 2018

Ok folks, I seem to have found a fix for this. Would you please check out the attached file and let me know how it works out.

jspanel.zip

In the end it was quite easy: The internally needed handlers for jspaneldragstop, jspanelresizestart and jspanelresizestop events were attached to the document. I changed that and attached the handlers to the panel itself so that the browser's garbage collection can remove the handlers when a panel is closed. At least that's my thinking as amateur ... and it works fine on first tests.

Other changes in the modified file:

  • fixed methods setHeaderLogo() and setHeaderTitle()
  • all parameters of option position accept a function as value (not sure about this yet, but could be quite useful)

@akstek
Copy link
Author

akstek commented Sep 12, 2018

@Flyer53
I updated my fiddle to use the attached version and everything seems to run as expected. No errors till now. Thank you for your hard workl 👍
https://jsfiddle.net/yjcwgdpr/39/

@Flyer53
Copy link
Owner

Flyer53 commented Sep 14, 2018

Issue fixed with v4.2.1

@Flyer53 Flyer53 closed this as completed Sep 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants