Skip to content
This repository has been archived by the owner on Dec 3, 2021. It is now read-only.

Can pass this to runSnippetInTab() to run previous code snippet #45

Merged
merged 2 commits into from
Apr 21, 2019

Conversation

jnpr-raylam
Copy link
Contributor

To avoid batch update the index in "Run this snippet" for insertion/removal of single snippet, suggest to modify the code, so that it accepts either an index or this object so it doesn't break existing lab guide content.

After the change, we can pass the index as usual:

<button type="button" class="btn btn-primary btn-sm" onclick="runSnippetInTab('vqfx', 0)">Run this snippet</button>

or pass this object:

<button type="button" class="btn btn-primary btn-sm" onclick="runSnippetInTab('vqfx', this)">Run this snippet</button>

and it will run the previous snippet just before the button object.

Copy link
Member

@Mierdin Mierdin left a comment

Choose a reason for hiding this comment

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

Just one comment below. Also, please add a CHANGELOG entry. Overall though, this is great work! Will make things much simpler in lesson guides.

@@ -42,10 +42,15 @@ function runSnippetInTab(tabName, snippetIndex) {
// Select tab
$('.nav-tabs a[href="#' + tabName + '"]').tab('show')

if (typeof snippetIndex == 'number') {
var snippetText = document.getElementById('labGuide').getElementsByTagName('pre')[parseInt(snippetIndex)].innerText;
} else if (typeof snippetIndex == 'object') {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
} else if (typeof snippetIndex == 'object') {
} else {

Let's just catch all non-number cases here. If there's an issue accessing the object fields, that will get popped up to the javascript console.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied the change according to your suggestion, and updated the CHANGELOG as well.

@Mierdin
Copy link
Member

Mierdin commented Apr 19, 2019

Excellent, thanks. I'd like to also update the nrelabs curriculum to use this (even though it's not strictly required based on your code here) so that people that look at other lessons for inspiration can see this as the de facto standard.

Since this will result in a fairly big change (and v0.3.2 is out today) I'll merge this for the v0.4.0 release.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants