-
Notifications
You must be signed in to change notification settings - Fork 58
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
added check for self.parentElement before attempting to remove #213
base: master
Are you sure you want to change the base?
Conversation
@mfoitzik Hey Mike, thanks for your addition 😄 |
Hi Stefan. The if (!self.parentElement) {return;} you reference is part of the self.close method. The change I made is in the self.remove method. I guess if you do not like the if statement you could add the same the (i.e., if (!self.parentElement) {return;}) at the beginning of the self.remove method.
By the way, I really love the work you have done on this project. I use it in my SimplicityBuilder drag and drop WYSIWYG builder. I plan on open sourcing this project in the near future.
Thank you,
Mike
Mike Foitzik
***@***.*** | www.mifo.com
1968 Valindo Way | Vista | CA 92084
Confidentiality Notice: This page and any accompanying documents contain information that is proprietary and confidential and may also be privileged. It is intended for the exclusive use of the addressee. This information is private. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the contents of this information in any manner is strictly prohibited.
…----------------------------------------
From: "Stefan Sträßer" ***@***.***>
Sent: 12/2/24 3:48 AM
To: Flyer53/jsPanel4 ***@***.***>
Cc: Mike Foitzik ***@***.***>, Mention ***@***.***>
Subject: Re: [Flyer53/jsPanel4] added check for self.parentElement before attempting to remove (PR #213)
@mfoitzik Hey Mike, thanks for your addition 😄
But I wonder how the line of code you referenced can cause this kind of error.
The code is used inside self.close() which starts with if (!self.parentElement) {return;} as the first line of code. So I don't understand the error. Do I miss something?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
Hi Mike, thanks for the quick answer ... and thanks for the praise 😄 It's not that I have a problem with the if statement. I just don't understand ... self.close = (cb, closedByUser) => {
// if panel does not exist return
if (!self.parentElement) {return;}
/*
more code ...
*/
self.remove()
} ... my thinking is that |
Hi Stefan,
To be honest I was never able to figure out where in my code the action was taking place to trigger the error in JSPanel. I was just seeing the error in the browser console and worked my way back from there.
Could the scenario be that I am removing a panel without closing it?
I can look further into it if it helps. I am also OK with hard coding the change in my application since it is straight forward for me.
Thank you,
Mike
Mike Foitzik
***@***.*** | www.mifo.com
1968 Valindo Way | Vista | CA 92084
Confidentiality Notice: This page and any accompanying documents contain information that is proprietary and confidential and may also be privileged. It is intended for the exclusive use of the addressee. This information is private. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the contents of this information in any manner is strictly prohibited.
…----------------------------------------
From: "Stefan Sträßer" ***@***.***>
Sent: 12/2/24 6:12 AM
To: Flyer53/jsPanel4 ***@***.***>
Cc: Mike Foitzik ***@***.***>, Mention ***@***.***>
Subject: Re: [Flyer53/jsPanel4] added check for self.parentElement before attempting to remove (PR #213)
Hi Mike, thanks for the quick answer ... and thanks for the praise 😄
It's not that I have a problem with the if statement. I just don't understand ...
self.close = (cb, closedByUser) => {
// if panel does not exist return
if (!self.parentElement) {return;}
/*
more code ...
*/
self.remove()
}
... my thinking is that self.remove() never gets called when there is no parent element because self.close() already returns in this case prior self.close() is called. Am I wrong here?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
Hi Mike, I did a few quick tests and could not reproduce the same error. Is there any reason that you remove a panel without using the close method? Somehow you have to reference the panel anyhow, so why not use the close method? |
Hi Stefan. I have dynamic configurations in my Simplicity Builder component. I rebuild panels as part of setting up new configurations and as part of that I go through and remove any existing panels.
I need to research this further to see the exact point that triggers the issue.
Mike Foitzik
***@***.*** | www.mifo.com
1968 Valindo Way | Vista | CA 92084
Confidentiality Notice: This page and any accompanying documents contain information that is proprietary and confidential and may also be privileged. It is intended for the exclusive use of the addressee. This information is private. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the contents of this information in any manner is strictly prohibited.
…----------------------------------------
From: "Stefan Sträßer" ***@***.***>
Sent: 12/3/24 3:09 AM
To: Flyer53/jsPanel4 ***@***.***>
Cc: Mike Foitzik ***@***.***>, Mention ***@***.***>
Subject: Re: [Flyer53/jsPanel4] added check for self.parentElement before attempting to remove (PR #213)
Hi Mike, I did a few quick tests and could not reproduce the same error.
Is there any reason that you remove a panel without using the close method? Somehow you have to reference the panel anyhow, so why not use the close method?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
Hi Mike, document.querySelectorAll('.jsPanel').forEach((item) => {
item.close();
}); ... since all panels have the CSS class jsPanel by default. |
OK. I'll give that a try.
Mike Foitzik
***@***.*** | www.mifo.com
1968 Valindo Way | Vista | CA 92084
Confidentiality Notice: This page and any accompanying documents contain information that is proprietary and confidential and may also be privileged. It is intended for the exclusive use of the addressee. This information is private. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution, or use of the contents of this information in any manner is strictly prohibited.
…----------------------------------------
From: "Stefan Sträßer" ***@***.***>
Sent: 12/3/24 6:28 AM
To: Flyer53/jsPanel4 ***@***.***>
Cc: Mike Foitzik ***@***.***>, Mention ***@***.***>
Subject: Re: [Flyer53/jsPanel4] added check for self.parentElement before attempting to remove (PR #213)
Hi Mike,
In order to remove all panels you could use ...
document.querySelectorAll('.jsPanel').forEach((item) => {
item.close();
});
... since all panels have the CSS class jsPanel by default.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
Hi Stefan. I came across this issue in my implementation. There is a step in the code that does: self.parentElement.removeChild(self), but sometimes self.parentElement does not exist and causes an error. I wrapped it in an if statement to first check for self.parentElement and it fixed the problem. Thank you.