inboundnow / retired-leads

Track visitor activity, capture and manage incoming leads, and send collected emails to your email service provider for WordPress
http://www.inboundnow.com/leads/
11 stars 3 forks source link

Making the plugin self contained.. #131

Closed vimes1984 closed 9 years ago

vimes1984 commented 9 years ago

line 60 of 'wp-content/leads/shared/assets/assets.loader.class.php' requires jquery, in my case my theme is concatenating all the js files and de_registering jquery, this causes the plugin to fail... Any ideas of making this completly self contained at some point?

atwellpub commented 9 years ago

We do have a plan to convert as much jquery to pure javascript as possible. But jQuery is so accepted these days, and it's a great tool. We do need this class to be available on the frontend too. Can you modify your theme to make it compatible?

vimes1984 commented 9 years ago

I've actually modified the plugin to require my concated js file including jquery...

On 14 August 2015 at 21:39, Hudson Atwell notifications@github.com wrote:

We do have a plan to convert as much jquery to pure javascript as possible. But jQuery is so accepted these days, and it's a great tool. We do need this class to be available on the frontend too. Can you modify your theme to make it compatible?

— Reply to this email directly or view it on GitHub https://github.com/inboundnow/leads/issues/131#issuecomment-131216779.

vimes1984 commented 9 years ago

have you thought about offering to allow a option to allow users to not use the wordpress jQuery ? maybe a checkbox that can read along the lines of: Do you wish to load your own version of jQuery? Am I the only person to un register the default wordpress jquery and concatnate the my js?

atwellpub commented 9 years ago

So far yes that I know of. We have dependencies on jQuery that we can't avoid at the moment.

I don't understand the core of the issue though. Why does the theme take protocol to deregister jquery?

vimes1984 commented 9 years ago

theoretically, the less number of http requests the better, so by running grunt in my theme build I'm concataning all of my JS files including jquery. Which I belive is relatively common practise, to accomodate this I'm deregistering jquery in my theme files which in turn causes leads to break since it has a dependency on jquery...

atwellpub commented 9 years ago

Can you craft your theme to leave an exception for WordPress's core jquery file?

vimes1984 commented 9 years ago

yes of course I could, but that would defeat the purpose of concatinating all the scripts..

On 20 August 2015 at 18:49, Hudson Atwell notifications@github.com wrote:

Can you craft your theme to leave an exception for WordPress's core jquery file?

— Reply to this email directly or view it on GitHub https://github.com/inboundnow/leads/issues/131#issuecomment-133073150.

atwellpub commented 9 years ago

It wouldn't be perfect. But why not concatenate jQuery along with the other scripts?

vimes1984 commented 9 years ago

and here lies the crux of the argument to concatante the jQuery with my other js files I have to then DEREGISTER jquery from the wordpress, causing your plugin to break :D

On 20 August 2015 at 19:00, Hudson Atwell notifications@github.com wrote:

It wouldn't be perfect. But why not concatenate jQuery along with the other scripts?

— Reply to this email directly or view it on GitHub https://github.com/inboundnow/leads/issues/131#issuecomment-133076807.

atwellpub commented 9 years ago

oh because of the requirement. I understand. Is there any way to fake the enqueue in a way that does not cause any extra load time?

vimes1984 commented 9 years ago

not that I know of .... hence my appearance here on this thread! :D

On 20 August 2015 at 19:08, Hudson Atwell notifications@github.com wrote:

oh because of the requirement. I understand. Is there any way to fake the enqueue in a way that does not cause any extra load time?

— Reply to this email directly or view it on GitHub https://github.com/inboundnow/leads/issues/131#issuecomment-133078333.

atwellpub commented 9 years ago

well you could register an empty file into the jquery namespace. It's still an http request but at least jquery gets concatenated and and leads gets tricked... What do you think?

vimes1984 commented 9 years ago

Not the cleanest of solutions:D For now I've just edited your core plugins files and noted on my repo what I've changed so as to apply the same changes after update.

atwellpub commented 9 years ago

that's a dangerous road though considering the upkeep requirements and the amount of updates we deploy. If it's a client site it might set the client up for eventual break down.

atwellpub commented 9 years ago

We won't have a pure JS setup until the middle of next year probably. It is one of our goals though.

vimes1984 commented 9 years ago

I've version controlled the theme and it's one edit in your files and it's docuemnted. I'm the only person updating it. I dodn't even need a full Js version just the ability to not require the jquery dependency on your front end files... or the ability to require a different file handle that isn't jquery...

On 20 August 2015 at 19:15, Hudson Atwell notifications@github.com wrote:

We won't have a pure JS setup until the middle of next year probably. It is one of our goals though.

— Reply to this email directly or view it on GitHub https://github.com/inboundnow/leads/issues/131#issuecomment-133079872.

vimes1984 commented 9 years ago

I may fork it this weekend and apply it myself...

On 20 August 2015 at 19:18, Christopher J.Churchill <churchill.c.j@gmail.com

wrote:

I've version controlled the theme and it's one edit in your files and it's docuemnted. I'm the only person updating it. I dodn't even need a full Js version just the ability to not require the jquery dependency on your front end files... or the ability to require a different file handle that isn't jquery...

On 20 August 2015 at 19:15, Hudson Atwell notifications@github.com wrote:

We won't have a pure JS setup until the middle of next year probably. It is one of our goals though.

— Reply to this email directly or view it on GitHub https://github.com/inboundnow/leads/issues/131#issuecomment-133079872.