Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8
    • Fix Version/s: 1.8
    • Component/s: AJAX and JavaScript
    • Labels:
      None
    • Affected Branches:
      MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE
    • Rank:
      34386

      Description

      Others and me are using YUI functions more often now. See for example http://moodle.org/mod/forum/discuss.php?d=63792.

      1) With the actual loading function we may encounter problems with double loaded libraries. How can the require_once_js() function be implemented?

      2) The YUI loader should be always on, independent from the Drag&Drop feature.

        Activity

        Hide
        Vy-Shane Sin Fat added a comment -

        I've checked in changes to require_js(), into HEAD. It basically behaves like the require_once() PHP function. You can give the (www) path of the JavaScript library you want loaded, or you can pass an array of paths. You an also give it the short names of some of the common library files (mostly YUI stuff). If a file is not found, we error() out.

        Used without arguments, require_js() will return the html needed to load the JS files. print_header() in weblib now calls require_js().

        Show
        Vy-Shane Sin Fat added a comment - I've checked in changes to require_js(), into HEAD. It basically behaves like the require_once() PHP function. You can give the (www) path of the JavaScript library you want loaded, or you can pass an array of paths. You an also give it the short names of some of the common library files (mostly YUI stuff). If a file is not found, we error() out. Used without arguments, require_js() will return the html needed to load the JS files. print_header() in weblib now calls require_js().
        Hide
        Urs Hunkler added a comment -

        Great Vy, thanks a lot.

        When I add YUI files within my themes I add the require_js() without a parameter in the header.html. Before I add the libraries by passing the array of all needed YUI libraries.

        This way the Moodle page contains two sets of YUI loading lines: A) all set within Moodle and B) containing A) plus my added libs.

        We need a way to add YUI libraries in header.html without duplicating the loading list.

        One more aspect to integrate.

        Show
        Urs Hunkler added a comment - Great Vy, thanks a lot. When I add YUI files within my themes I add the require_js() without a parameter in the header.html. Before I add the libraries by passing the array of all needed YUI libraries. This way the Moodle page contains two sets of YUI loading lines: A) all set within Moodle and B) containing A) plus my added libs. We need a way to add YUI libraries in header.html without duplicating the loading list. One more aspect to integrate.
        Hide
        Urs Hunkler added a comment -

        Vy, I needed YUI loading from the header file, therefore I changed your function. I hope that is ok.

        My changes are in CVS.

        I added a second parameter. If it is set to 1 and an array of YUI names is set, the function returns the loading strings only for those YUI libs which have not already been loaded in a previous empty call.

        a) within Moodle only require_js ($lib[]) is called to collect libraries
        b) print_header calls require_js() to output the library links
        c) from header.html you call require_js($lib[], 1) to add library links not yet set

        I commented your original lines out , so you can easily see what I have changed.

        One note from www.php.net: If you use array_push() to add one element to the array it's better to use $array[] = because in that way there is no overhead of calling a function. I changed that part.

        I needed to replace your recursive call with a foreach loop. I don't know if the recursive call of require_js is slower than a foreach loop??? Do you know?

        Show
        Urs Hunkler added a comment - Vy, I needed YUI loading from the header file, therefore I changed your function. I hope that is ok. My changes are in CVS. I added a second parameter. If it is set to 1 and an array of YUI names is set, the function returns the loading strings only for those YUI libs which have not already been loaded in a previous empty call. a) within Moodle only require_js ($lib[]) is called to collect libraries b) print_header calls require_js() to output the library links c) from header.html you call require_js($lib[], 1) to add library links not yet set I commented your original lines out , so you can easily see what I have changed. One note from www.php.net: If you use array_push() to add one element to the array it's better to use $array[] = because in that way there is no overhead of calling a function. I changed that part. I needed to replace your recursive call with a foreach loop. I don't know if the recursive call of require_js is slower than a foreach loop??? Do you know?
        Hide
        Vy-Shane Sin Fat added a comment -

        Urs, you broke backwards compatibility. This function should also accept $lib as a string (hence the recursive call).

        Show
        Vy-Shane Sin Fat added a comment - Urs, you broke backwards compatibility. This function should also accept $lib as a string (hence the recursive call).
        Hide
        Vy-Shane Sin Fat added a comment -

        Hi Urs,

        I've reverted the require_js() function.

        To enable you to load javascript libraries from themes, I've modified print_header() in lib/weblib.php. If you place your require_js() calls in your themedir/meta.php, these libs will get loaded too (no duplicates).

        I've changed array_push() to $array[] = as you suggested. It shouldn't really matter. We won't be including thousands of javascript libraries anyway. I've kept the recursive call, although it is mostly a matter of taste. It keeps the function succint and easy to understand.

        Usage examples:

        require_js(array('yui_logger', 'yui_slider')); // Accepts an array of shortnames or paths
        require_js('yui_animation'); // Accepts a shortname string as well
        require_js($CFG->wwwroot.'/lib/somecustomlibs/mylib.js'); // Or a path string
        require_js($CFG->wwwroot.'/lib/somecustomlibs/mylib.js'); // This will NOT be loaded twice

        require_js(); // Returns the html needed to load all of the above libs

        Show
        Vy-Shane Sin Fat added a comment - Hi Urs, I've reverted the require_js() function. To enable you to load javascript libraries from themes, I've modified print_header() in lib/weblib.php. If you place your require_js() calls in your themedir/meta.php, these libs will get loaded too (no duplicates). I've changed array_push() to $array[] = as you suggested. It shouldn't really matter. We won't be including thousands of javascript libraries anyway. I've kept the recursive call, although it is mostly a matter of taste. It keeps the function succint and easy to understand. Usage examples: require_js(array('yui_logger', 'yui_slider')); // Accepts an array of shortnames or paths require_js('yui_animation'); // Accepts a shortname string as well require_js($CFG->wwwroot.'/lib/somecustomlibs/mylib.js'); // Or a path string require_js($CFG->wwwroot.'/lib/somecustomlibs/mylib.js'); // This will NOT be loaded twice require_js(); // Returns the html needed to load all of the above libs
        Hide
        Vy-Shane Sin Fat added a comment -

        Reopen if there are still issues.

        Show
        Vy-Shane Sin Fat added a comment - Reopen if there are still issues.
        Hide
        Urs Hunkler added a comment -

        Hi Vy,

        sorry for breaking backwards compatibility and many thanks for your changes. I will see how they work when I update Moodle 1.8 and change my themes.

        Moodle is a very heavy app with noticeable page load times. I am sure every performance enhancement whatever small it may be will enhance the user experience.

        Show
        Urs Hunkler added a comment - Hi Vy, sorry for breaking backwards compatibility and many thanks for your changes. I will see how they work when I update Moodle 1.8 and change my themes. Moodle is a very heavy app with noticeable page load times. I am sure every performance enhancement whatever small it may be will enhance the user experience.

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: