Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

This one might get me slaughtered, since I'm pretty sure it's bad coding practice, so here goes:

I have an AJAX driven site which loads both content and javascript in one go using Mootools' Request.HTML.

Since I have initialization scripts that need to be run to finish "setting up" the template, I include those in a function called pageComplete(), on every page

Visiting one page to another causes the previous pageComplete() function to no longer apply, since a new one is defined.

The javascript function that loads pages dynamically calls pageComplete() blindly when the AJAX call is completed and is loaded onto the page:

function loadPage(page, params) {
    // page is a string, params is a javascript object
    if (pageRequest && pageRequest.isRunning) pageRequest.cancel();
    pageRequest = new Request.HTML({
        url: '<?=APPLICATION_LINK?>' + page,
        evalScripts: true,
        onSuccess: function(tree, elements, html) {
            // Empty previous content and insert new content
            $('content').empty();
            $('content').innerHTML = html;
            pageComplete();
            pageRequest = null;
        }
    }).send('params='+JSON.encode(params));
}

So yes, if pageComplete() is not defined in one the pages, the old pageComplete() is called, which could potentially be disastrous, but as of now, every single page has pageComplete() defined, even if it is empty.

Good idea, bad idea?

share|improve this question
    
Bad practice, just bind to domready rather then creating a global pageComplete –  Raynos Oct 14 '11 at 14:09
    
Can't. The DOM is already ready, since I am loading the page dynamically. setTimeouts are even worse, and fail when there is network congestion. –  Julian H. Lam Oct 14 '11 at 14:21
    
domready should be cached at the application level. You really should be firing immediatly if the DOM is already loaded. The content should bootstrap itself and don't even get me started on .innerHTML = html –  Raynos Oct 14 '11 at 14:26
    
Raynos, sorry if I'm being thick, but I don't seem to understand. Did you mean to suggest that I should create new events to domready on each template page? I cannot include the associated javascript code in the currently existing domready tag, since domready is called before the page loads, and the code would be meaningless. –  Julian H. Lam Oct 14 '11 at 17:45
    
Also, please do (if you have the time), expand on the faults behind .innerHTML = html... I'm always open to learning ;) –  Julian H. Lam Oct 14 '11 at 17:46

3 Answers 3

You could place a line like

pageComplete=function(){};

somewhere in the code to make sure that the old function is not called for the new page. Possible places would be:

  • First line of loadPage

  • First line of onSuccess

  • after the call pageComplete();

That said, it still smells.

share|improve this answer

You need a way for loadPage() to know when the user has taken some action which should effectively cancel the loading of the first page. For example, if they click on a "page 1" button and you call loadPage() and they then click "page 2" while the remote resources are still loading, loadPage() needs to know that this has occurred.

A simple way to do this would be to check in the onsuccess handler if a global pageName variable still has the same value as it did when the request was fired, or perhaps Mootools offers a way to cancel any pending http requests which you can invoke when changing pages.

share|improve this answer
    
Hi matt b, the first line of the function achieves that. If the request is already set, then it becomes canceled and I start over, –  Julian H. Lam Oct 14 '11 at 17:42

I see no purpose for pageComplete() in your example. You already have evalScripts set to true.

  • I doubt this concept would even work, because the browser is unlikely to have parsed html and inserted the new HTML by the next line, where pageComplete() is called.
  • Both Prototype and (I'm fairly certain) jQuery have an alternative AJAX call that does the .innerHTML = html for you. I'd be very surprised if mootools didn't.

Consider this template, which looks like something similar to what html would contain:

<div id='content1'>Stuff is here</div>
<script type='text/javascript'>
   function pageComplete() {
      console.log('foo');
   }
</script>

Drop the function definition, and don't call pageComplete(). This should work much better:

<div id='content1'>Stuff is here</div>
<script type='text/javascript'>
   console.log('foo');
</script>

evalScripts should run that javascript after the AJAX loads, without you having to fiddle around with redefining any functions.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.