Adding the promote-js embed script #2111

riverspirit · 2014-03-09T10:49:34Z

About promote MDN script
People can embed this code in their sites and it will scan their site content for keywords such as 'JavaScript' and will link it back to the corresponding page on MDN on that topic.

Code to embed in sites

<!-- Include the below line only if the page doesn't have jQuery included already -->
<script src="https://code.jquery.com/jquery-2.1.0.min.js"></script>

<!-- The rest of the lines are required -->
<script>
    (function(){
        window._promote_mdn_options = {maxlinks: 3}; // Configurable settings
        var e = document.createElement('script'); e.type='text/javascript';
        e.src = document.location.protocol + '//developer.cdn.mozilla.net/media/redesign/js/promote-mdn.js';
        var s = document.getElementsByTagName('script')[0]; s.parentNode.insertBefore(e, s);
    })();
</script>

icaaq · 2014-03-09T11:02:36Z

It would be awesome if jquery wasn't needed for this one, There's not a lot of $-functions. maybe you could change them if your looking at this http://youmightnotneedjquery.com/

darkwing · 2014-03-09T11:05:25Z

I agree with @icaaq ; we should avoid jQuery if possible. Additionally, I think we should make this one big function instead of setting globals as options. Then the user could call:

initPromoteLinks(/* options */);

riverspirit · 2014-03-09T11:08:22Z

Used jQuery since it made it easier for me to prototype not worrying about compatibility issues. I was planning to remove jQuery once I finished the basic stuff. Doing that now. Will remove the global config too. Thanks @icaaq & @darkwing

riverspirit · 2014-03-09T13:28:08Z

Removed jQuery dependency and use of global for settings. The embed script is now:

<script>
    (function (d,f,a) {
        // Your settings here
        var o = {maxlinks: 4, include_elems: ['p', 'div']};

        var s=d.createElement("script");s.type="text/javascript";if(s.f)
        {s.a=function(){if(s.f=="loaded"||s.f=="complete")
        {s.a=null;_mdn_promote_links(o)}}}else{s.onload=function()
        {_mdn_promote_links(o)}}s.src="promote-mdn.js";
        d.getElementsByTagName("head")[0].appendChild(s)
    })(document,'readyState','onreadystatechange');
</script>

darkwing · 2014-03-09T13:40:46Z

media/redesign/js/promote-mdn.js

+    // For the time being, we are not gonna do anything if querySelectorAll is not available in the browser
+    if ('querySelectorAll' in document) {
+        var elements = document.querySelectorAll(options.include_elems.join(', '));
+        Array.prototype.forEach.call(elements, function(o, i){


You can use elements.forEach directly, I believe; might be cleaner. We should probably also include an Array.prototype.forEach check as well, since that was introduced with querySelectorAll.

darkwing · 2014-03-09T13:44:22Z

One more tiny nitpick is that the JavaScript standard is camel-casing. Not a big deal.

darkwing · 2014-03-09T13:46:52Z

media/redesign/js/promote-mdn.js

@@ -0,0 +1,161 @@
+var _mdn_promote_links = window._mdn_promote_links || function (user_settings) {


Can this just be: function mdnPromoteLinks()?

riverspirit · 2014-03-09T14:26:56Z

Changes:

Removed a massive if with early return from function
Replaced all under_scored names with camelCased ones
Cleaned up usage of nodelist.forEach and added a polyfill
Added provision to pass custom className for outbound links

a2sheppy · 2014-03-12T20:43:48Z

FWIW, once this hits production, we need to be sure to remove the warning that this doesn't work from the docs pointing to it here: https://developer.mozilla.org/en-US/docs/MDN/Promote#Embed_code_%28JavaScript%29

riverspirit · 2014-03-13T03:00:24Z

I have some more improvements to make and some more tests to do. Once I complete them in a couple of days I'll deploy it in my blog for testing. Once it seems working ok, I'll edit the doc and remove the warning.

Thanks,

darkwing · 2014-03-18T13:51:08Z

Awesome, just let me know when it's ready to look at again!

darkwing · 2014-03-18T13:53:22Z

Also, I just thought of something. Can we add another userSettings key which lets the user add more items to the dataset? That will let users create their own links! :)

a2sheppy · 2014-03-18T14:52:12Z

I agree, adding s user settings option for custom links would be helpful.

Eric Shepherd
Sent from my iPad

On Mar 18, 2014, at 9:53 AM, David Walsh notifications@github.com wrote:

Also, I just thought of something. Can we add another userSettings key which lets the user add more items to the dataset? That will let users create their own links! :)

—
Reply to this email directly or view it on GitHub.

riverspirit · 2014-03-18T16:08:50Z

Yeah, will do. (Sorry, got caught up in pending work at office after reaching back from Paris)

riverspirit · 2014-03-20T12:23:05Z

Sorry for the delay :) Added provision for the user to pass extra links to the script to linkify. The updated embed code is:

<script>
    (function (d,f,a) {
        // Your settings here
        var o = {maxlinks: 4, includeElems: ['div', 'h'], linkClass: 'link-to-mdn', extraLinks: {'keywordx': 'http://example.com'}};

        var s=d.createElement("script");s.type="text/javascript";if(s.f)
        {s.a=function(){if(s.f=="loaded"||s.f=="complete")
        {s.a=null;mdnPromoteLinks(o)}}}else{s.onload=function()
        {mdnPromoteLinks(o)}}s.src="promote-mdn.js";
        d.getElementsByTagName("head")[0].appendChild(s)
    })(document,'readyState','onreadystatechange');
</script>

darkwing · 2014-03-20T13:59:30Z

media/redesign/js/promote-mdn.js

+    var options = {
+        includeElems: ['p', 'div', 'span'],
+        trackingSting: '?utm_source=js%20snippet&utm_medium=content%20link&utm_campaign=promote%20mdn',
+        maxlinks: 3,


To be consistent with the camel-casing of other Elements, we should call this maxLinks

darkwing · 2014-03-20T14:06:41Z

Yay more progress! 💃

riverspirit · 2014-03-20T15:55:16Z

@darkwing can you take another look please.

darkwing · 2014-03-20T15:59:51Z

media/redesign/js/promote-mdn.js

@@ -0,0 +1,206 @@
+var mdnPromoteLinks = window.mdnPromoteLinks || function (userSettings) {


Can we change this line to:

function PromoteMDNLinks(userSettings) {

darkwing · 2014-03-20T16:08:19Z

After your next commit, can you squash your commits into one? http://davidwalsh.name/squash-commits-git

That way I can more easily send you a pull request with changes in the future! Thank you so much!

darkwing · 2014-03-20T16:08:40Z

Oh, also, do you have a test document you could add to this PR? That way we can compare and test the same way!

Removed dependancy on jQuery Removed jQuery dependancy and use of gloabl for settings. Replacedd massive if block with an early return if browser doesn't support querySelectorAll. Repalced all under_scored names to camelCased Added provision to pass a custom classname to be used on outbound links to MDN. Cleaned up nodelist.forEach a little and added a polyfill for array.forEach Some stupid bug fixes and improvements in keyword mathcing. Renamed maxlinks param to maxLinks for consistency. Fixed typo in a variable name. Rewriting the array forEach shim code to use a standalone function, rather than extending the array prototype. Moved the checking for document.querySelectorAll support to the beginning of the code. Adding provision for the end user to pass extra keywords and links for the script to linkify. A couple of minor improvements.

riverspirit · 2014-03-20T18:16:03Z

Squashed the commits into one large commit.

The test document that I'm using is at https://github.com/riverspirit/promote-mdn-script/blob/master/promote-mdn-test.html

darkwing · 2014-03-20T18:28:32Z

media/redesign/js/promote-mdn.js

+
+    options = extend({}, options, userSettings || {});
+
+    for (var i in options.extraLinks) {


I think we can use your extend function here

…th the inbuilt keyword-links

riverspirit · 2014-03-20T18:40:29Z

Done. Should I squash it again?

darkwing · 2014-03-20T18:41:46Z

Yes please! :) Tomorrow I'm going to download your stuff and then submit PRs for further updates.

Great work on this! I'm so excited!

Removed dependancy on jQuery Removed jQuery dependancy and use of gloabl for settings. Replacedd massive if block with an early return if browser doesn't support querySelectorAll. Repalced all under_scored names to camelCased Added provision to pass a custom classname to be used on outbound links to MDN. Cleaned up nodelist.forEach a little and added a polyfill for array.forEach Some stupid bug fixes and improvements in keyword mathcing. Renamed maxlinks param to maxLinks for consistency. Fixed typo in a variable name. Rewriting the array forEach shim code to use a standalone function, rather than extending the array prototype. Moved the checking for document.querySelectorAll support to the beginning of the code. Adding provision for the end user to pass extra keywords and links for the script to linkify. A couple of minor improvements. Reusing the extend function to merge the user passed keyword-links with the inbuilt keyword-links

riverspirit · 2014-03-20T19:08:12Z

I think I messed up something with the rebasing. My actual squashed commit is riverspirit/kuma@042d82b

But every time I push the squashed commit, I'm being asked to pull again and that is causing another merge commit. Is that ok?

darkwing · 2014-03-21T17:07:12Z

Looking again

darkwing · 2014-03-21T19:08:37Z

I made some updates here: https://gist.github.com/darkwing/9693628

Something is up with your branch though :/ Can you try to rebase again?

riverspirit · 2014-03-22T13:52:37Z

@darkwing, i messed up something big time with the rebase. Is it ok to open a new pull request for the consolidated commit?

darkwing · 2014-03-24T17:54:30Z

Yes, of course :) I don't need credit, you've done all the work! :)

darkwing · 2014-03-26T13:51:57Z

@riverspirit I'm going to close this for now until you have your updated PR. Thank you!

riverspirit · 2014-03-26T16:09:13Z

@darkwing once again sorry about the delay. Been busy with http://html5conf.in that we're organizing early next month. Thanks for your encouragement.

darkwing reviewed Mar 9, 2014
View reviewed changes

riverspirit added a commit to riverspirit/promote-mdn-script that referenced this pull request Mar 9, 2014

Several changes and improvements as proposed in mdn/kuma#2111

dfd1f54

darkwing reviewed Mar 20, 2014
View reviewed changes

Reusing the extend function to merge the user passed keyword-links wi…

d63b4b8

…th the inbuilt keyword-links

riverspirit added 4 commits March 21, 2014 00:24

Merge branch 'master' of github.com:riverspirit/kuma

9f68bba

Merge branch 'master' of github.com:riverspirit/kuma

487ffa5

darkwing closed this Mar 26, 2014

riverspirit mentioned this pull request Mar 26, 2014

Adding the embeddable promote MDN JavaScript code. #2195

Closed

riverspirit mentioned this pull request Mar 31, 2014

Adding the Promote MDN embeddable script. #2210

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding the promote-js embed script #2111

Adding the promote-js embed script #2111

riverspirit commented Mar 9, 2014

icaaq commented Mar 9, 2014

darkwing commented Mar 9, 2014

riverspirit commented Mar 9, 2014

riverspirit commented Mar 9, 2014

darkwing Mar 9, 2014

darkwing commented Mar 9, 2014

darkwing Mar 9, 2014

riverspirit commented Mar 9, 2014

a2sheppy commented Mar 12, 2014

riverspirit commented Mar 13, 2014

darkwing commented Mar 18, 2014

darkwing commented Mar 18, 2014

a2sheppy commented Mar 18, 2014

riverspirit commented Mar 18, 2014

riverspirit commented Mar 20, 2014

darkwing Mar 20, 2014

darkwing commented Mar 20, 2014

riverspirit commented Mar 20, 2014

darkwing Mar 20, 2014

darkwing commented Mar 20, 2014

darkwing commented Mar 20, 2014

riverspirit commented Mar 20, 2014

darkwing Mar 20, 2014

riverspirit commented Mar 20, 2014

darkwing commented Mar 20, 2014

riverspirit commented Mar 20, 2014

darkwing commented Mar 21, 2014

darkwing commented Mar 21, 2014

riverspirit commented Mar 22, 2014

darkwing commented Mar 24, 2014

darkwing commented Mar 26, 2014

riverspirit commented Mar 26, 2014

		@@ -0,0 +1,161 @@
		var _mdn_promote_links = window._mdn_promote_links \|\| function (user_settings) {

		@@ -0,0 +1,206 @@
		var mdnPromoteLinks = window.mdnPromoteLinks \|\| function (userSettings) {


		options = extend({}, options, userSettings \|\| {});

		for (var i in options.extraLinks) {

Adding the promote-js embed script #2111

Adding the promote-js embed script #2111

Conversation

riverspirit commented Mar 9, 2014

icaaq commented Mar 9, 2014

darkwing commented Mar 9, 2014

riverspirit commented Mar 9, 2014

riverspirit commented Mar 9, 2014

darkwing Mar 9, 2014

Choose a reason for hiding this comment

darkwing commented Mar 9, 2014

darkwing Mar 9, 2014

Choose a reason for hiding this comment

riverspirit commented Mar 9, 2014

a2sheppy commented Mar 12, 2014

riverspirit commented Mar 13, 2014

darkwing commented Mar 18, 2014

darkwing commented Mar 18, 2014

a2sheppy commented Mar 18, 2014

riverspirit commented Mar 18, 2014

riverspirit commented Mar 20, 2014

darkwing Mar 20, 2014

Choose a reason for hiding this comment

darkwing commented Mar 20, 2014

riverspirit commented Mar 20, 2014

darkwing Mar 20, 2014

Choose a reason for hiding this comment

darkwing commented Mar 20, 2014

darkwing commented Mar 20, 2014

riverspirit commented Mar 20, 2014

darkwing Mar 20, 2014

Choose a reason for hiding this comment

riverspirit commented Mar 20, 2014

darkwing commented Mar 20, 2014

riverspirit commented Mar 20, 2014

darkwing commented Mar 21, 2014

darkwing commented Mar 21, 2014

riverspirit commented Mar 22, 2014

darkwing commented Mar 24, 2014

darkwing commented Mar 26, 2014

riverspirit commented Mar 26, 2014