On Wed, Mar 3, 2021 at 12:49 PM Brian Dukes ***@***.***> wrote: ***@***.**** requested changes on this pull request. Thanks! A couple of things to clean up, but nothing major. Let me know if you need any further direction. ------------------------------ In croppic-js/LICENSE.htm <#250 (comment)> : > @@ -0,0 +1,3 @@ +<p> + croppic is licensed under the <a href="http://croppic.net">GPLv3 license</a> for all open source applications. A commercial license is required for all commercial applications (including sites, themes and apps you plan to sell). I don't see anywhere referencing GPLv3 as a license, https://github.com/fussraider/croppic.js/blob/master/LICENSE (I believe this is the repo for the croppic-js npm library) and https://github.com/sconsult/croppic/blob/master/README.md#mit-license (the original source) both say MIT ------------------------------ In croppic-js/croppic.dnn <#250 (comment)> : > + <packages> + <package name="croppic-js" type="JavaScript_Library" version="<~=version~>"> + <friendlyName>croppic-js</friendlyName> + <description> + <![CDATA[Croppic is an image cropping jquery plugin that will satisfy your needs and much more.]]> + </description> + <owner> + <name>Engage Software</name> + <organization>Engage Software</organization> + <url>https://engagesoftware.com/</url> + ***@***.***</email> + </owner> + <license src="LICENSE.htm" /> + <releaseNotes src="CHANGES.htm" /> + <azureCompatible>true</azureCompatible> + <dependencies></dependencies> Let's add a jQuery dependency: ⬇️ Suggested change - <dependencies></dependencies> + <dependencies> + <dependency type="managedPackage" version="3.4.1">jQuery</dependency> + </dependencies> ------------------------------ In croppic-js/croppic.dnn <#250 (comment)> : > + <owner> + <name>Engage Software</name> + <organization>Engage Software</organization> + <url>https://engagesoftware.com/</url> + ***@***.***</email> + </owner> + <license src="LICENSE.htm" /> + <releaseNotes src="CHANGES.htm" /> + <azureCompatible>true</azureCompatible> + <dependencies></dependencies> + <components> + <component type="JavaScript_Library"> + <javaScriptLibrary> + <libraryName>croppic-js</libraryName> + <fileName>croppic.min.js</fileName> + <preferredScriptLocation>PageHead</preferredScriptLocation> Unless there's a specific need to have it in the head, libraries should prefer BodyBottom ⬇️ Suggested change - <preferredScriptLocation>PageHead</preferredScriptLocation> + <preferredScriptLocation>BodyBottom</preferredScriptLocation> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#250 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASW63XNNHLHPAX42Z4JNQDTB2AE7ANCNFSM4YRYEFGQ> .

On Wed, Mar 3, 2021 at 1:26 PM Brian Dukes ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In croppic-js/croppic.dnn <#250 (comment)> : > + <owner> + <name>Engage Software</name> + <organization>Engage Software</organization> + <url>https://engagesoftware.com/</url> + ***@***.***</email> + </owner> + <license src="LICENSE.htm" /> + <releaseNotes src="CHANGES.htm" /> + <azureCompatible>true</azureCompatible> + <dependencies></dependencies> + <components> + <component type="JavaScript_Library"> + <javaScriptLibrary> + <libraryName>croppic-js</libraryName> + <fileName>croppic.min.js</fileName> + <preferredScriptLocation>PageHead</preferredScriptLocation> @hkenuam <https://github.com/hkenuam> You got errors packaging, or you got errors trying to use it? You may need to adjust the timing of your scripts when you switch to using a JS library (e.g. wrap using code in jQuery(($) => { /* your original code goes here */ })) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#250 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASW63RUS56NGVJTYGAEL6LTB2ENFANCNFSM4YRYEFGQ> .