Ticket #362 (closed enhancement: fixed)

Opened 4 years ago

Last modified 17 months ago

Enable add External JS

Reported by: enzo Owned by: somebody
Priority: must Milestone: 2.1
Component: QForms Version: 1.0.0 Stable
Keywords: External, Javascript Cc:

Description

Sometimes we need to load external JS files for instance

ad systems
CDN services
etc

I did a patch to enable include external javascript files in our applications

Attachments

External-Javascripts.patch Download (8.8 KB) - added by enzo 4 years ago.
patch362_v2.patch Download (3.0 KB) - added by alex94040 17 months ago.
v2 of the patch, works like a charm. Ready for review.

Change History

Changed 4 years ago by enzo

Changed 4 years ago by scottux

I think this is necessary, but that the method proposed in the patch is a bit over thought. Rather than adding a switch to the function, we could simply check to see if the script location starts with http i.e.

foreach ($strJavaScriptArray as $strScript) {	
			if(strpos($strScript, "http") === 0){
				$ret .= sprintf('<script type="text/javascript" src="%s"></script>', $strScript);
			}else{
				$ret .= sprintf('<script type="text/javascript" src="%s/%s"></script>', __VIRTUAL_DIRECTORY__ . __JS_ASSETS__, $strScript);
			}
			$ret .= "\r\n";
		}

I pulled this out of some code I had done for the QPage class where I need the same functionality.

Changed 4 years ago by alex94040

Yeah, the approach offered by scottux does seem a bit lighter. Let's go with that one.

Changed 4 years ago by alex94040

  • milestone changed from 1.1 to 1.2

Moving out to 1.2.

Changed 4 years ago by VexedPanda

  • status changed from new to closed
  • resolution set to duplicate

Dupe of #335

Changed 3 years ago by alex94040

  • milestone changed from 1.2 to 1.1.2

Changed 3 years ago by alex94040

  • milestone changed from 1.1.2 to 1.1.3

Changed 17 months ago by alex94040

  • status changed from closed to new
  • resolution duplicate deleted
  • milestone changed from 1.1.3 to 2.1

I'm reactivating this ticket as #335 is much more a philosophical discussion about how JS files are to be included, and this is a practical very small sub-case (enabling external JS files, with the prime case of CDN of jQuery).

Changed 17 months ago by alex94040

v2 of the patch, works like a charm. Ready for review.

Changed 17 months ago by alex94040

  • status changed from new to in_QA

Changed 17 months ago by mikederfler

I am using the same approach as in patch v2 and for me it works really great.

Changed 17 months ago by vakopian

Alex,

Not sure if the changes in the lines where DOCROOT and SUBDIRECTORY are defined were intentional (lines 61 and 63). If not, please revert these two lines.

Otherwise, looks good, please commit.

Changed 17 months ago by alex94040

  • status changed from in_QA to closed
  • resolution set to fixed

(In [1366]) Fixes #362 (Enable add External JS). Code by scottux + alex94040, review by vakopian.

Changed 17 months ago by alex94040

Deployed to the examples site! Yay, it's now working using the Google jQuery CDN!!

Note: See TracTickets for help on using tickets.