Fix Runkit tags not being activated when comment is added (#6767)

* Fix Runkit tags not being activated when comment is added

* Runkit tag activation was ran once, on page load. I've changed it to
  run on on comment preview and submit.
* It was necessary to add a check to skip already activated Runkit
  tags. The code didn't take into account that a tag could be already
  processed, and would just crash.

* Fix Runkit tags and add tests

* Add test for previewing article with Runkit tag

* "Fix CodeClimate not finding waitForRunkitAndActivateTags()

* Refactor a test for readability

* Make Rubocop happy

* Improve test for previewing article with Runkit tag

* Use one method for determining active Runkit tags

* Defer loading of JS from embed.runkit.com

* Add utility function dynamicallyLoadScript(url)

* Switch to dynamic loading of Runkit

* parsed content code block is also hidden now to avoid displaying <code /> block before runkit iframe loads.

* Fix Runkit test

* Remove Runkit script caching in v2 form

* Use <%== instead of .html_safe in v2 form

* Update app/assets/javascripts/utilities/dynamicallyLoadScript.js

Co-authored-by: Nick Taylor <nick@iamdeveloper.com>

Co-authored-by: Nick Taylor <nick@iamdeveloper.com>

app/assets/javascripts/initializers/initializeCommentPreview.js

@@ -1,4 +1,4 @@
-'use strict';
+/* global activateRunkitTags */
 
 function getAndShowPreview(markdownPreviewPane, markdownEditor) {
   function successCb(body) {
@@ -6,6 +6,7 @@ function getAndShowPreview(markdownPreviewPane, markdownEditor) {
     markdownEditor.classList.toggle('preview-loading');
     markdownEditor.classList.toggle('preview-toggle');
     markdownPreviewPane.innerHTML = body.processed_html; // eslint-disable-line no-param-reassign
+    activateRunkitTags();
   }
 
   const payload = JSON.stringify({
@@ -15,11 +16,11 @@ function getAndShowPreview(markdownPreviewPane, markdownEditor) {
   });
   getCsrfToken()
     .then(sendFetch('comment-preview', payload))
-    .then(response => {
+    .then((response) => {
       return response.json();
     })
     .then(successCb)
-    .catch(err => {
+    .catch((err) => {
       console.log('error!'); // eslint-disable-line
       console.log(err); // eslint-disable-line no-console
     });

app/assets/javascripts/initializers/initializeCommentsPage.js.erb

@@ -247,6 +247,7 @@ function handleCommentSubmit(event) {
           initializeCommentsPage();
           initializeCommentDate();
           initializeCommentDropdown();
+          activateRunkitTags();
         })
       } else {
         response.json().then(function parseError(errorReponse) {

app/assets/javascripts/utilities/dynamicallyLoadScript.js

@@ -0,0 +1,10 @@
+
+
+function dynamicallyLoadScript(url) {
+  if (document.querySelector(`script[src='${url}']`)) return;
+
+  const script = document.createElement('script');
+  script.src = url;
+
+  document.head.appendChild(script);
+}

app/javascript/article-form/articleForm.jsx

@@ -21,6 +21,8 @@ import KeyboardShortcutsHandler from './elements/keyboardShortcutsHandler';
 import Tags from '../shared/components/tags';
 import { OrganizationPicker } from '../organization/OrganizationPicker';
 
+/* global activateRunkitTags */
+
 const SetupImageButton = ({
   className,
   imgSrc,
@@ -53,19 +55,7 @@ export default class ArticleForm extends Component {
   }
 
   static handleRunkitPreview() {
-    const targets = document.getElementsByClassName('runkit-element');
-    for (let i = 0; i < targets.length; i += 1) {
-      if (targets[i].children.length > 0) {
-        const preamble = targets[i].children[0].textContent;
-        const content = targets[i].children[1].textContent;
-        targets[i].innerHTML = '';
-        window.RunKit.createNotebook({
-          element: targets[i],
-          source: content,
-          preamble,
-        });
-      }
-    }
+    activateRunkitTags();
   }
 
   static propTypes = {
@@ -173,7 +163,7 @@ export default class ArticleForm extends Component {
     };
   };
 
-  toggleHelp = e => {
+  toggleHelp = (e) => {
     const { helpShowing } = this.state;
     e.preventDefault();
     window.scrollTo(0, 0);
@@ -182,7 +172,7 @@ export default class ArticleForm extends Component {
     });
   };
 
-  fetchPreview = e => {
+  fetchPreview = (e) => {
     const { previewShowing, bodyMarkdown } = this.state;
     e.preventDefault();
     if (previewShowing) {
@@ -194,7 +184,7 @@ export default class ArticleForm extends Component {
     }
   };
 
-  toggleImageManagement = e => {
+  toggleImageManagement = (e) => {
     const { imageManagementShowing } = this.state;
     e.preventDefault();
     window.scrollTo(0, 0);
@@ -205,7 +195,7 @@ export default class ArticleForm extends Component {
     });
   };
 
-  toggleMoreConfig = e => {
+  toggleMoreConfig = (e) => {
     const { moreConfigShowing } = this.state;
     e.preventDefault();
     this.setState({
@@ -213,7 +203,7 @@ export default class ArticleForm extends Component {
     });
   };
 
-  showPreview = response => {
+  showPreview = (response) => {
     if (response.processed_html) {
       this.setState({
         ...this.setCommonProps({ previewShowing: true }),
@@ -228,25 +218,25 @@ export default class ArticleForm extends Component {
     }
   };
 
-  handleOrgIdChange = e => {
+  handleOrgIdChange = (e) => {
     const organizationId = e.target.selectedOptions[0].value;
     this.setState({ organizationId });
   };
 
-  failedPreview = response => {
+  failedPreview = (response) => {
     // TODO: console.log should not be part of production code. Remove it!
     // eslint-disable-next-line no-console
     console.log(response);
   };
 
-  handleConfigChange = e => {
+  handleConfigChange = (e) => {
     e.preventDefault();
     const newState = {};
     newState[e.target.name] = e.target.value;
     this.setState(newState);
   };
 
-  handleMainImageUrlChange = payload => {
+  handleMainImageUrlChange = (payload) => {
     this.setState({
       mainImage: payload.links[0],
       imageManagementShowing: false,
@@ -259,7 +249,7 @@ export default class ArticleForm extends Component {
     window.removeEventListener('beforeunload', this.localStoreContent);
   };
 
-  onPublish = e => {
+  onPublish = (e) => {
     e.preventDefault();
     this.setState({ submitting: true, published: true });
     const { state } = this;
@@ -267,7 +257,7 @@ export default class ArticleForm extends Component {
     submitArticle(state, this.removeLocalStorage, this.handleArticleError);
   };
 
-  onSaveDraft = e => {
+  onSaveDraft = (e) => {
     e.preventDefault();
     this.setState({ submitting: true, published: false });
     const { state } = this;
@@ -275,15 +265,15 @@ export default class ArticleForm extends Component {
     submitArticle(state, this.removeLocalStorage, this.handleArticleError);
   };
 
-  handleTitleKeyDown = e => {
+  handleTitleKeyDown = (e) => {
     if (e.keyCode === 13) {
       e.preventDefault();
     }
   };
 
-  handleBodyKeyDown = _e => {};
+  handleBodyKeyDown = (_e) => {};
 
-  onClearChanges = e => {
+  onClearChanges = (e) => {
     e.preventDefault();
     // eslint-disable-next-line no-alert
     const revert = window.confirm(
@@ -314,7 +304,7 @@ export default class ArticleForm extends Component {
     });
   };
 
-  handleArticleError = response => {
+  handleArticleError = (response) => {
     window.scrollTo(0, 0);
     this.setState({
       errors: response,

app/liquid_tags/runkit_tag.rb

@@ -2,32 +2,68 @@ class RunkitTag < Liquid::Block
   PARTIAL = "liquids/runkit".freeze
 
   SCRIPT = <<~JAVASCRIPT.freeze
-    var checkRunkit = setInterval(function() {
-      try {
-        if (typeof(RunKit) !== 'undefined') {
-          var targets = document.getElementsByClassName("runkit-element");
-          for (var i = 0; i < targets.length; i++) {
-            var wrapperContent = targets[i].textContent;
-            if (/^(\<iframe src)/.test(wrapperContent) === false) {
-              if (targets[i].children.length > 0) {
-                var preamble = targets[i].children[0].textContent;
-                var content = targets[i].children[1].textContent;
-                targets[i].innerHTML = "";
-                var notebook = RunKit.createNotebook({
-                  element: targets[i],
-                  source: content,
-                  preamble: preamble
-                });
-              }
-            }
+    function activateRunkitTags() {
+      if (!areAnyRunkitTagsPresent())
+        return
+
+      var checkRunkit = setInterval(function() {
+        try {
+          dynamicallyLoadRunkitLibrary()
+
+          if (typeof(RunKit) === 'undefined') {
+            return
           }
+
+          replaceTagContentsWithRunkitWidget()
+          clearInterval(checkRunkit);
+        } catch(e) {
+          console.error(e);
           clearInterval(checkRunkit);
         }
-      } catch(e) {
-        console.error(e);
-        clearInterval(checkRunkit);
+      }, 200);
+    }
+
+    function isRunkitTagAlreadyActive(runkitTag) {
+      return runkitTag.querySelector("iframe") !== null;
+    };
+
+    function areAnyRunkitTagsPresent() {
+      var presentRunkitTags = document.getElementsByClassName("runkit-element");
+
+      return presentRunkitTags.length > 0
+    }
+
+    function replaceTagContentsWithRunkitWidget() {
+      var targets = document.getElementsByClassName("runkit-element");
+      for (var i = 0; i < targets.length; i++) {
+        if (isRunkitTagAlreadyActive(targets[i])) {
+          continue;
+        }
+
+        var wrapperContent = targets[i].textContent;
+        if (/^(\<iframe src)/.test(wrapperContent) === false) {
+          if (targets[i].children.length > 0) {
+            var preamble = targets[i].children[0].textContent;
+            var content = targets[i].children[1].textContent;
+            targets[i].innerHTML = "";
+            var notebook = RunKit.createNotebook({
+              element: targets[i],
+              source: content,
+              preamble: preamble
+            });
+          }
+        }
       }
-    }, 200);
+    };
+
+    function dynamicallyLoadRunkitLibrary() {
+      if (typeof(dynamicallyLoadScript) === "undefined")
+        return
+
+      dynamicallyLoadScript("//embed.runkit.com")
+    }
+
+    activateRunkitTags();
   JAVASCRIPT
 
   def initialize(tag_name, markup, tokens)

app/views/articles/_v2_form.html.erb

@@ -1,5 +1,3 @@
-<%= javascript_include_tag "https://embed.runkit.com", defer: true %>
-
 <div class="articleformcontainer">
   <div class="articleform"
        id="article-form"
@@ -51,6 +49,10 @@
   </div>
 </div>
 
+<script async>
+  <%== RunkitTag.script %>
+</script>
+
 <style>
   #footer-container {
     display: none

app/views/articles/show.html.erb

@@ -35,10 +35,6 @@
       <%= @article_json_ld.to_json.html_safe %>
     </script>
   <% end %>
-
-  <% if @article.processed_html.include? "runkit-element" %>
-    <%= javascript_include_tag "https://embed.runkit.com" %>
-  <% end %>
 <% end %>
 
 <% if internal_navigation? %>

app/views/liquids/_runkit.html.erb

@@ -2,7 +2,7 @@
   <code style="display: none">
     <%= preamble %>
   </code>
-  <code>
+  <code style="display: none;">
     <%= parsed_content %>
   </code>
 </div>

spec/fixtures/files/article_with_runkit_tag.txt

@@ -0,0 +1,12 @@
+---
+title: Sample Article
+published: true
+description: this is a sample article
+tags: test
+---
+
+Suspendisse ac lobortis velit, a feugiat sapien. Aenean condimentum, nulla at fermentum sagittis, tellus nisi suscipit velit, vel sollicitudin odio ligula a odio. Integer eget efficitur massa, in sodales velit. Nunc fermentum consequat scelerisque. Morbi elementum tristique faucibus. Nulla vel lectus non justo euismod varius. Vivamus id nisl sit amet odio tincidunt fringilla. Pellentesque odio odio, vulputate in risus eu, lacinia porttitor lorem. Nunc posuere tempus est, imperdiet suscipit odio maximus id. Nam eget feugiat magna.
+
+{% runkit %}
+console.log(42)
+{% endrunkit %}

spec/support/fixtures/approvals/runkit_liquid_tag_spec.approved.html

@@ -6,7 +6,7 @@
     const myVar = 9001
 
   </code>
-  <code>
+  <code style="display: none;">
     // GeoJSON!
 var getJSON = require("async-get-json");
 

spec/support/runkit_tag_context.rb

@@ -0,0 +1,18 @@
+shared_context "with runkit_tag" do
+  def compose_runkit_comment(title)
+    <<~COMMENT
+      #{title}
+      {% runkit %}
+      console.log("#{title}")
+      {% endrunkit %}
+    COMMENT
+  end
+
+  def expect_runkit_tag_to_be_active(count: 1)
+    expect(page).to have_css(".runkit-element iframe", count: count)
+  end
+
+  def expect_no_runkit_tag_to_be_active
+    expect_runkit_tag_to_be_active count: 0
+  end
+end

spec/system/articles/user_creates_an_article_spec.rb

@@ -1,8 +1,13 @@
 require "rails_helper"
 
 RSpec.describe "Creating an article with the editor", type: :system do
+  include_context "with runkit_tag"
+
   let(:user) { create(:user) }
   let!(:template) { file_fixture("article_published.txt").read }
+  let!(:template_with_runkit_tag) do
+    file_fixture("article_with_runkit_tag.txt").read
+  end
 
   before do
     sign_in user
@@ -14,4 +19,30 @@ RSpec.describe "Creating an article with the editor", type: :system do
     click_button "SAVE CHANGES"
     expect(page).to have_selector("header h1", text: "Sample Article")
   end
+
+  context "with Runkit tag", js: true do
+    it "creates a new article with a Runkit tag" do
+      visit new_path
+      fill_in "article_body_markdown", with: template_with_runkit_tag
+      click_button "SAVE CHANGES"
+
+      expect_runkit_tag_to_be_active
+    end
+
+    it "previews article with a Runkit tag and creates it" do
+      visit new_path
+      fill_in "article_body_markdown", with: template_with_runkit_tag
+      click_button "PREVIEW"
+
+      expect_runkit_tag_to_be_active
+
+      click_button "EDIT"
+
+      expect_no_runkit_tag_to_be_active
+
+      click_button "SAVE CHANGES"
+
+      expect_runkit_tag_to_be_active
+    end
+  end
 end

spec/system/comments/user_fills_out_comment_spec.rb

@@ -1,8 +1,13 @@
 require "rails_helper"
 
 RSpec.describe "Creating Comment", type: :system, js: true do
+  include_context "with runkit_tag"
+
   let(:user) { create(:user) }
   let(:raw_comment) { Faker::Lorem.paragraph }
+  let(:runkit_comment) { compose_runkit_comment "comment 1" }
+  let(:runkit_comment2) { compose_runkit_comment "comment 2" }
+
   # the article should be created before signing in
   let!(:article) { create(:article, user_id: user.id, show_comments: true) }
 
@@ -20,6 +25,40 @@ RSpec.describe "Creating Comment", type: :system, js: true do
     expect(page).to have_text(raw_comment)
   end
 
+  context "with Runkit tags" do
+    before do
+      visit article.path.to_s
+
+      wait_for_javascript
+    end
+
+    it "Users fills out comment box with a Runkit tag" do
+      fill_in "text-area", with: runkit_comment
+      click_button("SUBMIT")
+
+      expect_runkit_tag_to_be_active
+    end
+
+    it "Users fills out comment box 2 Runkit tags" do
+      fill_in "text-area", with: runkit_comment
+      click_button("SUBMIT")
+
+      expect_runkit_tag_to_be_active
+
+      fill_in "text-area", with: runkit_comment2
+      click_button("SUBMIT")
+
+      expect_runkit_tag_to_be_active(count: 2)
+    end
+
+    it "User fill out comment box with a Runkit tag, then clicks preview" do
+      fill_in "text-area", with: runkit_comment
+      click_button("PREVIEW")
+
+      expect_runkit_tag_to_be_active
+    end
+  end
+
   it "User fill out comment box then click previews and submit" do
     visit article.path.to_s