Remove defer attributes from inline scripts - gist embeds (#13407)

* replace inline attribute defer with external js - show.html.erb

* fix typo in filename

* disable no-undef and bail if not defined

* put js on subfolder utilities

* refactor jsx instead of loading chunk with defer

* fix typo in desc

* better name for global gist helper

* use InstantClick

* working script on preview AND single view, not dynamic import, though

* embed only if gists

* combine all approaches to make dynamic import work

* make gist embeds work on submit comment form

* make gist embeds work on preview comment

* refactor - preview comments and submit

* add pack editComment + helper embedGistsInComments

* comment gist helper

* delete useless file utility

* use new syntax for events

* put code in method embedGistsInComments

* delete older pack gist

* handle edge case 'view full discussion'

* resolve conflict with package ibm-openapi-validator

* resolve conflict with package husky

* empty commit to test random error travis

* better name for class dismiss

* handle future events submit

* delete test code on click toggle form

* delete unused file

* rename pack as js file, not jsx

* Added POC using MutationObserver.

* missing case: notification page

* use custom pack notification page

* add e2e test for comment with embed gist

* add e2e test for preview post with embed gist

* add an extra step in tests to check gist is present

Co-authored-by: Nick Taylor <nick@dev.to>
Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
This commit is contained in:
Julien Maury 2021-06-11 09:22:50 +02:00 committed by GitHub
parent c256ef124a
commit a2677cc0a9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 150 additions and 39 deletions

View file

@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
import linkState from 'linkstate';
import postscribe from 'postscribe';
import { KeyboardShortcuts } from '../shared/components/useKeyboardShortcuts';
import { embedGists } from '../utilities/gist';
import { submitArticle, previewArticle } from './actions';
import { EditorActions, Form, Header, Help, Preview } from './components';
import { Button, Modal } from '@crayons';
@ -41,13 +42,6 @@ const LINT_OPTIONS = {
};
export class ArticleForm extends Component {
static handleGistPreview() {
const els = document.getElementsByClassName('ltag_gist-liquid-tag');
for (let i = 0; i < els.length; i += 1) {
postscribe(els[i], els[i].firstElementChild.outerHTML);
}
}
static handleRunkitPreview() {
activateRunkitTags();
}
@ -142,7 +136,7 @@ export class ArticleForm extends Component {
const { previewResponse } = this.state;
if (previewResponse) {
this.constructor.handleGistPreview();
embedGists(this.formElement);
this.constructor.handleRunkitPreview();
this.constructor.handleAsciinemaPreview();
}
@ -374,6 +368,9 @@ export class ArticleForm extends Component {
return (
<form
ref={(element) => {
this.formElement = element;
}}
id="article-form"
className="crayons-article-form"
onSubmit={this.onSubmit}

View file

@ -1,6 +1,7 @@
import { h, render } from 'preact';
import { Snackbar, addSnackbarItem } from '../Snackbar';
import { addFullScreenModeControl } from '../utilities/codeFullscreenModeSwitcher';
import { embedGists } from '../utilities/gist';
import { initializeDropdown } from '@utilities/dropdownUtils';
/* global Runtime */
@ -161,3 +162,6 @@ actionsContainer.addEventListener('click', async (event) => {
toggleArticlePin(event.target);
}
});
const targetNode = document.querySelector('#comments');
targetNode && embedGists(targetNode);

View file

@ -0,0 +1,4 @@
import { embedGists } from '../utilities/gist';
const targetNode = document.querySelector('.comment-form');
targetNode && embedGists(targetNode);

View file

@ -0,0 +1,12 @@
import { embedGists } from '../utilities/gist';
function handleEmbedGists() {
const targetNode = document.querySelector('#articles-list');
targetNode && embedGists(targetNode);
}
window.InstantClick.on('change', () => {
handleEmbedGists();
});
handleEmbedGists();

View file

@ -0,0 +1,4 @@
import { embedGists } from '../utilities/gist';
const targetNode = document.querySelector('#comments-container');
targetNode && embedGists(targetNode);

View file

@ -0,0 +1,74 @@
let postscribeImport;
async function getPostScribe() {
if (postscribeImport) {
// Grab the cached import so we're not always fetching it from the network.
return postscribeImport;
}
const { default: postscribe } = await import('postscribe');
postscribeImport = postscribe;
return postscribeImport;
}
function getGistTags(nodes) {
const gistNodes = [];
for (const node of nodes) {
if (node.nodeType === 1) {
if (node.classList.contains('ltag_gist-liquid-tag')) {
gistNodes.push(node);
}
gistNodes.push(...node.querySelectorAll('.ltag_gist-liquid-tag'));
}
}
return gistNodes;
}
function loadEmbeddedGists(postscribe, gistTags) {
for (const gistTag of gistTags) {
postscribe(gistTag, gistTag.firstElementChild.outerHTML, {
beforeWrite(text) {
return gistTag.childElementCount > 3 ? '' : text;
},
});
}
}
function watchForGistTagInsertion(targetNode, postscribe) {
const config = { attributes: false, childList: true, subtree: true };
const callback = function (mutationsList) {
for (const { type, addedNodes } of mutationsList) {
if (type === 'childList' && addedNodes.length > 0) {
loadEmbeddedGists(postscribe, getGistTags(addedNodes));
}
}
};
const observer = new MutationObserver(callback);
observer.observe(targetNode, config);
InstantClick.on('change', () => {
observer.disconnect();
});
window.addEventListener('beforeunload', () => {
observer.disconnect();
});
}
export async function embedGists(targetNode) {
const postscribe = await getPostScribe();
// Load gist tags that were rendered server-side
loadEmbeddedGists(
postscribe,
document.querySelectorAll('.ltag_gist-liquid-tag'),
);
watchForGistTagInsertion(targetNode, postscribe);
}

View file

@ -14,32 +14,6 @@
<% end %>
<% cache("content-related-optional-scripts-#{@article.id}-#{@article.updated_at}-#{internal_navigation?}-#{user_signed_in?}", expires_in: 30.hours) do %>
<% if @article.processed_html.include? "ltag_gist-liquid-tag" %>
<script src="https://cdnjs.cloudflare.com/ajax/libs/postscribe/2.0.8/postscribe.min.js"
integrity="sha256-xOIPU/XvDtRLeDQ3qj9GOKmlbMSqKa6D7ZIS6ygHBSo="
crossorigin="anonymous" defer></script>
<script defer>
var waitingOnPostscribe = setInterval(function () {
if (typeof (postscribe) === "function") {
clearInterval(waitingOnPostscribe);
var els = document.getElementsByClassName("ltag_gist-liquid-tag");
for (var i = 0; i < els.length; i++) {
let current = els[i];
postscribe(current, current.firstElementChild.outerHTML, {
beforeWrite: function (context) {
return function (text) {
if (context.childElementCount > 3) {
return "";
}
return text;
}
}(current)
});
}
}
}, 500);
</script>
<% end %>
<% unless internal_navigation? || user_signed_in? %>
<script type="application/ld+json">
<%= @article_json_ld.to_json.html_safe %>

View file

@ -92,7 +92,7 @@
<div class="comment-form__buttons mb-4">
<button type="submit" class="crayons-btn mr-2 js-btn-enable" onclick="validateField(event)" disabled>Submit</button>
<button type="button" class="preview-toggle crayons-btn crayons-btn--secondary comment-action-preview js-btn-enable mr-2" disabled>Preview</button>
<a href="<%= @comment.path %>" class="crayons-btn crayons-btn--ghost js-btn-dismiss hidden">Dismiss</a>
<a href="<%= @comment.path %>" class="dismiss-edit-comment crayons-btn crayons-btn--ghost js-btn-dismiss hidden">Dismiss</a>
</div>
</div>

View file

@ -1,3 +1,4 @@
<%= javascript_packs_with_chunks_tag "commentsDisplay", defer: true %>
<div class="crayons-layout crayons-layout--limited-l gap-0">
<div
class="comments-container crayons-card min-w-0 text-padding"

View file

@ -1,3 +1,4 @@
<%= javascript_packs_with_chunks_tag "postCommentsPage", defer: true %>
<% if @root_comment.present? %>
<% title("#{@root_comment.title} - #{community_name}") %>
<% else %>
@ -86,7 +87,7 @@
</article>
<% else %>
<div id="response-templates-data" class="hidden"></div>
<%= javascript_packs_with_chunks_tag "responseTemplates", defer: true %>
<%= javascript_packs_with_chunks_tag "responseTemplates", "commentsDisplay", defer: true %>
<header class="p-2 pb-4 m:p-6 m:pb-6 crayons-card crayons-card--secondary s:mx-2 m:mx-4 -mb-1 z-0">
<h1 class="crayons-subtitle-1 mb-4">
<span class="fw-normal color-base-60">Discussion on: </span>
@ -94,7 +95,7 @@
</h1>
<div class="flex">
<a class="crayons-btn crayons-btn--outlined mr-2" href="<%= @commentable.path %>">View post</a>
<a class="crayons-btn crayons-btn--outlined mr-2" href="<%= @commentable.path %><%= user_signed_in? ? "/" : "#" %>comments">Full discussion</a>
<a class="view-discussion crayons-btn crayons-btn--outlined mr-2" href="<%= @commentable.path %><%= user_signed_in? ? "/" : "#" %>comments">Full discussion</a>
</div>
</header>
<% end %>

View file

@ -15,6 +15,8 @@
<meta name="twitter:site" content="@<%= Settings::General.social_media_handles["twitter"] %>">
<meta name="twitter:title" content="Notifications - <%= community_name %>">
<meta name="twitter:description" content="Notifications inbox for <%= community_name %>">
<%= javascript_packs_with_chunks_tag "notificationPage", defer: true %>
<% end %>
<script>

View file

@ -501,6 +501,21 @@ describe('Comment on articles', () => {
cy.findByRole('button', { name: /^Submit$/i }).should('have.focus');
});
it('should add a comment with a gist embed', () => {
cy.findByRole('main').within(() => {
cy.findByRole('textbox', {
name: /^Add a comment to the discussion$/i,
}).type(
'Here is a gist: {% gist https://gist.github.com/CristinaSolana/1885435.js %}',
{ parseSpecialCharSequences: false },
);
cy.findByRole('button', { name: /^Submit$/i }).click();
});
cy.get('#gist1885435').should('be.visible');
cy.findByRole('link', { name: 'view raw' });
});
it('should provide a dropdown of options', () => {
cy.findByRole('main').within(() => {
// Add a comment

View file

@ -11,7 +11,7 @@ describe('Post Editor', () => {
});
});
it('should preview blank content of an post', () => {
it('should preview blank content of a post', () => {
cy.findByRole('form', { name: /^Edit post$/i }).as('articleForm');
cy.get('@articleForm')
@ -94,7 +94,7 @@ describe('Post Editor', () => {
});
});
it('should preview blank content of an post', () => {
it('should preview blank content of a post', () => {
cy.findByRole('form', { name: /^Edit post$/i }).as('articleForm');
cy.get('@articleForm')
@ -108,6 +108,29 @@ describe('Post Editor', () => {
cy.findByTestId('error-message').should('not.exist');
});
it('should preview content of a post with a gist embed', () => {
cy.findByRole('form', { name: /^Edit post$/i }).as('articleForm');
cy.get('@articleForm')
.findByLabelText('Post Content')
.type(
'Here is a gist: {% gist https://gist.github.com/CristinaSolana/1885435.js %}',
{ parseSpecialCharSequences: false },
);
cy.get('@articleForm')
.findByRole('button', { name: /^Preview$/i })
.as('previewButton');
cy.get('@previewButton').should('not.have.attr', 'aria-current');
cy.get('@previewButton').click();
cy.get('@previewButton').should('have.attr', 'aria-current', 'page');
cy.findByTestId('error-message').should('not.exist');
cy.get('#gist1885435').should('be.visible');
cy.findByRole('link', { name: 'view raw' });
});
it(`should show error if the post content can't be previewed`, () => {
cy.findByRole('form', { name: /^Edit post$/i }).as('articleForm');