[RFC|Merged] Template tag to include JS files

[callumacrae]

I propose an INCLUDEJS template tag. It would work as follows:

Code: Select all

<!-- INCLUDEJS 'template/somefile.js' -->

It would be callable from anywhere with the document, and would add a script tag to include JS in the footer after jQuery.

This would have the advantage of reducing the number of conditionals in the header and footer for JavaScript files that are only used on some pages (such as editor.js).

[Arty]

+1

Another advantage of using this is allowing template inheritance functionality. No more {T_SUPER_TEMPLATE_PATH} to include js file from main style. To locate js file phpBB can use template locator class.

How would it be implemented code wise? I suggest using the easy way: converting it to HTML <script> code and adding it to template variable {SCRIPTS} that will be added after jQuery in footer template

Another question is how path to js files will be calculated? Relative to template directory or relative to style directory? I think relative to template directory would make more sense for consistency with INCLUDE and allow path to start with ../ if js file is in other style's directory

[Arty]

Test implementation: https://github.com/cyberalien/phpbb3/tr ... /includejs
It also includes second test style: progreen, that inherits from prosilver.

Implemented instructions:

Code: Select all

<!-- INCLUDEJS test.js -->

This will locate file test.js and append code for it to variable {SCRIPTS}. If file is not found, locator returns url to where file is supposed to be in current style.
Then in overall_footer.html after jQuery I've added variable {SCRIPTS}

It also works with template variables, like this:

Code: Select all

<!-- DEFINE $TEST = 'test.js' -->
<!-- INCLUDEJS {$TEST} -->

In cases like this files are located dynamically during template display, so its a bit lower than hard coded file names.


In order to implement that, I had to add some other functionality that might be useful for other modifications:
- $template->context->append_var(var, value) - append to template variable (maybe add template->append_var for it too?)
- $template->locate(file, is_template) - finds file, returns file name relative to phpBB root directory. If second parameter is true, $file is relative to template directory, if false, $file is relative to style directory.

[callumacrae]

Looks good.

Could this be moved to 3.1, please?

[nextgen]

Test implementation: https://github.com/cyberalien/phpbb3/tr ... /includejs
It also includes second test style: progreen, that inherits from prosilver.

Implemented instructions:

Code: Select all

<!-- INCLUDEJS test.js -->

This will locate file test.js and append code for it to variable {SCRIPTS}. If file is not found, locator returns url to where file is supposed to be in current style.
Then in overall_footer.html after jQuery I've added variable {SCRIPTS}

It also works with template variables, like this:

Code: Select all

<!-- DEFINE $TEST = 'test.js' -->
<!-- INCLUDEJS {$TEST} -->

In cases like this files are located dynamically during template display, so its a bit lower than hard coded file names.


In order to implement that, I had to add some other functionality that might be useful for other modifications:
- $template->context->append_var(var, value) - append to template variable (maybe add template->append_var for it too?)
- $template->locate(file, is_template) - finds file, returns file name relative to phpBB root directory. If second parameter is true, $file is relative to template directory, if false, $file is relative to style directory.

+1

[brunoais]

Looks good. Does that work among the template inheritance RFC?

[callumacrae]

Looks good. Does that work among the template inheritance RFC?

Yes

[brunoais]

^ This will certainly create more transparency between the final code (the cached code) and the template code which means more flexibility without hitting performance. I love it!
* brunoais went to take the pills to calm down the excitment (kidding )

Anyway, for optimization purposes, we could have that tag to place the script after jQuery and we could have another tag to place the script before jQuery. This is meant for optimization only, no big deal if you don't want to.

You and performance >_<

I'm a speed freak and hate when things take longer as they should.

[callumacrae]

You and performance >_<

[Arty]

Updated tree to latest code that uses [RFC] Template class changes. Also changed all scripts that could be changed from <script> to INCLUDEJS without changing scripts (yes, all 3 of them )

Will have to wait for that PR to be merged before making PR for this RFC because having 2 pull requests that share code will get messy.