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

Fix ckeditor-init.js by giving it access to a valid jQuery instance, along with other minor changes to ckeditor-init.js #137

Merged
merged 1 commit into from Oct 18, 2014

Conversation

natejlong
Copy link

fixes #13 . Previously ckeditor-init.js failed to properly run as '$' was undefined. The fix is to use django's jQuery instance if '$' is undefined.

Also, I replaced the .click() with a .on() because I was running into issues with the event being attached before all elements had been placed. This is particularly an issue for users of django-nested-inlines.

Changes:

  • Set $ to django.jQuery if $ not defined
  • organize ckeditor-init.js
  • replace .click(...) with delegated .on(...) in case elements do not exist when code is run

…eplace .click(...) with delegated .on(...) in case elements do not exist when code is run
@riklaunim
Copy link
Contributor

django.jQuery is a nice one. As for "on" I assume it helps on nodes that are dynamically inserted?

@natejlong
Copy link
Author

That's right. I am using one of the implementations of django-nested-inlines on a project and I was running into issues with that. I figured it didn't cost too much to toss in support for nested inlines and other dynamically added nodes.

riklaunim added a commit that referenced this pull request Oct 18, 2014
Fix ckeditor-init.js by giving it access to a valid jQuery instance, along with other minor changes to ckeditor-init.js
@riklaunim riklaunim merged commit 5ebc347 into django-ckeditor:master Oct 18, 2014
@simonpanay
Copy link
Contributor

Since this commit I run into issues when loading ckeditor (it does not concerns the admin interface). I get a "ReferenceError: django is not defined" in the firefox console. I think it is due to this line :
"var $ = $ || django.jQuery;" at the beginning of the script. The following javascript code is not run at all.
Any idea what is going on ?
(My js knowledge is crap)

@natejlong
Copy link
Author

@simonpanay , what context are you running ckeditor in? If you are running it within django's admin, django should exist.

The issue itself is this: this commit fixes an issue where we try to use the $ object (which is jQuery), but it is not defined. In order to get around this issue, we first check to see if $ is defined, if it is not, then we set $ to be django.jQuery. The issue that you are having is that neither $ or django.jQuery is defined, and the code has no idea where to get jQuery. Fixing it is easy enough, but you need to find an instance of jQuery to point the code at.

Just as a sanity check, do you know if you have jQuery running?

@simonpanay
Copy link
Contributor

Indeed, this doesn't concern the admin interface where everything works as expected.
And yes, jQuery is loaded just before ckeditor.js and ckeditor-init.js
That's why I'm a bit lost here...

@natejlong
Copy link
Author

@simonpanay Hmm. You don't have a live example of the issue that I could look at, do you? Are you using require.js or browsify?

@riklaunim
Copy link
Contributor

we had some weir cases where jQuery was there but still undefined for the script. So it looks like it's not done yet.

@simonpanay
Copy link
Contributor

@natejlong , no I'm not using require.js nor browsify... And no live example, sorry...

@simonpanay
Copy link
Contributor

But I'm using Raven.js and bootstrap.
Do you thing it could be caused by those ?

@natejlong
Copy link
Author

@simonpanay , nah those wouldn't be causing the problem.
@riklaunim , Do you know what the conditions were in those cases?

@riklaunim
Copy link
Contributor

Nothing special. To address that there was few commits making the jQuery usage more typical and it always worked for me. I can't reproduce that problem in any way.

@simonpanay
Copy link
Contributor

@natejlong I made my email address public. Contact me and I will be able to give you access to a live example

@simonpanay
Copy link
Contributor

@natejlong Just by removing this line in ckeditor-init.js :
var $ = window.$ || django.jQuery;
Everything seem to go back to normal (in admin interface as well as in a front office)

@natejlong
Copy link
Author

Haha alright, thanks for figuring that out. I thought that that $ || django.jquery would have caught that but I guess not. Anyone know why this works?

@simonpanay
Copy link
Contributor

@natejlong Replacing the line
var $ = $ || django.jQuery;
by
$ = $ || django.jQuery;
seems to work too.

No idea what the difference is thought

@natejlong
Copy link
Author

The difference is the scoping of $. var $ constricts $ so it is only defined inside of ckeditor-init.js (the immediately invoked function at the top, at least). Removing the var makes $ defined globally, so any javascript code on the site has access to it. I would rather not make it global if it can be avoided, just for the sake of keeping the global namespace clean. The issue must be with how we're scoping $.

@simonpanay
Copy link
Contributor

... and it's the moment where my js knowledge stops :-)
Hope my comments will help. Thanks a lot anyway

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

Successfully merging this pull request may close these issues.

Adding inline RichTextField doesn't work
3 participants