fix donutty.js

[TemporaryTetra]

Check for HTMLElement does not work correctly if el is in an iframe. Since iframe creates a new space.

Therefore, in this case, it will be better to check if el exists in the document.

[simeydotme]

Hi @TemporaryTetra I kind of get what the problem is, as window.HTMLElement is bound to the top window, and not the iFrame ... but your solution is totally missing the point in that the input parameter can be either;

1. A string ( .myClass ); whereby doc.querySelectorAll() finds the parent
2. An element ( HTMLElement ); whereby the element is used as-is for the parent
3. Or empty; whereby the body element is used

By your code, the ability to pass in a HTMLElement reference would be broken altogether.

[simeydotme]

Please make the change suggested and I can build and test

[simeydotme]

this should be;

} else if (  el instanceof HTMLIFrameElement || el instanceof HTMLElement ) {

[TemporaryTetra]

Thanks for the quick reply, I also tried HTMLIFrameElement but this check doesn't see my wrapper class. https://i.imgur.com/uHCSZ2n.png пт, 15 січ. 2021 о 17:28 Simon Goellner <notifications@github.com> пише:

…

***@***.**** requested changes on this pull request. Please make the change suggested and I can build and test ------------------------------ In src/donutty.js <#21 (comment)>: > @@ -22,7 +22,7 @@ this.$wrapper = doc.querySelectorAll( el )[0]; - } else if ( el instanceof window.HTMLElement ) { + } else if ( doc.querySelector( `.${el.className}` ) ) { this should be; } else if ( el instanceof HTMLIFrameElement || el instanceof HTMLElement ) { — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALEM52RD32G2EQOAYBTTENLS2BNJZANCNFSM4WDV4U6Q> .

[TemporaryTetra]

But the way it works on the front end is correct https://i.imgur.com/uHCSZ2n.png пт, 15 січ. 2021 о 21:26 Nikolai Nasibullin <zedoir1@gmail.com> пише:

…

Thanks for the quick reply, I also tried HTMLIFrameElement but this check doesn't see my wrapper class. https://i.imgur.com/uHCSZ2n.png пт, 15 січ. 2021 о 17:28 Simon Goellner ***@***.***> пише: > ***@***.**** requested changes on this pull request. > > Please make the change suggested and I can build and test > ------------------------------ > > In src/donutty.js > <#21 (comment)>: > > > @@ -22,7 +22,7 @@ > > this.$wrapper = doc.querySelectorAll( el )[0]; > > - } else if ( el instanceof window.HTMLElement ) { > + } else if ( doc.querySelector( `.${el.className}` ) ) { > > this should be; > > } else if ( el instanceof HTMLIFrameElement || el instanceof HTMLElement ) { > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#21 (review)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ALEM52RD32G2EQOAYBTTENLS2BNJZANCNFSM4WDV4U6Q> > . >

[simeydotme]

Sorry, I totally don't understand what that means, are you able to show me a reduced test case in CodeSandbox or something?