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

check that jquery is loaded before using #229

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tisdall
Copy link
Contributor

@tisdall tisdall commented Jul 15, 2014

solves #156 #211 #215 indirectly

Instead of blindly calling jQuery in deform.js, first test that it's loaded before calling. This way you can now postpone loading jQuery until the end of a document and then make your own call to $(document).ready(function(){ deform.load(); });. Without this change, the forced call to jQuery simply throws an error and no further javascript is processed.

@tisdall
Copy link
Contributor Author

tisdall commented Jul 21, 2014

comments? Isn't this the most ideal fix for everyone?

@mvaled
Copy link

mvaled commented Aug 8, 2014

Well, both #215 and #156 have been closed in favor of this one. Hope that amounts to something.

@tisdall
Copy link
Contributor Author

tisdall commented Oct 15, 2014

Alright, this has been collecting dust for 3 months now... Can I get some feedback or a commit? All this code is doing is confirming jQuery is loaded before calling it. It allows for the very common case of loading js at the end of the page.

@mvaled
Copy link

mvaled commented Oct 23, 2014

Hi @tseaver, Would you consider this PR to be included? Would it break anything?

@tisdall
Copy link
Contributor Author

tisdall commented Apr 7, 2015

@tisdall
Copy link
Contributor Author

tisdall commented Apr 29, 2015

Okay, update for @mvaled @dwt and anyone else reading this...

The consensus is this won't be included until the following other changes are made:

  • all templates need to be able to load properly with jquery being postponed (sequence.pt makes a call to jquery outside of addCallback, richtext does too, probably others...)
  • changes to the docs to explain how you can postpone loading jquery
  • addition of tests with postponing jquery loading

Until then... You just need to run your own copy of deform.js instead of the one included in the package.

@mvaled
Copy link

mvaled commented Apr 29, 2015

Hi @tisdall,

Thanks for the update. I wasn't aware of those other cases.

However, I do suggest a change to the current documentation. As I commented in #156, the documentation suggests to put the call near the end of the HTML page. Currently, it reads:

The JavaScript function deform.load() must be called by the HTML page (usually in a script tag near the end of the page, ala <script..>deform.load()</script>) which renders a Deform form in order for widgets which use JavaScript to do proper event and behavior binding. If this function is not called, built-in widgets which use JavaScript will not function properly. For example, you might include this within the body of the rendered page near its end:

(The bolds are mine to emphasize). The call is mostly useless since deform.js itself registers the callback. Is there any use case for calling deform.load() by hand at the end of the HTML page? If not you can simply remove that from the doc, until this PR can be merged.

@Themanwithoutaplan
Copy link
Contributor

TBH with HTTP/2 coming over the horizon I don't think we really need to work too much about when we load which library. However, I do agree with the general thrust of the change.

@stevepiercy stevepiercy added this to the 3.0.0 milestone Aug 23, 2020
@stevepiercy
Copy link
Member

Hi @tisdall, I am working on a release of Deform 3.0.0. I would like to include this improvement, contingent to the items in your comment for how to move this forward.

I don't think we'll put it into Deform 2.x because of the breaking changes from those contingent items.

Base automatically changed from master to main January 19, 2021 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants