Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Bug 1148743 - Add button to open samples in JSFiddle #3147

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions media/js/wiki.js
Original file line number Diff line number Diff line change
Expand Up @@ -847,5 +847,90 @@
if($('details').length){
initDetailsTags();
}

function openJSFiddle(title, htmlCode, cssCode, jsCode) {
var $form = $('<form method="post" target="_blank" action="https://jsfiddle.net/api/mdn/">'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a large string, let's create the elements with JS. We retrieve most of them to set their values anyway.

+ '<input type="hidden" name="html" />'
+ '<input type="hidden" name="css" />'
+ '<input type="hidden" name="js" />'
+ '<input type="hidden" name="title" />'
+ '<input type="hidden" name="wrap" value="b" />'
+ '<input type="submit" />'
+ '</form>');
$form.find('input[name=html]').val(htmlCode);
$form.find('input[name=css]').val(cssCode);
$form.find('input[name=js]').val(jsCode);
$form.find('input[name=title]').val(title);
$form.submit();
}

function openCodepen(title, htmlCode, cssCode, jsCode) {
var $form = $('<form method="post" target="_blank" action="http://codepen.io/pen/define">'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here -- let's create these elements programatically

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think creating elements programatically will make the code too long and unreadable. But I'm happy to do it, if this breaks our style guide.

Also, should I move my code into separate file. IIRC, we want to waffle flag this feature as well as A/B test. Is this part as simple as creating separate js file, or do I need to do waffle flag checking in js also?

Let me know if there is anything else that needs to be fixed. I want this feature in production asap :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's waffle this, please

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be sure, did you see that the buttons need to be manually inserted with JS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be suffice to move the code to a new file for waffle flag or is there more to it? Can you link me to a commit that does something similar?

+ '<input type="hidden" name="data">'
+ '<input type="submit" />'
+ '</form>');
var data = {'title': title, 'html': htmlCode, 'css': cssCode, 'js': jsCode};
$form.find('input[name=data]').val(JSON.stringify(data));
$form.submit();
}

function openSample(sampleCodeHost, title, htmlCode, cssCode, jsCode) {
if(sampleCodeHost == 'jsfiddle') {
openJSFiddle(title, htmlCode, cssCode, jsCode);
} else if(sampleCodeHost == 'codepen') {
openCodepen(title, htmlCode, cssCode, jsCode);
}
}

$('#wikiArticle').on('click', 'button.open-in-host', function(){
var $button = $(this);
var section = $button.attr('data-section');
var sampleCodeHost = $button.attr('data-host');

// track the click and sample code host
mdn.analytics.trackEvent({
category: 'Samples',
action: 'open-' + sampleCodeHost,
label: section
});

// disable the button, till we open the fiddle
$button.attr('disabled', 'disabled');
var sampleUrl = window.location.href.split('#')[0] + '?section=' + section + '&raw=1';
$.get(sampleUrl).then(function(sample) {
var $sample = $('<div />').append(sample);
var htmlCode = $sample.find('.brush\\:.html, .brush\\:.html\\;').text();
var cssCode = $sample.find('.brush\\:.css, .brush\\:.css\\;').text();
var jsCode = $sample.find('.brush\\:.js, .brush\\:.js\\;').text();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darkwing Will these selectors match all our code sample snippets?

I have written selectors for the following 2 variations

// Case 1
<pre class="brush: js"></pre>
// Case 2
<pre class="brush: js; highlight:[7]"></pre>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm not sure if converting code to base64 is necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering the same thing. What do you think, @darkwing?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not - see example in https://jsfiddle.net/zalun/mk29ttc2/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will these selectors match all our code sample snippets?

12:09 <davidwalsh> I believe so
12:09 <davidwalsh> I think taht's right

var title = $sample.find('h2[name=' + section + ']').text();

openSample(sampleCodeHost, title, htmlCode, cssCode, jsCode);

$button.removeAttr('disabled');
});
});

function createSampleButtons(section, sites) {
var buttons = '<div style="margin-bottom:10px;">';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't inline styles -- a CSS class should be created.

sites.forEach(function(site){
// convert sitename to lowercase for icon name and host identifier
buttons += '<button class="open-in-host" data-host="'+ site.toLowerCase() +'" data-section="' +
section + '"><i aria-hidden="true" class="icon-'+ site.toLowerCase() +'"></i> Open in ' +
site +'</button>';
});
buttons += '</div>';
return buttons;
}

mdn.insertSampleButtons = function(sites) {
// using id to get sample code section since macros discard other attributes
$('.sample-code-frame').before(function() {
var section = $(this).attr('id').substring("frame_".length);
return createSampleButtons(section, sites);
});
$('.sample-code-table').before(function(){
var section = $(this).find('iframe').attr('id').substring("frame_".length);
return createSampleButtons(section, sites);
});
}
})(window, document, jQuery);
4 changes: 4 additions & 0 deletions media/stylus/base/elements/forms.styl
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,7 @@ label {
text-decoration: none;
}
}

.open-in-host + .open-in-host {
bidi-style(margin-left, ($grid-spacing / 2), margin-right, 0);
}